Skip to content

Ensure we do not try to inline async versions of pinvokes#129797

Open
jakobbotsch wants to merge 2 commits into
dotnet:mainfrom
jakobbotsch:fix-129782
Open

Ensure we do not try to inline async versions of pinvokes#129797
jakobbotsch wants to merge 2 commits into
dotnet:mainfrom
jakobbotsch:fix-129782

Conversation

@jakobbotsch

Copy link
Copy Markdown
Member

We normally fail inlining of pinvokes when we call getMethodInfo, but if we tried inlining an async variant of a pinvoke then that call would succeed and we would end up breaking in getEHinfo that does not handle this case.

Fix #129782

We normally fail inlining of pinvokes when we call getMethodInfo, but if
we tried inlining an async variant of a pinvoke then that call would
succeed and we would end up breaking in getEHinfo that does not handle
this case.
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Comment thread src/coreclr/vm/jitinterface.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the CoreCLR JIT/EE interface to consistently use the ordinary (non-async-variant) MethodDesc as the source of IL/EH metadata when a method participates in async-variant codegen, preventing the JIT from treating async variants of P/Invokes as inlineable IL methods and later asserting when EH info is requested.

Changes:

  • Add MethodDesc::GetOrdinaryVariantIfAsyncVersion() to normalize async-variant methods to their ordinary variant where appropriate.
  • Extend CEEInfo::getMethodInfoWorker to take an explicit ilFtn (the method whose IL/header should be used) and update call sites accordingly.
  • Update getMethodInfo, a canInline EH-count check path, and CEECodeGenInfo::getEHinfo to use the ordinary variant for IL/EH sourcing.

Reviewed changes

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

File Description
src/coreclr/vm/method.hpp Adds a helper to map async-variant methods to their ordinary variant for IL/EH sourcing.
src/coreclr/vm/jitinterface.h Updates getMethodInfoWorker signature to accept an explicit IL-source MethodDesc.
src/coreclr/vm/jitinterface.cpp Threads ilFtn through method-info/EH-info paths to avoid inlining/metadata mismatches for async P/Invoke variants.

Comment thread src/coreclr/vm/jitinterface.cpp
result = true;
}
else if (ftn->IsIL() || ftn->IsDynamicMethod())
else if (ilFtn->IsIL() || ilFtn->IsDynamicMethod())

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The change to use ilFtn in this check is the actual fix for the problem, the rest is just avoiding multiple GetOrdinaryVariant calls.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assert failure: !"No IL header; cannot get EH info for function"

2 participants