Skip to content

Fix diagnostic for calls on marshalled delegates #117073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jun 27, 2025

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jun 27, 2025

  • Fix diagnostic for calls on marshalled delegates. It got broken by switch to precode thunks.
  • Add test for diagnostic for calls on marshalled delegates.
  • Pass UMEntryThunkData* around in the delegate marshaling code instead of UMEntryThunk*. This avoids constant back and forth hops between the two.

@jkotas jkotas requested review from Copilot and davidwrighton and removed request for Copilot June 27, 2025 00:58
Copy link
Contributor

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

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 restores and enhances diagnostics for calls on marshalled delegates by switching from UMEntryThunk* to UMEntryThunkData* and adding a test to verify the behavior.

  • Added an unhandled exception check for collected delegates and a new test project/source (collecteddelegate)
  • Refactored runtime delegate marshaling code to pass and operate on UMEntryThunkData* instead of UMEntryThunk*
  • Introduced CallbackOnCollectedDelegate to emit a fail-fast message when invoking a collected delegate

Reviewed Changes

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

Show a summary per file
File Description
src/tests/baseservices/exceptions/unhandled/unhandledTester.cs Assert the correct diagnostic message for a collected delegate scenario
src/tests/baseservices/exceptions/unhandled/collecteddelegate.csproj & .cs New test project and source triggering a callback on a collected delegate
src/coreclr/vm/dllimportcallback.h / dllimportcallback.cpp Updated APIs to use UMEntryThunkData*, added CallbackOnCollectedDelegate
src/coreclr/vm/syncblk.h / syncblk.cpp Changed member types and getters to UMEntryThunkData*
src/coreclr/vm/comdelegate.cpp Adjusted COM delegate marshaling to the new UMEntryThunkData interface
src/coreclr/vm/stubmgr.cpp Updated reverse P/Invoke tracing to accept UMEntryThunkData*
Comments suppressed due to low confidence (2)

src/coreclr/vm/dllimportcallback.cpp:230

  • Missing semicolon after CallbackOnCollectedDelegate(pUMEntryThunkData), causing a compile error.
        CallbackOnCollectedDelegate(pUMEntryThunkData)

src/coreclr/vm/comdelegate.cpp:1435

  • pUMEntryThunk is now a UMEntryThunkData* and does not have a GetData() method; this call will not compile. Consider adding a GetObjectHandle() method to UMEntryThunkData or calling the correct accessor directly on pUMEntryThunk.
        ? pUMEntryThunk->GetData()->GetObjectHandle()

@jkotas jkotas merged commit f0b4af7 into dotnet:main Jun 27, 2025
103 of 105 checks passed
@jkotas jkotas deleted the UMEntryThunkData branch June 27, 2025 20:18
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.

2 participants