Skip to content

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Aug 23, 2025

GCStress is not able to find the original version of the code for PInvoke stubs after #117901. We need to compensate for the MethodDesc adjustment done for PInvoke stubs when storing the original version of the code during GC stress.

@Copilot Copilot AI review requested due to automatic review settings August 23, 2025 03:00
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a GCStress regression where the original version of PInvoke stub code could not be found after changes made in PR #117901. The fix compensates for the MethodDesc adjustment done for PInvoke stubs during code allocation by ensuring GC instrumentation uses the correct code version that matches the CodeHeader.

  • Adds logic to detect when a PInvoke stub's MethodDesc has been adjusted during code allocation
  • Retrieves the actual MethodDesc from the CodeHeader and creates the appropriate NativeCodeVersion
  • Ensures GC coverage setup uses the correct code version for proper instrumentation

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 23, 2025
@jkotas
Copy link
Member Author

jkotas commented Aug 23, 2025

Context #118879 (comment)

@jkotas
Copy link
Member Author

jkotas commented Aug 23, 2025

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jkotas
Copy link
Member Author

jkotas commented Aug 23, 2025

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

@jkotas jkotas added area-VM-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 23, 2025
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

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

jkotas added 2 commits August 23, 2025 01:21
GCStress is not able to find the original version of the code for PInvoke stubs after dotnet#117901. We need to compensate
for the MethodDesc adjustment done for PInvoke stubs when storing the original version of the code during GC stress.
Interop marshalling of varargs needs MethodDesc calling convention to support ldftn <PInvoke method with varargs>. It is not possible to smugle the target MethodDesc* via vararg cookie in this case.
@jkotas
Copy link
Member Author

jkotas commented Aug 23, 2025

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@VSadov
Copy link
Member

VSadov commented Aug 24, 2025

I have combined this PR with the change that makes GC stress faster in #119037 - to rule out timeouts.

It looks like the combined change passes stress tests, except on linux-arm.
Not sure if we have some other change causing that, or this fix misses something on arm32.

(There is also one failure in tailcall_v4/hijacking, which I think is unrelated, since neither interop, nor finalization is involved)

@jkotas
Copy link
Member Author

jkotas commented Aug 24, 2025

this fix misses something on arm32

Yes, there was a bug on arm32. It should be fixed by last commit.

@jkotas
Copy link
Member Author

jkotas commented Aug 24, 2025

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jkotas
Copy link
Member Author

jkotas commented Aug 24, 2025

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jkotas
Copy link
Member Author

jkotas commented Aug 24, 2025

/azp run runtime-coreclr gcstress-extra, runtime-coreclr gcstress0x3-gcstress0xc

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@VSadov
Copy link
Member

VSadov commented Aug 24, 2025

stress is passing on linux-arm 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants