Skip to content

Fix initialization of t_MostRecentUMEntryThunkData in reverse pinvokes #126579

Open
BrzVlad wants to merge 4 commits intodotnet:mainfrom
BrzVlad:fix-clrinterp-reverse-pinvoke
Open

Fix initialization of t_MostRecentUMEntryThunkData in reverse pinvokes #126579
BrzVlad wants to merge 4 commits intodotnet:mainfrom
BrzVlad:fix-clrinterp-reverse-pinvoke

Conversation

@BrzVlad
Copy link
Copy Markdown
Member

@BrzVlad BrzVlad commented Apr 6, 2026

In reverse pinvokes, the interpreter receives the UMEntryThunkData in this TLS variable as a hidden argument. The way this is implemented is that reverse pinvokes always go through the prestub which checks whether the target is interpreter, in which case it writes this TLS variable. This means that first call was going through the prestub twice, first to initialize the thunk data and secondly to set this argument (TheUMEntryPrestubWorker was returning itself so it gets called again).

Following 60f1dc1, we obtain the entryPoint so we end up calling the interpreter precode directly. The problem is that we failed to set the TLS variable, which is what this PR is doing.

This also does minor cleanup by removing dead interpreter code from COM sources and renaming targetIsPrecode to canSkipPreStub, because the first name was misleading (it was false when entryPoint was pointing to interpreter precode).

BrzVlad added 3 commits April 6, 2026 19:25
Interpreter doesn't have cominterop support
After recent change, `TheUMEntryPrestubWorker` returns the interpreter precode rather than the prestub (which was setting this variable in the fast path). The interpreter needs to access the hidden argument from this TLS variable.
targetIsPrecode was misleading because, in interpreter case, it was false but the target was pointing to interpreter precode, and not the prestub.
Copilot AI review requested due to automatic review settings April 6, 2026 16:41
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

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 reverse P/Invoke behavior when targeting the interpreter by ensuring t_MostRecentUMEntryThunkData is populated even when the call path can directly jump to the interpreter precode (skipping the second prestub pass that previously performed the TLS write).

Changes:

  • Update TheUMEntryPrestubWorker to set t_MostRecentUMEntryThunkData when the prestub cannot be skipped (interpreter path), including a rename from targetIsPrecodecanSkipPreStub for clarity.
  • Remove interpreter-specific TLS plumbing from COM call prestub code, which is now dead/unnecessary.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/coreclr/vm/dllimportcallback.cpp Ensures reverse P/Invoke interpreter path initializes the TLS thunk-data used as the hidden argument.
src/coreclr/vm/comcallablewrapper.cpp Removes unused interpreter-specific TLS handling from COM prestub flow.

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.

5 participants