Skip to content

Migrate CLR to COM stubs to be IL stubs#126330

Open
jkoritzinsky wants to merge 6 commits intodotnet:mainfrom
jkoritzinsky:clr-to-com
Open

Migrate CLR to COM stubs to be IL stubs#126330
jkoritzinsky wants to merge 6 commits intodotnet:mainfrom
jkoritzinsky:clr-to-com

Conversation

@jkoritzinsky
Copy link
Copy Markdown
Member

Today, CLR->COM eventing and late binding stubs use a generic stub model, like COM->CLR stubs (being changed in #126002 as well). However, the CLR->COM cases have one important difference: they call into managed methods that exist with known signature shapes. Additionally, there's a decent amount of manually managed code written to support calling back into managed, as well as some diagonstics workarounds to avoid getting confused about a Managed-to-Unmanaged transition frame being on the stack in the same frame as managed code.

This PR converts CLR->COM eventing and late binding stubs to be IL stubs like early-bound COM stubs.

Eventing stubs are extremely simple, so they're implemented on top of the basic IL stub infrastructure.

Late binding stubs don't use marshalling in the IL stub itself, but they do depend on some of the marshalling logic. As a result, the late binding stubs do build on top of the interop IL stub infrastructure with some slight modifications.

A number of features were added to the IL stub infrastructure (namely, support for ldstr, newarr, some stelem instructions) to support calling the ForwardToInvokeMember call from an IL stub.

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
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 migrates CLR-to-COM eventing and late-binding stubs away from the legacy “generic stub + CLRToCOMMethodFrame” model to IL stubs, aligning CLR-to-COM behavior with the existing IL stub infrastructure and reducing special-case debugger/stackwalk handling.

Changes:

  • Convert CLR→COM late-bound and eventing paths to IL stubs, removing CLRToCOMMethodFrame and the architecture-specific GenericCLRToCOMCallStub implementations.
  • Extend IL stub generation/resolution to support user strings (ldstr) and additional IL instructions (newarr, stelem.i1, stelem.i4) needed by the new stubs.
  • Simplify debugger/stackwalk logic by removing CLRToCOMMethodFrame-specific stepping/suppression workarounds.

Reviewed changes

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

Show a summary per file
File Description
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StackWalk/FrameHandling/FrameIterator.cs Removes CLRToCOMMethodFrame handling from CDAC stackwalking frame classification.
src/coreclr/vm/stubmgr.h Updates comments to reflect removal of GenericCLRToCOMCallStub recognition.
src/coreclr/vm/stubmgr.cpp Routes CLR→COM trace logic through IL-stub-based path and adds late-bound COM target resolution helper.
src/coreclr/vm/stubgen.h Adds user-string token support and new IL emit helpers (ldstr, newarr, stelem.*).
src/coreclr/vm/stubgen.cpp Implements EmitLDSTR, EmitNEWARR, EmitSTELEM_I1/I4, and user-string token plumbing.
src/coreclr/vm/prestub.cpp Updates comment wording to “CLR-to-COM”.
src/coreclr/vm/method.hpp Extends IL stub type enum and adjusts CLR→COM call info layout to support both IL stub + event provider MD.
src/coreclr/vm/ilstubresolver.h Changes string-literal construction API to return STRINGREF*.
src/coreclr/vm/ilstubresolver.cpp Implements IL-stub string literal support backed by new user-string token map.
src/coreclr/vm/ilstubcache.cpp Tags COM event call stubs with a dedicated IL stub type for debugger behavior.
src/coreclr/vm/i386/asmhelpers.asm Removes x86 GenericCLRToCOMCallStub implementation.
src/coreclr/vm/FrameTypes.h Removes CLRToCOMMethodFrame from frame-type list.
src/coreclr/vm/frames.h Removes CLRToCOMMethodFrame documentation and implementation declarations.
src/coreclr/vm/frames.cpp Removes CLRToCOMMethodFrame implementation and related GC promotion logic.
src/coreclr/vm/dynamicmethod.h Updates DynamicResolver interface to return STRINGREF* for string literals.
src/coreclr/vm/dynamicmethod.cpp Updates LCGMethodResolver string literal construction to return STRINGREF*.
src/coreclr/vm/dllimport.cpp Introduces LateBoundCLRToCOM_ILStubState and refactors IL-stub state abstraction; late-bound stubs now build argument arrays/types and call into managed reflection helper.
src/coreclr/vm/corelib.h Switches COM interop method binder entries to NoSig for direct managed calls (non-UCO).
src/coreclr/vm/CMakeLists.txt Removes obsolete sources/headers and ensures clrtocomcall.* remains correctly included for COM interop builds.
src/coreclr/vm/clrtocomcall.h Removes obsolete debugger helper declarations tied to CLRToCOMMethodFrame.
src/coreclr/vm/clrtocomcall.cpp Generates eventing stubs as IL stubs and unifies CLR→COM stub generation under IL stubs.
src/coreclr/vm/cgensys.h Removes exports for CLRToCOMWorker/GenericCLRToCOMCallStub.
src/coreclr/vm/callsiteinspect.h Deletes callsite inspection helper header (no longer needed).
src/coreclr/vm/callsiteinspect.cpp Deletes callsite inspection helper implementation (no longer needed).
src/coreclr/vm/arm64/asmhelpers.asm Removes ARM64 GenericCLRToCOMCallStub.
src/coreclr/vm/amd64/GenericCLRToCOMCallStubs.asm Deletes AMD64 GenericCLRToCOMCallStub file.
src/coreclr/vm/amd64/cgencpu.h Removes COM_STUBS_SEPARATE_FP_LOCATIONS define tied to old CLR→COM frame/stub model.
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs Removes unmanaged-entry wrapper for ForwardCallToInvokeMember now that IL stubs call managed method directly.
src/coreclr/System.Private.CoreLib/src/System/__ComObject.cs Removes unmanaged-entry wrapper for GetEventProvider now that IL stubs call managed method directly.
src/coreclr/debug/ee/frameinfo.h Removes CLRToCOMMethodFrame-specific stepping suppression flag.
src/coreclr/debug/ee/frameinfo.cpp Removes initialization and propagation of CLRToCOMMethodFrame-specific stepping suppression flag.
src/coreclr/debug/ee/controller.h Removes suppressUMChainFromCLRToCOMMethodFrameGeneric parameter/state.
src/coreclr/debug/ee/controller.cpp Removes CLRToCOMMethodFrame-specific stackwalk suppression logic and related comments.
src/coreclr/debug/daccess/dacdbiimpl.cpp Removes CLRToCOMMethodFrame from illustrative frame list.
docs/design/datacontracts/StackWalk.md Updates StackWalk contract docs to remove CLRToCOMMethodFrame.

Comment on lines +1621 to +1623
STANDARD_VM_CONTRACT;
m_wrapperTypes.ReSizeThrows(m_signature.NumFixedArgs());

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

LateBoundCLRToCOM_ILStubState reads m_wrapperTypesNeeded in FinishEmit/EmitInvokeTarget, but the field is never initialized in the constructor. If a late-bound call has no wrapper types (or has 0 args), this can lead to nondeterministic IL generation (e.g., trying to load m_wrapperArrayLocalNum without creating the local). Initialize m_wrapperTypesNeeded (and ideally m_wrapperArrayLocalNum) to a known default (e.g., FALSE / LOCAL_NUM_UNUSED) in the ctor or member initializer list.

Copilot uses AI. Check for mistakes.
_ASSERTE(FALSE);
return FALSE;

return TypeFromToken(metaTok) == mdtString && metaTok <= m_pCompileTimeState->m_tokenLookupMap.GetMaxUserStringToken();
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

ILStubResolver::IsValidStringRef treats any mdtString token <= GetMaxUserStringToken() as valid, but doesn’t reject RID 0 (token == mdtString), which is not a valid user-string token. This can make later code (e.g., LookupUserString preconditions) fail for edge tokens. Consider also requiring RidFromToken(metaTok) != 0 (and optionally metaTok >= TokenFromRid(1, mdtString)).

Suggested change
return TypeFromToken(metaTok) == mdtString && metaTok <= m_pCompileTimeState->m_tokenLookupMap.GetMaxUserStringToken();
if (TypeFromToken(metaTok) != mdtString)
return FALSE;
if (RidFromToken(metaTok) == 0)
return FALSE;
mdToken maxUserStringToken = m_pCompileTimeState->m_tokenLookupMap.GetMaxUserStringToken();
return metaTok >= TokenFromRid(1, mdtString) && metaTok <= maxUserStringToken;

Copilot uses AI. Check for mistakes.
STANDARD_VM_CHECK;
THROWS;
GC_NOTRIGGER;
MODE_ANY;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this multiple modes now? Is the no trigger to narrow impact for some reason?

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.

This can be called from the StubManager to trace the target of the IL stub for the debugger. That's in (technically) COOP right now (something @hoyosjs is looking into).

GC_NOTRIGGER is to help with the problems he's seeing (deadlocks between stack walking and GC).

);

