Uh oh!
There was an error while loading. Please reload this page.
Use hidden arguments for NativeAOT thunks - #133222
Conversation
Replace the TLS-based thunk context handoff with SecretStubArgument across supported architectures and remove the obsolete common-stub helpers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
🟡 Changes recommended
The generated AMD64 and x86 thunk encodings use misaligned typed stores, introducing C++ undefined behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Replaces NativeAOT’s TLS-based thunk context handoff with the hidden SecretStubArgument, building on #132534.
Changes:
- Adds hidden context parameters to delegate and interface thunks.
- Emits direct architecture-specific context loads and removes common-stub/TLS helpers.
- Preserves the x86 hidden argument across stack probing.
File summaries
| File | Description |
|---|---|
src/coreclr/tools/Common/TypeSystem/IL/Stubs/PInvokeILEmitter.cs | Passes hidden context to delegate helpers. |
src/coreclr/tools/Common/TypeSystem/IL/Stubs/ILEmitter.cs | Adds a hidden-argument signature helper. |
src/coreclr/tools/Common/TypeSystem/IL/Stubs/DelegateMarshallingMethodThunk.cs | Extends reverse delegate signatures. |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.InterfaceThunks.cs | Uses hidden context for interface thunks. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/ThunkPool.cs | Removes common-stub configuration. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs | Removes obsolete runtime imports. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs | Accepts explicit thunk context. |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/IDynamicInterfaceCastable.cs | Simplifies thunk-pool creation. |
src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SharedCodeHelpers.cs | Removes TLS context retrieval. |
src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs | Forwards explicit context arguments. |
src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/Augments/RuntimeAugments.cs | Simplifies thunk-heap API. |
src/coreclr/nativeaot/Runtime/unix/unixasmmacrosarm64.inc | Removes obsolete TLS helper macro. |
src/coreclr/nativeaot/Runtime/unix/PalUnix.cpp | Removes thunk TLS storage and exports. |
src/coreclr/nativeaot/Runtime/ThunksMapping.cpp | Generates direct context/target thunk code. |
src/coreclr/nativeaot/Runtime/riscv64/InteropThunksHelpers.S | Removes the RISC-V common stub. |
src/coreclr/nativeaot/Runtime/portable.cpp | Removes obsolete portable stubs. |
src/coreclr/nativeaot/Runtime/loongarch64/InteropThunksHelpers.S | Removes the LoongArch common stub. |
src/coreclr/nativeaot/Runtime/i386/InteropThunksHelpers.asm | Removes the Windows x86 common stub. |
src/coreclr/nativeaot/Runtime/CMakeLists.txt | Stops compiling common-stub assembly. |
src/coreclr/nativeaot/Runtime/arm64/ThunkPoolThunks.S | Loads context directly on Unix ARM64. |
src/coreclr/nativeaot/Runtime/arm64/ThunkPoolThunks.asm | Loads context directly on Windows ARM64. |
src/coreclr/nativeaot/Runtime/arm64/InteropThunksHelpers.S | Removes the Unix ARM64 common stub. |
src/coreclr/nativeaot/Runtime/arm64/InteropThunksHelpers.asm | Removes the Windows ARM64 common stub. |
src/coreclr/nativeaot/Runtime/arm/InteropThunksHelpers.S | Removes the ARM common stub. |
src/coreclr/nativeaot/Runtime/amd64/ThunkPoolThunks.S | Loads context directly on Unix AMD64. |
src/coreclr/nativeaot/Runtime/amd64/ThunkPoolThunks.asm | Loads context directly on Windows AMD64. |
src/coreclr/nativeaot/Runtime/amd64/InteropThunksHelpers.S | Removes the Unix AMD64 common stub. |
src/coreclr/nativeaot/Runtime/amd64/InteropThunksHelpers.asm | Removes the Windows AMD64 common stub. |
src/coreclr/jit/codegenxarch.cpp | Preserves the x86 hidden argument during stack probes. |
Review details
Suppressed comments (1)
src/coreclr/nativeaot/Runtime/ThunksMapping.cpp:182
- This target-address field starts at byte offset 7, so writing through
void**is an unaligned typed store and undefined behavior in C++. Use an unaligned-safe write for both absolute-address fields in this x86 thunk.
*((void **)pCurrentThunkAddress) = pCurrentDataAddress + POINTER_SIZE;
- Files reviewed: 29/29 changed files
- Comments generated: 1
- Review effort level: Balanced
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Replace the TLS-based thunk context handoff with SecretStubArgument across supported architectures and remove the common-stub helpers.
RyuJIT must support the special hidden arguments for regular CoreCLR with JIT, so we can take advantage of it in NativeAOT to reduce amount of arch-specific asm code.
Depends on #132534