Uh oh!
There was an error while loading. Please reload this page.
Use explicit secret arguments in IL stubs - #132534
Conversation
|
Azure Pipelines: Successfully started running 9 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
Updates CoreCLR IL stub plumbing so the “secret” stub context is represented and consumed via an explicit native int parameter annotated with the SecretStubArgument required modifier, removing the older StubHelpers.GetStubContext() / publish-secret-parameter paths. This aligns the VM, JIT, interpreter, SuperPMI tooling, and ABI docs around a single mechanism for identifying and binding the secret stub argument register.
Changes:
- Adds explicit SecretStubArgument handling in stub generation and IL emission, and removes the
StubHelpers.GetStubContext()intrinsic path. - Removes
CORJIT_FLAG_PUBLISH_SECRET_PARAM, replaces it with a new method attribute flag (CORINFO_FLG_HAS_MD_CONTEXT_ARG) and updates JIT/interpreter consumption accordingly. - Updates SuperPMI dumps, JIT interface type definitions, and ABI documentation to reflect the new model.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/stubgen.h | Adds secret-arg placeholder constant, tracking index, and IL emission helper. |
| src/coreclr/vm/stubgen.cpp | Patches placeholder LDARG to the real index during linking; tracks secret-arg index. |
| src/coreclr/vm/jitinterface.cpp | Exposes new method flag for stubs with an MD context argument; recognizes SecretStubArgument modreq. |
| src/coreclr/vm/dllimport.h | Replaces stub-context load helper with signature augmentation helper for secret argument. |
| src/coreclr/vm/dllimport.cpp | Appends SecretStubArgument to signatures where needed and updates stub IL to load it via LDARG. |
| src/coreclr/vm/corelib.h | Removes binding for StubHelpers.GetStubContext. |
| src/coreclr/tools/superpmi/superpmi-shared/spmidumphelper.cpp | Adds dump support for CORINFO_FLG_HAS_MD_CONTEXT_ARG; removes dump of publish-secret JIT flag. |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | Removes CORJIT_FLAG_PUBLISH_SECRET_PARAM stringification. |
| src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs | Adds CORINFO_FLG_HAS_MD_CONTEXT_ARG; removes CORJIT_FLAG_PUBLISH_SECRET_PARAM. |
| src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs | Removes the GetStubContext intrinsic stub. |
| src/coreclr/jit/targetx86.h | Updates comment for REG_SECRET_STUB_PARAM to match explicit SecretStubArgument usage. |
| src/coreclr/jit/targetwasm.h | Same comment update for wasm target. |
| src/coreclr/jit/targetriscv64.h | Same comment update for riscv64 target. |
| src/coreclr/jit/targetloongarch64.h | Same comment update for loongarch64 target. |
| src/coreclr/jit/targetarm64.h | Same comment update for arm64 target. |
| src/coreclr/jit/targetarm.h | Same comment update for arm target. |
| src/coreclr/jit/targetamd64.h | Same comment update for amd64 target. |
| src/coreclr/jit/namedintrinsiclist.h | Removes named intrinsic entry for StubHelpers.GetStubContext. |
| src/coreclr/jit/morph.cpp | Switches frame-pointer GC-info requirement to new “has MD context arg” signal. |
| src/coreclr/jit/lsrabuild.cpp | Removes publish-secret-param live-in handling (now modeled via explicit parameter). |
| src/coreclr/jit/lower.cpp | Uses MD-context-arg + explicit secret arg local to populate InlinedCallFrame secret field. |
| src/coreclr/jit/lclvars.cpp | Tracks SecretStubArgument parameter as a well-known arg and adjusts stack-space logic. |
| src/coreclr/jit/jitee.h | Removes JIT_FLAG_PUBLISH_SECRET_PARAM mapping. |
| src/coreclr/jit/importercalls.cpp | Removes import-time intrinsic expansion for GetStubContext; handles SecretStubArgument modifier on call args. |
| src/coreclr/jit/flowgraph.cpp | Uses presence of explicit secret stub arg for reverse pinvoke tracking and uses MD-context-arg for frame sizing. |
| src/coreclr/jit/compiler.hpp | Stops treating the old stub-arg temp as a special “never zero-init” var. |
| src/coreclr/jit/compiler.h | Adds helpers to query presence of explicit secret stub arg and MD-context-arg flag; removes compPublishStubParam. |
| src/coreclr/jit/compiler.cpp | Removes allocation/initialization of the old “published stub argument” temp. |
| src/coreclr/jit/codegenxarch.cpp | Updates stack-probing path to preserve secret stub register based on explicit-secret-arg presence. |
| src/coreclr/jit/codegenloongarch64.cpp | Updates assertions to key off explicit-secret-arg presence. |
| src/coreclr/jit/codegencommon.cpp | Removes homing/spilling logic tied to the old publish-secret-param mechanism. |
| src/coreclr/jit/codegenarm64.cpp | Updates assertions to key off explicit-secret-arg presence. |
| src/coreclr/jit/codegenarm.cpp | Updates assertions to key off explicit-secret-arg presence. |
| src/coreclr/interpreter/intrinsics.cpp | Removes interpreter named intrinsic lookup for GetStubContext. |
| src/coreclr/interpreter/compiler.cpp | Detects SecretStubArgument modifier on parameters and removes intrinsic-based expansion path. |
| src/coreclr/inc/jiteeversionguid.h | Bumps JIT/EE interface GUID for flag/contract changes. |
| src/coreclr/inc/corjitflags.h | Removes CORJIT_FLAG_PUBLISH_SECRET_PARAM. |
| src/coreclr/inc/corinfo.h | Adds CORINFO_FLG_HAS_MD_CONTEXT_ARG and keeps SecretStubArgument modifier flag. |
| docs/design/coreclr/botr/clr-abi.md | Updates ABI documentation to describe SecretStubArgument as an explicit parameter. |
| *Stub dispatch* - when a virtual call uses a VSD stub, rather than back-patching the calling code (or disassembling it), the JIT must place the address of the stub used to load the call target, the "stub indirection cell", in (x86) `EAX` / (AMD64) `R11` / (ARM) `R12` / (ARM64) `R11`. In the JIT, this is encapsulated in the `VirtualStubParamInfo` class. | ||
| *Normal PInvoke* - The VM shares IL stubs based on signatures, but wants the right method to show up in call stack and exceptions, so the MethodDesc for the exact PInvoke is passed in the (x86) `EAX` / (AMD64) `R10` / (ARM, ARM64) `R12` (in the JIT: `REG_SECRET_STUB_PARAM`). Then in the IL stub, when the JIT gets `CORJIT_FLG_PUBLISH_SECRET_PARAM`, it must move the register into a compiler temp. The value is returned for the intrinsic `NI_System_StubHelpers_GetStubContext`. | ||
| *Normal PInvoke* - The VM shares IL stubs based on signatures, but wants the right method to show up in call stack and exceptions, so the MethodDesc for the exact PInvoke is passed in the (x86) `EAX` / (AMD64) `R10` / (ARM, ARM64) `R12` (in the JIT: `REG_SECRET_STUB_PARAM`). The IL stub represents this value as an explicit `native int` parameter with a required `System.Runtime.CompilerServices.SecretStubArgument` modifier, which tells the JIT to bind the parameter to the secret stub register. | ||
| { | ||
| WRAPPER_NO_CONTRACT; | ||
| if (SF_IsFieldGetterStub(dwStubFlags) || SF_IsFieldSetterStub(dwStubFlags)) |
There was a problem hiding this comment.
Can we make the logic positive and check for stubs that do need the hidden arg - it will make it easier to match the conditions with the emitter. I think it should be just:
if (SF_IsForwardStub(dwStubFlags))
{
if (SF_IsVarArgStub(dwStubFlags))
return true;
}
else
{
// All native-managed stub need the hidden arg currently
return true;
}
| { | ||
| genSpillOrAddNonStandardRegisterParam(m_compiler->lvaStubArgumentVar, REG_SECRET_STUB_PARAM, &graph); | ||
| } | ||
There was a problem hiding this comment.
You can delete genSpillOrAddNonStandardRegisterParam.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/coreclr/jit/flowgraph.cpp:1862
- The comment describing the Reverse P/Invoke helper’s third argument is misleading: the secret stub argument passed here is thunk data (e.g., UMEntryThunkData*) that the helper uses to recover the target MethodDesc, not the MethodDesc itself. Updating the comment would avoid future confusion while working on this code path.
// If we have a secret param for a Reverse P/Invoke, that means that we are in an IL stub.
// In this case, the method handle we pass down to the Reverse P/Invoke helper should be
// the target method, which is passed in the secret parameter.
| if (lclNum == lvaSecretStubArg) | ||
| { | ||
| // The secret stub argument is passed in a nonstandard register without caller-reserved stack space. | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Can you instead switch the Windows ABI case to base the result on lvaGetRelativeOffsetToCallerAllocatedSpaceForParameter?
21f3e07 to
d378425CompareCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: jkoritzinsky <1571408+jkoritzinsky@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
d378425 to
4725c3aCompare
Summary
native int modreq(SecretStubArgument)parameterStubHelpers.GetStubContext()and the obsolete publish-secret-parameter consumption pathsValidation
Note
This pull request description was generated by GitHub Copilot.