Migrate CLR->COM stubs to transient IL - #131494
Draft
jkoritzinsky wants to merge 7 commits into
Draft
Conversation
Following the pattern used for P/Invokes in #126509 and #127400, CLR->COM calls no longer get their own IL stub DynamicMethodDesc. Instead the marshalling IL is generated as transient IL for the CLRToCOMCallMethodDesc itself and jitted onto that method. CLRToCOMCall::CreateCLRToCOMCallMethodIL is hooked into MethodDesc::TryGenerateTransientILImplementation, and DoPrestub now routes CLR->COM methods through PrepareInitialCode. The only remaining special case is a user-supplied predefined IL stub (ManagedToNativeComInteropStubAttribute), which is handled up front by CLRToCOMCall::GetPredefinedILStubMethod. Because the IL is compiled as the CLR->COM method rather than as a shared stub, there is no JIT secret parameter to carry the stub context. PInvokeStubLinker now bakes the MethodDesc pointer into the IL as a constant for forward COM stubs. This is safe because the transient IL's resolver reports the CLR->COM method as its owner, so the jitted code cannot outlive the MethodDesc. CLR->COM MethodDescs always require a stable entry point, and MethodDesc::SetNativeCodeInterlocked needs a native code slot for such methods, so MethodTableBuilder::NeedsNativeCodeSlot now allocates one for mcComInterop. Note that without FEATURE_COMINTEROP these same methods are classified as mcIL and already get a native code slot from the tiered compilation check, so this brings Windows in line with the other platforms. Since CLR->COM calls are no longer IL stubs, add a CLRToCOMStubManager (mirroring the PInvokeStubManager added in #123411) so mixed-mode debugging can still step through to the COM target or, for COM event calls, to the managed event provider. MethodDesc::IsInteropStub() and DebuggerWalkStackProc are updated accordingly, and the now-unreachable CLR->COM branch is removed from ILStubManager::TraceManager. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Forward CLR->COM stubs are now generated as transient IL on the CLRToCOMCallMethodDesc itself, so the JIT no longer publishes a secret parameter for them. The previous commit worked around this with a generic PInvokeStubLinker::m_pStubContextMD hook that made EmitLoadStubContext bake the MethodDesc pointer as a constant. Instead of baking the MethodDesc and re-deriving everything from it at runtime, compute the data each helper actually needs at IL generation time and bake that: - The HRESULT-swapping error path only needs the interface MethodTable, so bake it and pass it straight to StubHelpers.GetCOMHRExceptionObject. The StubHelpers.GetComInterfaceFromMethodDesc FCall is no longer needed and is removed. - StubHelpers.GetCOMIPFromRCW only needs CLRToCOMCallInfo::m_pInterfaceMT and m_cachedComSlot, so bake both. The native fast-path FCall and the slow-path QCall now take (MethodTable*, INT32) and no longer touch CLRToCOMCallInfo, which also removes a level of indirection from the CLR->COM fast path. - The profiler transition callback and the pinned-object heap validation helpers genuinely need the MethodDesc, so thread the stub MethodDesc that FinishEmit already has down into EmitProfilerBeginTransitionCallback, EmitObjectValidation and EmitValidateLocal, mirroring the P/Invoke branch. With no COM caller left, PInvokeStubLinker::SetStubContextMD and m_pStubContextMD are removed and EmitLoadStubContext goes back to just reading the secret argument. It also gains a CONSISTENCY_CHECK rejecting forward COM stubs, so a future change that tries to read the secret argument from one fails loudly instead of silently reading garbage. Drive-by fix: corelib.h declared StubHelpers.ValidateObject and StubHelpers.ValidateByref with three-argument metasigs while the managed methods take two arguments. This is a pre-existing bug on main that asserts "EE expects method to exist" in CoreLibBinder::LookupMethodLocal for any P/Invoke with a pinned local under DOTNET_InteropValidatePinnedObjects=1. It is fixed here because it blocked validating the EmitValidateLocal changes above. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Emit the interface type with ldtoken instead of baking the raw MethodTable* into the IL, and change StubHelpers.GetCOMHRExceptionObject to take a RuntimeTypeHandle. The stub's token map keeps the type reachable, so the helper no longer has to reconstitute a RuntimeType from an unmanaged pointer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Forward CLR->COM calls are compiled as transient IL on the CLRToCOMCallMethodDesc itself, so StubCLRToCOMInterop and StubCLRToCOMEvent can no longer be set on any DynamicMethodDesc. Remove the VM code that produced and consumed them and comment out the enum values. The values are left reserved rather than reused because the cDAC still interprets them when reading older runtimes, so the cDAC contract and its documentation are intentionally unchanged. Dropping StubCLRToCOMInterop from IsInteropStub() is a no-op because that helper already short-circuits on IsCLRToCOMCall(), and reducing HasMDContextArg() to IsPInvokeVarArgStub() makes the existing _ASSERTE(pMD->IsPInvoke()) in ILStubManager::TraceManager provably correct. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Every abstract instance method on an interface is a CLRToCOMCallMethodDesc, so a plain managed class that claims to implement a [ComImport] interface must not pick the interface method up as an implementation - only RCWs dispatch through those. Assert that such a type fails to load with the same TypeLoadException as any other type with an unimplemented interface method, using a non-COM interface as a control. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Only one condition is left after CLR->COM calls stopped requiring a MethodDesc context argument, so return the predicate directly instead of branching on it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/interop-contrib |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates CoreCLR’s CLR→COM call stubs from per-method ILStub DynamicMethodDesc + “secret MethodDesc argument” dispatch to transient IL generated directly on the CLRToCOMCallMethodDesc, aligning the CLR→COM pipeline with the transient-IL approach used for P/Invokes. It also updates debugger/profiler integration to recognize CLR→COM calls without relying on ILStub MethodDescs, and adds a regression test to guard type-load behavior for [ComImport] interface methods.
Changes:
- Generate CLR→COM marshalling IL as transient IL on the target
CLRToCOMCallMethodDesc(and route CLR→COM through the normal prestub/PrepareInitialCode path), keeping predefined IDispatch stubs as a special case. - Remove CLR→COM ILStub MD “secret argument” dependencies by baking needed constants/tokens into generated IL and updating helper signatures accordingly.
- Add
CLRToCOMStubManagerfor debugger stepping/tracing of CLR→COM calls, and add a new interop regression test.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/Interop/COM/ComImportUnimplementedMethod/Program.cs | New xUnit test asserting [ComImport] interface methods aren’t treated as managed implementations on ordinary classes. |
| src/tests/Interop/COM/ComImportUnimplementedMethod/ComImportUnimplementedMethod.csproj | New test project (process isolation; NativeAOT-incompatible). |
| src/tests/Interop/COM/ComImportUnimplementedMethod/ComImportTypes.ilproj | IL project to define invalid-by-C#-compiler types needed for the regression scenario. |
| src/tests/Interop/COM/ComImportUnimplementedMethod/ComImportTypes.il | Defines [ComImport] and non-COM interfaces + “missing implementation” classes in IL. |
| src/coreclr/vm/stubmgr.h | Declares new CLRToCOMStubManager for debugger stub tracing of CLR→COM calls. |
| src/coreclr/vm/stubmgr.cpp | Removes CLR→COM handling from ILStubManager and implements CLRToCOMStubManager. |
| src/coreclr/vm/stubhelpers.h | Updates COM helper signatures to accept MethodTable* + COM slot instead of MethodDesc*. |
| src/coreclr/vm/stubhelpers.cpp | Implements updated COM helper fast/slow paths using MethodTable* + slot constants. |
| src/coreclr/vm/prestub.cpp | Enables transient-IL generation for CLR→COM and routes CLR→COM through PrepareInitialCode unless a predefined stub exists. |
| src/coreclr/vm/methodtablebuilder.cpp | Ensures mcComInterop methods get a native code slot (transient IL is jitted onto the method). |
| src/coreclr/vm/method.inl | Treats CLR→COM calls as interop stubs and removes retired CLR→COM ILStubType handling. |
| src/coreclr/vm/method.hpp | Retires (reserves) CLR→COM ILStubType values; removes CLR→COM IL stub fields from call info. |
| src/coreclr/vm/method.cpp | Updates MayHaveNativeCode, tiering eligibility, and removes CLR→COM from MD-context-arg logic. |
| src/coreclr/vm/metasig.h | Updates metasig definitions to match adjusted StubHelpers signatures. |
| src/coreclr/vm/jitinterface.cpp | Excludes CLR→COM calls from profiler ELT enter/leave flags (transition callbacks instead). |
| src/coreclr/vm/ilstubcache.cpp | Ensures COM ILStubCache only produces reverse COM stubs; forward CLR→COM no longer creates ILStub MDs. |
| src/coreclr/vm/ecalllist.h | Removes GetComInterfaceFromMethodDesc FCALL export (no longer needed). |
| src/coreclr/vm/dllimport.h | Adds CLR→COM marshalling IL generation entrypoint and updates stub linker helper signatures. |
| src/coreclr/vm/dllimport.cpp | Generates forward CLR→COM marshalling IL as transient IL, removes secret-arg dependencies, updates profiling/validation emission. |
| src/coreclr/vm/corelib.h | Fixes binder signatures for ValidateObject/ValidateByref and updates COM helper method signatures. |
| src/coreclr/vm/clrtocomcall.h | Adds transient-IL generation API for CLR→COM and predefined-stub lookup helper. |
| src/coreclr/vm/clrtocomcall.cpp | Implements transient IL generation for CLR→COM calls (including COM event call IL). |
| src/coreclr/vm/appdomain.cpp | Registers CLRToCOMStubManager during domain attach. |
| src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs | Updates managed declarations for COM helpers to match new native signatures; removes GetComInterfaceFromMethodDesc. |
| src/coreclr/inc/vptr_list.h | Adds vptr registration for CLRToCOMStubManager. |
| src/coreclr/debug/ee/frameinfo.cpp | Skips frameless CLR→COM calls in stack walking similarly to inlined P/Invoke stubs. |
COMDelegate::PopulateCLRToCOMCallInfo was the only writer of DelegateEEClass::m_pCLRToCOMCallInfo, and it has no callers. That made the IsEEImpl() branch in CLRToCOMCallInfo::FromMethodDesc unreachable: all four callers pass a CLRToCOMCallMethodDesc. Collapse FromMethodDesc to the CLR->COM case with an assert and delete the unreachable delegate machinery it read from. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
|
@jkoritzinsky Let's move this to draft if this is for .NET 12. I think main is still .NET 11. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #126330, which migrated CLR->COM stubs to IL stubs. This moves them the rest of the way to transient IL, the same technology #126509 (plus the regression fixes in #127400) used for P/Invokes.
Before this change, every
[ComImport]interface method allocated a separate IL stubDynamicMethodDescplus anILStubResolver, and dispatched through it using a secretMethodDesccontext argument. With transient IL the marshalling IL is generated directly onto theCLRToCOMCallMethodDescand discarded after JIT, so there is no secondMethodDesc, no secret argument, and no extra frame.What changed
Transient IL generation.
CLRToCOMCall::CreateCLRToCOMCallMethodILandPInvoke::CreateCLRToCOMMarshallingILbuild IL into a resolver owned by the COM method itself, hooked up throughMethodDesc::TryGenerateTransientILImplementation.DoPrestubnow routes CLR->COM through the normalPrepareInitialCodepath; the late-boundIDispatchpredefined stub is the only remaining special case.mcComInteropgainsMayHaveNativeCodeand a native code slot, and is excluded from tiering and ELT profiling.No secret argument. Because the IL is per-method rather than shared, everything the stub used to read out of the secret
MethodDescis baked in at generation time instead: the HRESULT-swap error path takes the interface type as anldtoken, andGetCOMIPFromRCWtakes the interfaceMethodTable*and COM slot as constants. That letPInvokeStubLinker::m_pStubContextMDand theStubHelpers.GetComInterfaceFromMethodDescFCall go away entirely.Debugger. A new
CLRToCOMStubManager, modelled on thePInvokeStubManageradded in #123411, handles stepping into early-bound, late-bound, and event calls, replacing theILStubManagerpaths that no longer see a stubMethodDesc.DebuggerWalkStackProcskips frameless CLR->COM methods.Cleanup.
DynamicMethodDesc::StubCLRToCOMInteropandStubCLRToCOMEventcan no longer be set on anything, so the VM code that produced and consumed them is removed and the enum values are commented out. The unreachable delegate path inCLRToCOMCallInfo::FromMethodDescis removed as well (see below).Things worth a careful look
ILStubTypevalues are reserved, not reused. The cDAC still interprets value6when reading older runtimes, soRuntimeTypeSystem_1.csanddocs/design/datacontracts/RuntimeTypeSystem.mdare intentionally untouched. As we're near the end of .NET 11, I wanted to make this change such that it can go in as part of .NET 12.corelib.hdeclaredVALIDATE_OBJECTandVALIDATE_BYREFwith 3-argument binder signatures while the managed methods take 2. Any P/Invoke with a pinned local asserted"EE expects method to exist"underDOTNET_InteropValidatePinnedObjects=1. This reproduces onmainand is fixed here because the same file was already being modified.CLRToCOMCallInfo::FromMethodDeschad anIsEEImpl()branch readingDelegateEEClass::m_pCLRToCOMCallInfo. That field's only writer,COMDelegate::PopulateCLRToCOMCallInfo, has no callers (it is already dead onmain), so the branch was unreachable and all four callers pass aCLRToCOMCallMethodDesc. The branch, the field, and the populator are all deleted. Nothing in the DAC, cDAC, or data descriptors referenced theDelegateEEClassfield, and every removed piece was inside#ifdef FEATURE_COMINTEROP.New test
src/tests/Interop/COM/ComImportUnimplementedMethodguards a subtlety that this area makes easy to get wrong: every abstract instance method on an interface is aCLRToCOMCallMethodDesc, so a plain managed class that claims to implement a[ComImport]interface must not silently pick the interface method up as an implementation. Only RCWs should dispatch through those. The test asserts the sameTypeLoadExceptionas a non-COM control case. The runtime already behaved correctly here, so this is purely a regression guard.Validation
src/tests/Interop73/73, includingNETClientEvents,NETClientDispatch,NETClientPrimitives(itsErrorTestscovers the HRESULT-swap path across 10 HRESULTs plus the IID-sensitiveHelpLinkcase), andNETClientPrimitivesInALCfor the collectible-ALC lifetime scenario.src/tests/InteropunderDOTNET_InteropValidatePinnedObjects=1: all passing, verified when the binder signature fix landed.profiler/transitions1/1.src/tests/Loader64/65;PlatformNativeR2Rfails for an unrelated local environment reason.There is no COM profiler-transition test in the repo, so that one branch is compile-verified only.
Note
This pull request description was drafted with GitHub Copilot.