Build vararg support only where it is reachable

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.

[jkoritzinsky]

Build vararg support only where it is reachable

Stacked on #131654 — targets dev/jkoritzinsky/calli-pinvoke, not main.

What and why

The vararg calling convention is only supported on Windows: ConvToJitSig rejects both IMAGE_CEE_CS_CALLCONV_VARARG and IMAGE_CEE_CS_CALLCONV_NATIVEVARARG everywhere else. The supporting infrastructure was nonetheless compiled on every platform — assembly stubs, cookie allocation, GC reporting, marshalling helpers and their managed surface.

This introduces FEATURE_VARARGS and builds that machinery only where it can be reached. No behavior change on Windows.

ConvToJitSig now keys off the same feature instead of spelling out TARGET_UNIX || TARGET_ARM, so there is a single definition of "varargs are supported" rather than two that have to be kept in agreement. That is not a behavior change: browser and wasi both set CLR_CMAKE_TARGET_UNIX (configureplatform.cmake lines 434 and 439), so every target that threw before still throws.

The feature is declared twice, deliberately

# clrdefinitions.cmake — nativeif (CLR_CMAKE_TARGET_WIN32)

<!-- clr.featuredefines.props — managed, alongside FeatureIjw / FeatureCominterop -->
<PropertyGroupCondition="'$(TargetsWindows)' == 'true'">

Both are required, and the reason is worth writing down because the failure mode is silent.

vm/corelib.h is not only a C++ header. It is also parsed by the CreateRuntimeRootILLinkDescriptorFile MSBuild task (System.Private.CoreLib/CreateRuntimeRootILLinkDescriptorFile.targets) to generate the ILLink root descriptor for System.Private.CoreLib — and that task receives the managed $(DefineConstants), not the native CMake defines.

So a native-only #ifdef around a DEFINE_METHOD there still compiles, but the ILLink root quietly disappears, the trimmer removes a method the runtime resolves by name, and you get:

Assert failure: pMD != NULL && ("EE expects method to exist")
CoreLibBinder::LookupMethodLocal
ILCodeStream::EmitCALL
ILArgIteratorMarshaler::EmitConvertSpaceAndContentsCLRToNative

which points at the marshaller and gives no hint that a build-system conditional is the cause. The existing #ifdef FEATURE_COMINTEROP in that header is not a counter-example: FEATURE_COMINTEROP is emitted on both sides by clr.featuredefines.props, so its root is dropped exactly when the managed code is also absent. Declaring FeatureVarargs in the same place gives FEATURE_VARARGS that same symmetry, which is what makes gating corelib.h safe here.

Deleted outright

Unix VarargPInvokeStub implementations in amd64, arm, arm64, loongarch64 and riscv64pinvokestubs.S and in i386/asmhelpers.S; the wasm PORTABILITY_ASSERT placeholders in vm/wasm/helpers.cpp; and the VASigCookie offsets in the arm, loongarch64 and riscv64asmconstants.h. None of those files is deletable whole — each also holds JIT_PInvokeBegin/End/InitPInvokeFrame.

Gated — native

The cookie machinery (struct VASigCookie and its cDAC data descriptor, VASigCookieBlock, Module::GetVASigCookie/GetVASigCookieWorker, VASigCookieEx), VarargPInvokeStubWorker, GetILStubForVarargPInvoke, TheVarargPInvokeStub, InteropDispatchStubManager (plus its vptr_list.h entry and Init()), clrvarargs.cpp and varargsnative.cpp, the ArgIterator → va_list marshaller and the StubHelpers entrypoints behind it, TransitionFrame::GetVASigCookie, and the vararg GC reporting in eetwain.cpp and gc_unwind_x86.inl.

Module::GetLoaderModuleForSignature is not gated — the unmanaged-calli stub path calls it on every platform.

The Windows .asm files are guarded rather than excluded. Strictly unnecessary, since every Windows arch enables the feature — but it buys a compile-time proxy: forcing FEATURE_VARARGS off on windows-x64 compiles the whole VM and DAC as a stand-in for the six targets that can't be built locally. That proxy earned its keep immediately by catching promoteVarArgs in eetwain.cpp, which takes PTR_VASigCookie and is compiled on every target — a real cross-platform break that inspection alone had missed.

