Migrate CLR to COM stubs to be IL stubs

docs/design/datacontracts/StackWalk.md

@@ -278,7 +278,6 @@ When updating the context from a TransitionFrame, the IP, SP, and all ABI specif
 
 The following Frame types also use this mechanism:
 * FramedMethodFrame
-* CLRToCOMMethodFrame
 * PInvokeCallIFrame
 * PrestubMethodFrame
 * StubDispatchFrame

src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs

@@ -4221,39 +4221,6 @@ private object InvokeDispMethod(
             return ret;
         }
 
-        [RequiresUnreferencedCode("The member might be removed")]
-        [UnmanagedCallersOnly]
-        private static unsafe void ForwardCallToInvokeMember(
-            RuntimeType* pRuntimeType,
-            string* pMemberName,
-            int flags,
-            object* pTarget,
-            object[]* pArgs,
-            bool[]* pArgsIsByRef,
-            int[]* pArgsWrapperTypes,
-            Type[]* pArgsTypes,
-            Type* pRetType,
-            object* pResult,
-            Exception* pException)
-        {
-            try
-            {
-                *pResult = pRuntimeType->ForwardCallToInvokeMember(
-                    *pMemberName,
-                    (BindingFlags)flags,
-                    *pTarget,
-                    *pArgs,
-                    *pArgsIsByRef,
-                    *pArgsWrapperTypes,
-                    *pArgsTypes,
-                    *pRetType);
-            }
-            catch (Exception ex)
-            {
-                *pException = ex;
-            }
-        }
-
         private static void WrapArgsForInvokeCall(object[] aArgs, int[] aArgsWrapperTypes)
         {
             int cArgs = aArgs.Length;

src/coreclr/System.Private.CoreLib/src/System/__ComObject.cs

@@ -140,21 +140,6 @@ internal object GetEventProvider(
             return CreateEventProvider(t);
         }
 
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2062:Value passed to parameter cannot be statically determined", Justification = "The runtime passes a RuntimeType describing the COM event provider. The dynamic constructor access requirements are enforced by runtime callsite semantics.")]
-        [UnmanagedCallersOnly]
-        [RequiresUnsafe]
-        private static unsafe void GetEventProvider(__ComObject* pComObject, RuntimeType* pProviderType, object* pResult, Exception* pException)
-        {
-            try
-            {
-                *pResult = pComObject->GetEventProvider(*pProviderType);
-            }
-            catch (Exception ex)
-            {
-                *pException = ex;
-            }
-        }
-
         private object CreateEventProvider(
             [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] RuntimeType t)
         {

src/coreclr/debug/daccess/dacdbiimpl.cpp

@@ -6127,7 +6127,7 @@ HRESULT STDMETHODCALLTYPE DacDbiInterfaceImpl::GetContext(VMPTR_Thread vmThread,
 
                 // Going through thread Frames and looking for first (deepest one) one that
                 // that has context available for stackwalking (SP and PC)
-                // For example: RedirectedThreadFrame, InlinedCallFrame, DynamicHelperFrame, CLRToCOMMethodFrame
+                // For example: RedirectedThreadFrame, InlinedCallFrame, DynamicHelperFrame
                 Frame *frame = pThread->GetFrame();
 
                 while (frame != NULL && frame != FRAME_TOP)

src/coreclr/debug/ee/controller.cpp

@@ -468,15 +468,12 @@ StackTraceTicket::StackTraceTicket(DebuggerUserBreakpoint * p)
 //      caller wants information about a specific frame.
 // CONTEXT* pContext:  A pointer to a CONTEXT structure.  Can be null,
 // we use our temp context.
-// bool suppressUMChainFromCLRToCOMMethodFrameGeneric - A ridiculous flag that is trying to narrowly
-//      target a fix for issue 650903.
 // StackTraceTicket - ticket to ensure that we actually have permission for this stacktrace
 void ControllerStackInfo::GetStackInfo(
     StackTraceTicket ticket,
     Thread *thread,
     FramePointer targetFP,
-    CONTEXT *pContext,
-    bool suppressUMChainFromCLRToCOMMethodFrameGeneric
+    CONTEXT *pContext
     )
 {
     _ASSERTE(thread != NULL);
@@ -504,7 +501,6 @@ void ControllerStackInfo::GetStackInfo(
     m_targetFP  = targetFP;
     m_targetFrameFound = (m_targetFP == LEAF_MOST_FRAME);
     m_specialChainReason = CHAIN_NONE;
-    m_suppressUMChainFromCLRToCOMMethodFrameGeneric = suppressUMChainFromCLRToCOMMethodFrameGeneric;
 
     int result = DebuggerWalkStack(thread,
                                    LEAF_MOST_FRAME,
@@ -565,26 +561,6 @@ StackWalkAction ControllerStackInfo::WalkStack(FrameInfo *pInfo, void *data)
     if (i->m_bottomFP == LEAF_MOST_FRAME)
         i->m_bottomFP = pInfo->fp;
 
-    // This is part of the targeted fix for issue 650903 (see the other
-    // parts in code:TrackUMChain and code:DebuggerStepper::TrapStepOut).
-    //
-    // pInfo->fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric has been
-    // set by TrackUMChain to help us remember that the current frame we're looking at is
-    // CLRToCOMMethodFrameGeneric (we can't rely on looking at pInfo->frame to check
-    // this), and i->m_suppressUMChainFromCLRToCOMMethodFrameGeneric has been set by the
-    // dude initiating this walk to remind us that our goal in life is to do a Step Out
-    // during managed-only debugging. These two things together tell us we should ignore
-    // this frame, rather than erroneously identifying it as the target frame.
-    //
-#ifdef FEATURE_COMINTEROP
-    if(i->m_suppressUMChainFromCLRToCOMMethodFrameGeneric &&
-        (pInfo->chainReason == CHAIN_ENTER_UNMANAGED) &&
-        (pInfo->fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric))
-    {
-        return SWA_CONTINUE;
-    }
-#endif // FEATURE_COMINTEROP
-
     //have we reached the correct frame yet?
     if (!i->m_targetFrameFound &&
         IsEqualOrCloserToLeaf(i->m_targetFP, pInfo->fp))
@@ -5488,7 +5464,7 @@ InterpreterStepHelper::StepSetupResult InterpreterStepHelper::SetupStep(
                     LOG((LF_CORDB, LL_INFO10000, "ISH::SS: skipIP == nextIP, only one patch needed\n"));
                 }
                 else
-                {                
+                {
                     LOG((LF_CORDB, LL_INFO10000, "ISH::SS: No skip IP for conditional branch!\n"));
                     return SSR_Failed;
                 }
@@ -6909,15 +6885,7 @@ void DebuggerStepper::TrapStepOut(ControllerStackInfo *info, bool fForceTraditio
         // stack.
         StackTraceTicket ticket(info);
 
-        // The last parameter here is part of a really targeted (*cough* dirty) fix to
-        // disable getting an unwanted UMChain to fix issue 650903 (See
-        // code:ControllerStackInfo::WalkStack and code:TrackUMChain for the other
-        // parts.) In the case of managed step out we know that we aren't interested in
-        // unmanaged frames, and generating that unmanaged frame causes the stackwalker
-        // not to report the managed frame that was at the same SP. However the unmanaged
-        // frame might be used in the mixed-mode step out case so I don't suppress it
-        // there.
-        returnInfo.GetStackInfo(ticket, GetThread(), info->GetReturnFrame().fp, NULL, !(m_rgfMappingStop & STOP_UNMANAGED));
+        returnInfo.GetStackInfo(ticket, GetThread(), info->GetReturnFrame().fp, NULL);
         info = &returnInfo;
 
 #ifdef _DEBUG
@@ -7840,7 +7808,7 @@ TP_RESULT DebuggerStepper::TriggerPatch(DebuggerControllerPatch *patch,
 
     // With the addition of Async Thunks, it is now possible that an unjitted method trace
     // represents a stub. We need to check for this case and follow the stub trace to find
-    // the real target. 
+    // the real target.
     // Replica patches do not contain a reference to the original trace so we can not rely
     // on the trace type == TRACE_UNJITTED_METHOD to identify this case.
     {
@@ -7878,7 +7846,7 @@ TP_RESULT DebuggerStepper::TriggerPatch(DebuggerControllerPatch *patch,
                     traceOk, traceOk ? trace.GetTraceType() : -1));
             }
 
-            if (traceOk && 
+            if (traceOk &&
                 g_pEEInterface->FollowTrace(&trace) &&
                 PatchTrace(&trace, info.m_activeFrame.fp,
                            (m_rgfMappingStop & STOP_UNMANAGED) ? true : false))
@@ -8936,8 +8904,7 @@ TP_RESULT DebuggerThreadStarter::TriggerPatch(DebuggerControllerPatch *patch,
         else if ((patch->trace.GetTraceType() == TRACE_FRAME_PUSH) && (thread->GetFrame()->IsTransitionToNativeFrame()))
         {
             // If we've got a frame that is transitioning to native, there's no reason to try to keep tracing. So we
-            // bail early and save ourselves some effort. This also works around a problem where we deadlock trying to
-            // do too much work to determine the destination of a CLRToCOMMethodFrame. (See issue 87103.)
+            // bail early and save ourselves some effort.
             //
             // Note: trace call is still enabled, so we can just ignore this patch and wait for trace call to fire
             // again...

src/coreclr/debug/ee/controller.h

@@ -187,15 +187,12 @@ class ControllerStackInfo
     //      caller wants information about a specific frame.
     // CONTEXT* pContext:  A pointer to a CONTEXT structure.  Can be null,
     //      we use our temp context.
-    // bool suppressUMChainFromCLRToCOMMethodFrameGeneric - A ridiculous flag that is trying to narrowly
-    //      target a fix for issue 650903.
     // StackTraceTicket - ticket ensuring that we have permission to call this.
     void GetStackInfo(
         StackTraceTicket ticket,
         Thread *thread,
         FramePointer targetFP,
-        CONTEXT *pContext,
-        bool suppressUMChainFromCLRToCOMMethodFrameGeneric = false
+        CONTEXT *pContext
         );
 
     //bool ControllerStackInfo::HasReturnFrame()  Returns
@@ -218,11 +215,6 @@ class ControllerStackInfo
     bool                    m_returnFound;
     FrameInfo               m_returnFrame;
 
-    // A ridiculous flag that is targeting a very narrow fix at issue 650903
-    // (4.5.1/Blue).  This is set for the duration of a stackwalk designed to
-    // help us "Step Out" to a managed frame (i.e., managed-only debugging).
-    bool                    m_suppressUMChainFromCLRToCOMMethodFrameGeneric;
-
     // Track if this stackwalk actually happened.
     // This is used by the StackTraceTicket(ControllerStackInfo * info) ticket.
     INDEBUG(bool m_dbgExecuted);

src/coreclr/debug/ee/frameinfo.cpp

@@ -107,8 +107,6 @@ struct DebuggerFrameData
         this->info.fIsFunclet = false;
         this->info.fIsFilter  = false;
 
-        this->info.fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric = false;
-
 #if defined(_DEBUG)
         this->previousFP = LEAF_MOST_FRAME;
 #endif // _DEBUG
@@ -1185,22 +1183,6 @@ StackWalkAction TrackUMChain(CrawlFrame *pCF, DebuggerFrameData *d)
 
         f.InitForUMChain(fpRoot, d->GetUMChainStartRD());
 
-#ifdef FEATURE_COMINTEROP
-        if ((frame != NULL) &&
-            (frame->GetFrameIdentifier() == FrameIdentifier::CLRToCOMMethodFrame))
-        {
-            // This condition is part of the fix for 650903. (See
-            // code:ControllerStackInfo::WalkStack and code:DebuggerStepper::TrapStepOut
-            // for the other parts.) Here, we know that the frame we're looking it may be
-            // a CLRToCOMMethodFrameGeneric (this info is not otherwise plubmed down into
-            // the walker; even though the walker does get to see "f.frame", that may not
-            // be "frame"). Given this, if the walker chooses to ignore these frames
-            // (while doing a Step Out during managed-only debugging), then it can ignore
-            // this frame.
-            f.fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric = true;
-        }
-#endif // FEATURE_COMINTEROP
-
         if (d->InvokeCallback(&f) == SWA_ABORT)
         {
             // don't need to cancel if they abort.

src/coreclr/debug/ee/frameinfo.h

@@ -100,15 +100,6 @@ struct FrameInfo
     bool IsFilterFrame()           { return fIsFilter;  }
     bool IsNonFilterFuncletFrame() { return (fIsFunclet && !fIsFilter); }
 
-    // A ridiculous flag that is targeting a very narrow fix at issue 650903 (4.5.1/Blue).
-    // This is set when the currently walked frame is a CLRToCOMMethodFrameGeneric. If the
-    // dude doing the walking is trying to ignore such frames (see
-    // code:ControllerStackInfo::m_suppressUMChainFromCLRToCOMMethodFrameGeneric), AND
-    // this is set, then the walker just continues on to the next frame, without
-    // erroneously identifying this frame as the target frame. Only used during "Step
-    // Out" to a managed frame (i.e., managed-only debugging).
-    bool                fIgnoreThisFrameIfSuppressingUMChainFromCLRToCOMMethodFrameGeneric;
-
     // In addition to a Method, a FrameInfo may also represent either a Chain or a Stub (but not both).
     // chainReason corresponds to ICorDebugChain::GetReason().
     CorDebugChainReason  chainReason;

src/coreclr/vm/CMakeLists.txt

@@ -410,7 +410,6 @@ set(VM_HEADERS_WKS
     baseassemblyspec.inl
     cachelinealloc.h
     callhelpers.h
-    callsiteinspect.h
     callconvbuilder.hpp
     cdacdata.h
     ceemain.h
@@ -577,14 +576,6 @@ if(CLR_CMAKE_TARGET_WIN32)
         amsi.h
     )
 
-    # COM interop scenarios
-    list(APPEND VM_SOURCES_DAC_AND_WKS_WIN32
-        clrtocomcall.cpp
-        )
-    list(APPEND VM_HEADERS_DAC_AND_WKS_WIN32
-        clrtocomcall.h
-        )
-
     list(APPEND VM_SOURCES_WKS
         ${VM_SOURCES_DAC_AND_WKS_WIN32}
         # These should not be included for Linux
@@ -603,14 +594,14 @@ if(CLR_CMAKE_TARGET_WIN32)
 
     # COM interop scenarios
     list(APPEND VM_SOURCES_WKS
-        callsiteinspect.cpp
         classcompat.cpp
         comcache.cpp
         comcallablewrapper.cpp
         comconnectionpoints.cpp
         cominterfacemarshaler.cpp
         commtmemberinfomap.cpp
         comtoclrcall.cpp
+        clrtocomcall.cpp
         dispatchinfo.cpp
         dispparammarshaler.cpp
         olecontexthelpers.cpp
@@ -626,6 +617,7 @@ if(CLR_CMAKE_TARGET_WIN32)
         cominterfacemarshaler.h
         commtmemberinfomap.h
         comtoclrcall.h
+        clrtocomcall.h
         dispatchinfo.h
         dispparammarshaler.h
         olecontexthelpers.h
@@ -662,7 +654,6 @@ elseif(CLR_CMAKE_TARGET_WIN32)
             ${ARCH_SOURCES_DIR}/CallDescrWorkerAMD64.asm
             ${ARCH_SOURCES_DIR}/ComCallPreStub.asm
             ${ARCH_SOURCES_DIR}/GenericComCallStubs.asm
-            ${ARCH_SOURCES_DIR}/GenericCLRToCOMCallStubs.asm
             ${ARCH_SOURCES_DIR}/getstate.asm
             ${ARCH_SOURCES_DIR}/JitHelpers_Fast.asm
             ${ARCH_SOURCES_DIR}/JitHelpers_FastWriteBarriers.asm

src/coreclr/vm/FrameTypes.h

@@ -20,7 +20,6 @@ FRAME_TYPE_NAME(FuncEvalFrame)
 #endif // DEBUGGING_SUPPORTED
 #ifdef FEATURE_COMINTEROP
 FRAME_TYPE_NAME(ComMethodFrame)
-FRAME_TYPE_NAME(CLRToCOMMethodFrame)
 FRAME_TYPE_NAME(ComPrestubMethodFrame)
 #endif // FEATURE_COMINTEROP
 FRAME_TYPE_NAME(PInvokeCalliFrame)

src/coreclr/vm/amd64/cgencpu.h

@@ -64,7 +64,6 @@ class ComCallMethodDesc;
 #define ENREGISTERED_RETURNTYPE_INTEGER_MAXSIZE 8    // bytes
 #define ENREGISTERED_PARAMTYPE_MAXSIZE          8    // bytes
 #define ENREGISTERED_RETURNTYPE_MAXSIZE         8    // bytes
-#define COM_STUBS_SEPARATE_FP_LOCATIONS
 #define CALLDESCR_REGTYPEMAP                    1
 #endif