Clean up dynamic method desc native stack arg tracking#126585
Merged
jkoritzinsky merged 7 commits intodotnet:mainfrom Apr 25, 2026
Merged
Clean up dynamic method desc native stack arg tracking#126585jkoritzinsky merged 7 commits intodotnet:mainfrom
jkoritzinsky merged 7 commits intodotnet:mainfrom
Conversation
It's only needed on x86 now and we don't need to track it quite as heavily as we have been.
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR narrows native stack-argument-size tracking for dynamic/IL stubs to x86-only, reducing bookkeeping now that the behavior is only required on that architecture (per follow-up concerns from #126509).
Changes:
- Remove the
FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZEindirection and gate related APIs/fields directly onTARGET_X86. - Restrict
DynamicMethodDesc::Get/SetNativeStackArgSizeandMethodDesc::SizeOfNativeArgStackusage to x86 call paths. - Simplify some interop stub sites by removing now-unneeded native stack arg size propagation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/method.hpp | Gates native stack arg size APIs for MethodDesc/DynamicMethodDesc to x86 only. |
| src/coreclr/vm/method.cpp | Gates MethodDesc::SizeOfNativeArgStack() implementation to x86 only. |
| src/coreclr/vm/dllimport.cpp | Updates IL stub signature swap and stub generation paths to only store/copy native stack sizes on x86. |
| src/coreclr/vm/comtoclrcall.h | Refactors ComCallMethodDesc x86-only stack-pop tracking fields/accessor. |
| src/coreclr/vm/comtoclrcall.cpp | Updates COM-to-CLR stub creation to use x86-only native stack arg size tracking. |
| src/coreclr/vm/clrtocomcall.cpp | Removes explicit native stack arg size seeding for certain COM stubs. |
jkotas
approved these changes
Apr 6, 2026
jkotas
changed the title
Member
Author
|
/azp run runtime,runtime-dev-innerloop,dotnet-linker-tests |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This was referenced Apr 15, 2026
Member
|
The failure looks related Assert failure(PID 832 [0x00000340], Thread: 4520 [0x11a8]): HasMarshalError() || !pStubMD->IsILStub() || pStubMD->AsDynamicMethodDesc()->GetNativeStackArgSize() == m_StackBytes |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…-native-stack-arg-size
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It's only needed on x86 now and we don't need to track it quite as heavily as we have been.
Addresses concerns in #126509 (comment)