Gated — managed

NativeAOT's throwing ArgIterator moves to the shared partition as System/ArgIterator.PlatformNotSupported.cs and is now used by CoreCLR too whenever the feature is off, so the throwing copy exists once instead of twice:

Build	ArgIterator source
CoreCLR, varargs supported	its own ArgIterator.cs (real implementation)
CoreCLR, varargs unsupported	shared ArgIterator.PlatformNotSupported.cs
NativeAOT	shared ArgIterator.PlatformNotSupported.cs
Mono	its own real implementation, untouched

The Shared.projitems condition is '$(FeatureVarargs)' != 'true' and '$(FeatureMono)' != 'true'. The Mono clause matters: Mono's CoreLib also imports Shared.projitems, has its own real ArgIterator, and does not import clr.featuredefines.props — without it, Mono would pick up a duplicate System.ArgIterator.

Because CoreCLR's ArgIterator.cs is now included conditionally, its #if TARGET_WINDOWS split and the ~50-line duplicated throwing branch both go away; the file has no preprocessor directives left. The three StubHelpersva_list members that only the marshaller calls are gated the same way.

Deliberately unchanged

• GCREFMAP_VASIG_COOKIE (= 5) — part of the ReadyToRun GC ref map format. Value kept; only its handling is gated.
• DynamicMethodDesc::StubPInvokeVarArg (= 4) — the cDAC contract depends on the value.
• x86 GC info varargs bit / FLIP_VARARGS — serialized format, and x86 is a supported vararg target. There is no unreachable x86 GC/unwind vararg code.
• tools/Common/CallingConvention/ArgIterator.cs — crossgen2 cross-targets, so it cannot be gated at compile time.
• inc/dacdbi.idl / debug/inc/dacdbiinterface.h — versioned interface shape untouched; only the GetVarArgSig implementation is gated.
• getVarArgsHandle keeps its ICorJitInfo vtable slot and asserts instead of being removed. ICorJitInfo is not per-platform and varargs do work on Windows, so the JIT still needs the method there; this change does not reshape the interface.

GetStubForILStub now throws IDS_EE_VARARG_NOT_SUPPORTED for every target without the feature rather than only under FEATURE_PORTABLE_ENTRYPOINTS — a strict generalization, since that feature is wasm-only and already outside FEATURE_VARARGS.

cDAC

struct VASigCookie and its CDAC_TYPE descriptor are gated, so a target without the feature stops advertising a type it can never allocate. The managed cDAC is unchanged and reads the target's own descriptor, so older runtimes are unaffected; the lookup is lazy and only reachable from DacDbiImpl.GetVarArgSig on a vararg frame. Verified by inspecting the emitted contract descriptor in both configurations.

Renamed

PINVOKE_CALLI_SIGTOKEN_REGNUM/REGISTER on amd64 → PINVOKE_VARARG_SIGTOKEN_REGNUM/REGISTER. That register (r11) carries the VASigCookie* into VarargPInvokeStub; it outlived the unmanaged-calli helpers removed in the parent layer, so the old name referred to a caller that no longer exists.

Note for reviewers touching asmconstants.h

h2inc runs at CMake configure time and asmconstants.h is not registered as a configure dependency, so editing a constant's name or value does not regenerate AsmConstants.inc on an incremental build — the assembler keeps consuming the stale copy. The rename above surfaced this as error A2006: undefined symbol. Pre-existing infrastructure behavior, not changed here; a reconfigure picks it up. Worth knowing because a silently stale value would be far less obvious than a stale name.

Validation

Target	Result
windows-x64 clr+libs -rc Checked	builds clean
windows-x86	builds clean (exercises i386/asmhelpers.asm + asmconstants)
windows-arm64	builds clean (exercises arm64/PInvokeStubs.asm + asmconstants)
windows-x64, FEATURE_VARARGS forced off on both sides	builds clean, including the DAC
NativeAOT (clr.nativeaotlibs)	builds clean, picks up the shared file
linux/osx x64, linux arm32/arm64, loongarch64, riscv64, browser-wasm	not built locally — covered by inspection plus the forced-off proxy