if (strcmp(pMD->GetName(), "BeginInvoke") == 0)
#if defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#if defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE)
#if defined(FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE)

// Retrieve the method table and the method desc of the call.
MethodTable *pItfMT = pMD->GetInterfaceMethodTable();
CLRToCOMCallMethodDesc *pItfMD = pMD;
#endif
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#endif
#endif // FEATURE_DYNAMIC_METHOD_HAS_NATIVE_STACK_ARG_SIZE

LOG((LF_CORDB, LL_INFO10000,
"CLRToCOMMethodFrame::TraceFrame: ip=0x%p\n", ip));
PCODE pCode = JitILStub(*ppStubMD);
InterlockedCompareExchangeT<PCODE>(pComInfo->GetAddrOfILStubField(), pCode, NULL);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we ned to do any backout here if this fails to set?


STRINGREF ILStubResolver::GetStringLiteral(mdToken metaTok)
{
LIMITED_METHOD_CONTRACT;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This need to be marked GCX_COOP mode.

Comment on lines +1747 to +1748


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

pUnk = ComObject::GetComIPFromRCWThrowing(&oref, pCLRToCOMCallInfo->m_pInterfaceMT);
GCPROTECT_END();

LPVOID *lpVtbl = *(LPVOID **)(IUnknown *)pUnk;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd expect an assert that shows a positive QI for IDispatch.

FieldDesc *m_pFD;
};

class LateBoundCLRToCOM_ILStubState : public ILStubState
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
class LateBoundCLRToCOM_ILStubState : public ILStubState
class LateBoundCLRToCOM_ILStubState final : public ILStubState

class LateBoundCLRToCOM_ILStubState : public ILStubState
{
public:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +151 to +153
SString& str = m_pCompileTimeState->m_tokenLookupMap.LookupUserString(metaTok);

LPCWSTR unicodeStr = str.GetUnicode();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is the contract, I would do this slightly different. I don't think exposing a reference to an SString is ever a good idea. In particular, the current approach means if another callers GetUTF8(), we have race and will have a problem. I assume that since callers are all working with tokens it means they are expecting UTF0-8, which means I would ensure the cached SString is properly in that encoding and then update the LookupUserString() API to return a LPCWSTR.

If the length is really important, make the length an out.

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants