Skip to content

Disable ctor calls on classes marked with ComImport when COM interop feature isn't enabled. #115009

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 6 commits into from
Apr 26, 2025

Conversation

AaronRobinsonMSFT
Copy link
Member

Fixes #114933

@AaronRobinsonMSFT
Copy link
Member Author

/cc @dotnet/interop-contrib

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 disables COM interop constructor calls in builds that do not support COM interop by wrapping related code in FEATURE_COMINTEROP conditionals.

  • Wraps COM interop-specific logic in a preprocessor conditional
  • Removes inline conditional checks inside the COM interop block
Comments suppressed due to low confidence (1)

src/coreclr/vm/ecall.cpp:339

  • It would be beneficial to add or update unit tests to verify that COM interop constructor calls are properly disabled in builds without COM interop support.
#ifdef FEATURE_COMINTEROP

Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Apr 24, 2025

SecurityException: ECall methods must be packaged into a system module.

This is confusing error to give when somebody tries to use built-in COM on non-Windows. Can we throw IDS_EE_ERROR_COM instead?

@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Disable COM interop ctor calls when COM interop feature isn't supported. Disable ctor calls on classes marked with ComImport when COM interop feature isn't enabled. Apr 24, 2025
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Is it worth it to add a regression test for PrepareMethod?

@AaronRobinsonMSFT
Copy link
Member Author

Is it worth it to add a regression test for PrepareMethod?

Yeah, I thought about it. I'm not sure how useful it is in practice, but I added one none the less.

…/Runtime/CompilerServices/RuntimeHelpersTests.cs

Co-authored-by: Jan Kotas <[email protected]>
@AaronRobinsonMSFT
Copy link
Member Author

/ba-g WASM reliability

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 68a1b4a into dotnet:main Apr 26, 2025
125 of 127 checks passed
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the runtime_114933 branch April 26, 2025 10:37
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: pMD->IsCtor()
2 participants