Tests (windows-x64 Checked):

• JIT/Directed/arglist/vararg_TargetWindows — 241/241 passed
• Interop — 333 total, 321 passed, 10 failed; the 10 are the known pre-existing out-of-proc COM and DisabledRuntimeMarshalling environment failures, unchanged from baseline. VarargsTest, CrossAssemblyVarargsTest and IJW NativeVarargsTest all pass.
• JIT/Directed — 686 total, 679 passed, 0 failed, 7 skipped

Feature-off behavior was verified by inspecting build output rather than assuming: the contract descriptor drops VASigCookie (while MethodTable remains), and CoreLib drops ArgIterator_Init/CalcVaListSize while gaining PlatformNotSupported_ArgIterator.

Note

This pull request description was generated by GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
15 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[jkotas]

This looks good to me. I do not understand the point of it being part of a stacked PR. Can this change be merged once the CI is greeen?

[dotnet-policy-service]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[jkoritzinsky]

This is stacked because it depends on unmanaged calli moving away from using VASigCookie on all platforms. Right now VASigCookie is used by varargs and calli.

[jkotas]

Ah ok, part of it depends on the change.

[MichalPetryka]

Does this mean that #48796 won't happen?

[Copilot]

Pull request overview

This PR introduces a FEATURE_VARARGS build feature and uses it to compile vararg-related infrastructure only on platforms where vararg calling convention support is reachable (primarily Windows). The change centralizes “varargs supported” logic behind a single feature define and removes or gates otherwise-dead VM/JIT interop stubs, cookie machinery, marshalling helpers, and managed surface area on unsupported targets.

Changes:

• Add FEATURE_VARARGS (native CMake + managed MSBuild) and use it to gate VM/JIT vararg support, including ConvToJitSig rejection logic.
• Rework CoreLib’s ArgIterator inclusion: real implementation only when varargs are supported; otherwise use a shared PlatformNotSupported implementation (also used by NativeAOT).
• Remove unreachable non-Windows vararg P/Invoke stub implementations / placeholders and gate related assembly constants, stub managers, QCalls, and cDAC descriptors.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Private.CoreLib/src/System/ArgIterator.PlatformNotSupported.cs	Remove unused using; keep PNSE ArgIterator implementation.
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems	Include PNSE ArgIterator only when varargs are off and not Mono.
src/coreclr/vm/wasm/helpers.cpp	Remove wasm VarargPInvokeStub* PORTABILITY_ASSERT placeholders.
src/coreclr/vm/stubmgr.h	Gate InteropDispatchStubManager declaration behind FEATURE_VARARGS.
src/coreclr/vm/stubmgr.cpp	Gate vararg stub recognition/tracing + DAC enumeration behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.h	Gate vararg-related FCALL/QCALL declarations behind FEATURE_VARARGS.
src/coreclr/vm/stubhelpers.cpp	Gate vararg-related FCALL/QCALL implementations behind FEATURE_VARARGS.
src/coreclr/vm/riscv64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/riscv64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/qcallentrypoints.cpp	Gate ArgIterator_* and StubHelpers_*VaList QCall entries behind FEATURE_VARARGS.
src/coreclr/vm/prestub.cpp	Gate TheVarargPInvokeStub behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/mtypes.h	Gate MARSHAL_TYPE_ARGITERATOR behind FEATURE_VARARGS.
src/coreclr/vm/mlinfo.cpp	Gate ArgIterator marshaler detection behind FEATURE_VARARGS.
src/coreclr/vm/metasig.h	Gate vararg metasig entry behind FEATURE_VARARGS.
src/coreclr/vm/loongarch64/pinvokestubs.S	Remove unreachable vararg P/Invoke stub generator macro and stubs.
src/coreclr/vm/loongarch64/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/jitinterface.cpp	Use FEATURE_VARARGS for vararg rejection; gate vararg class/cookie paths with asserts when off.
src/coreclr/vm/ilmarshalers.h	Gate ILArgIteratorMarshaler definition behind FEATURE_VARARGS.
src/coreclr/vm/ilmarshalers.cpp	Gate ILArgIteratorMarshaler implementation behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmhelpers.S	Remove unreachable non-Windows x86 vararg stub implementation.
src/coreclr/vm/i386/asmhelpers.asm	Gate Windows x86 vararg stub + worker extern behind FEATURE_VARARGS.
src/coreclr/vm/i386/asmconstants.h	Gate VASigCookie stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/gc_unwind_x86.inl	Assert/PORTABILITY_ASSERT varargs GC unwind when varargs feature is off.
src/coreclr/vm/frames.h	Gate TransitionFrame::GetVASigCookie declaration behind FEATURE_VARARGS.
src/coreclr/vm/frames.cpp	Gate GetVASigCookie; restructure caller stack promotion and GC refmap handling for varargs.
src/coreclr/vm/eetwain.cpp	Gate promoteVarArgs + varargs GC reporting; assert unreachable when feature is off.
src/coreclr/vm/ecalllist.h	Gate StubHelpers::CalcVaListSize FCALL exposure behind FEATURE_VARARGS.
src/coreclr/vm/dllimport.cpp	Gate vararg P/Invoke stub building and VarargPInvokeStubWorker; throw IDS_EE_VARARG_NOT_SUPPORTED when feature off.
src/coreclr/vm/datadescriptor/datadescriptor.inc	Gate cDAC VASigCookie type descriptor behind FEATURE_VARARGS.
src/coreclr/vm/corelib.h	Gate CoreLib binder entries for ArgIterator/RuntimeArgumentHandle and va_list StubHelpers behind FEATURE_VARARGS.
src/coreclr/vm/CMakeLists.txt	Only compile clrvarargs.* / varargsnative.* sources when FEATURE_VARARGS is enabled.
src/coreclr/vm/class.h	Gate TheVarargPInvokeStub declaration behind FEATURE_VARARGS.
src/coreclr/vm/cgensys.h	Gate vararg stub worker/stub exports behind FEATURE_VARARGS; simplify ret-buff arch conditions.
src/coreclr/vm/ceeload.h	Gate VASigCookie / cookie block storage and APIs behind FEATURE_VARARGS.
src/coreclr/vm/ceeload.cpp	Gate Module::GetVASigCookie*; gate clrvarargs.h include for asmconstant asserts.
src/coreclr/vm/arm64/pinvokestubs.S	Remove unreachable unix arm64 vararg stub generator/stub.
src/coreclr/vm/arm64/PInvokeStubs.asm	Gate Windows arm64 vararg stub machinery behind FEATURE_VARARGS.
src/coreclr/vm/arm64/asmconstants.h	Gate VASigCookie IL stub offset constant behind FEATURE_VARARGS.
src/coreclr/vm/arm/pinvokestubs.S	Remove unreachable unix arm32 vararg stubs and generator macro.
src/coreclr/vm/arm/asmconstants.h	Remove VASigCookie IL stub offset constant on unsupported target.
src/coreclr/vm/appdomain.cpp	Only initialize InteropDispatchStubManager when FEATURE_VARARGS is enabled.
src/coreclr/vm/amd64/pinvokestubs.S	Remove unreachable unix amd64 vararg stubs.
src/coreclr/vm/amd64/PInvokeStubs.asm	Gate Windows amd64 vararg stubs behind FEATURE_VARARGS; rename sigtoken register macro.
src/coreclr/vm/amd64/asmconstants.h	Gate vararg sigtoken register and VASigCookie stub offset constants behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj	Compile ArgIterator.cs only when FeatureVarargs is true.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs	Gate va_list marshalling imports and CalcVaListSize behind FEATURE_VARARGS.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs	Remove platform preprocessor split; keep only the real implementation (now conditionally compiled by csproj).
src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj	Stop compiling NativeAOT’s old System\\ArgIterator.cs (use shared PNSE implementation instead).
src/coreclr/inc/vptr_list.h	Gate InteropDispatchStubManager VPTR entry behind FEATURE_VARARGS.
src/coreclr/debug/daccess/dacdbiimpl.cpp	Return E_NOTIMPL for GetVarArgSig when varargs are off (implementation gated).
src/coreclr/clrdefinitions.cmake	Define FEATURE_VARARGS for Windows builds (native define).
src/coreclr/clr.featuredefines.props	Define FeatureVarargs for Windows builds and propagate FEATURE_VARARGS to managed DefineConstants.

[jkoritzinsky]

Does this mean that #48796 won't happen?

That will likely use a different mechanism than CLR-native varargs. Either that or we'd enable FEATURE_VARARGS and do the work, but I don't think we'd implement it as such as managed varargs is not something we want to do.

[MichalPetryka]

managed varargs is not something we want to do

Yeah I agree that those are not needed.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/coreclr/vm/jitinterface.cpp:6268

• Similar to the CLASSID_ARGUMENT_HANDLE case: in retail builds this will just return NULL (and leave *ppIndirection NULL) if called, which risks later crashes rather than a clear "varargs not supported" failure.

Since this is a THROWS path already, it’s safer to throw IDS_EE_VARARG_NOT_SUPPORTED here when FEATURE_VARARGS is off.
src/coreclr/clrdefinitions.cmake:126

• FEATURE_VARARGS is defined for all Win32 targets here. That’s broader than the actual varargs implementation (e.g., the old ConvToJitSig guard also excluded TARGET_ARM), and can accidentally enable varargs on Windows targets/arches that don’t have the supporting stubs/GC/reporting.

Consider scoping FEATURE_VARARGS to the same set of Windows architectures that actually support varargs (x86/x64/arm64), similar to FEATURE_INTEROP_DEBUGGING just above.

if (CLR_CMAKE_TARGET_WIN32)
set(FEATURE_VARARGS 1)
add_definitions(-DFEATURE_VARARGS)
endif (CLR_CMAKE_TARGET_WIN32)

src/coreclr/clr.featuredefines.props:54

• FeatureVarargs is enabled for all Windows builds, but the implementation is only expected to exist on specific Windows architectures (x86/x64/arm64). Keeping this scoped avoids accidentally defining FEATURE_VARARGS in managed builds where the native support isn’t present, and keeps the managed/native definitions truly “in sync” as the comment suggests.

 <!-- The vararg calling convention is only supported on Windows.
Must stay in sync with FEATURE_VARARGS in clrdefinitions.cmake. -->
<FeatureVarargs>true</FeatureVarargs>
</PropertyGroup>

src/coreclr/vm/jitinterface.cpp:4098

• In retail builds (where _ASSERTE is compiled out), this path returns a null CORINFO_CLASS_HANDLE for CLASSID_ARGUMENT_HANDLE. If the JIT ever does request it (e.g., due to unexpected IL), that would likely turn into a harder-to-diagnose null deref later.

Since ConvToJitSig uses IDS_EE_VARARG_NOT_SUPPORTED for unsupported varargs, consider throwing the same exception here as a defensive fail-fast.

This issue also appears on line 6264 of the same file.

[Copilot]

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/debug/daccess/dacdbiimpl.cpp:5529

• In the !FEATURE_VARARGS path, this method unconditionally dereferences pArgBase and pRetVal. Even if the call should be unreachable on non-varargs targets, this is a public DAC/DBI entrypoint and should not AV when passed null out-params. Please validate pointers (return E_INVALIDARG) before writing outputs, then return E_NOTIMPL.

#ifndef FEATURE_VARARGS
*pArgBase = (CORDB_ADDRESS)NULL;
*pRetVal = TargetBuffer();
return E_NOTIMPL;
#else // FEATURE_VARARGS

[jkotas]

Thanks!

[jkoritzinsky]

/ba-g build failures correspond to the known helix test failures.