Fix mixed mode debugging for PInvoke by adding PInvokeStubManager

src/coreclr/debug/ee/controller.cpp

@@ -3987,7 +3987,7 @@ bool DebuggerController::DispatchTraceCall(Thread *thread,
                         _ASSERTE(info.HasReturnFrame());
 
                         // This check makes sure that we don't do this logic for inlined frames.
-                        if (info.GetReturnFrame().md->IsILStub())
+                        if (info.GetReturnFrame().md->IsInteropStub())
                         {
                             // Make sure that the frame pointer of the active frame is actually
                             // the address of an exit frame.
@@ -7070,20 +7070,20 @@ bool DebuggerStepper::Step(FramePointer fp, bool in,
     Thread *thread = GetThread();
     CONTEXT *context = g_pEEInterface->GetThreadFilterContext(thread);
 
-    // ControllerStackInfo doesn't report IL stubs, so if we are in an IL stub, we need
-    // to handle the single-step specially.  There are probably other problems when we stop
-    // in an IL stub.  We need to revisit this later.
-    bool fIsILStub = false;
+    // ControllerStackInfo doesn't report interop stubs (IL stubs and P/Invokes), so if we are
+    // in an interop stub, we need to handle the single-step specially.  There are probably other
+    // problems when we stop in an interop stub.  We need to revisit this later.
+    bool fIsInteropStub = false;
     if ((context != NULL) &&
         g_pEEInterface->IsManagedNativeCode(reinterpret_cast<const BYTE *>(GetIP(context))))
     {
         MethodDesc * pMD = g_pEEInterface->GetNativeCodeMethodDesc(GetIP(context));
         if (pMD != NULL)
         {
-            fIsILStub = pMD->IsILStub();
+            fIsInteropStub = pMD->IsInteropStub();
         }
     }
-    LOG((LF_CORDB, LL_INFO10000, "DS::S - fIsILStub = %d\n", fIsILStub));
+    LOG((LF_CORDB, LL_INFO10000, "DS::S - fIsInteropStub = %d\n", fIsInteropStub));
 
     ControllerStackInfo info;
 
@@ -7148,9 +7148,9 @@ bool DebuggerStepper::Step(FramePointer fp, bool in,
         rangeCount = 0;
     }
 
-    if (fIsILStub)
+    if (fIsInteropStub)
     {
-        // Don't use the ControllerStackInfo if we are in an IL stub.
+        // Don't use the ControllerStackInfo if we are in an interop stub.
         m_fp = fp;
     }
     else
@@ -7168,9 +7168,9 @@ bool DebuggerStepper::Step(FramePointer fp, bool in,
     LOG((LF_CORDB,LL_INFO10000,"DS::Step %p STEP_NORMAL\n",this));
     m_reason = STEP_NORMAL; //assume it'll be a normal step & set it to
     //something else if we walk over it
-    if (fIsILStub)
+    if (fIsInteropStub)
     {
-        LOG((LF_CORDB, LL_INFO10000, "DS::Step: stepping in an IL stub\n"));
+        LOG((LF_CORDB, LL_INFO10000, "DS::Step: stepping in an interop stub\n"));
 
         // Enable the right triggers if the user wants to step in.
         if (in)
@@ -7409,7 +7409,7 @@ TP_RESULT DebuggerStepper::TriggerPatch(DebuggerControllerPatch *patch,
                     {
                         // We're hitting this code path with MC++ assemblies
                         // that have an unmanaged entry point so the stub returns to CallDescrWorker.
-                        _ASSERTE(g_pEEInterface->GetNativeCodeMethodDesc(dac_cast<PCODE>(patch->address))->IsILStub());
+                        _ASSERTE(g_pEEInterface->GetNativeCodeMethodDesc(dac_cast<PCODE>(patch->address))->IsInteropStub());
                     }
 
                 }
@@ -7725,12 +7725,12 @@ bool DebuggerStepper::TriggerSingleStep(Thread *thread, const BYTE *ip)
     StackTraceTicket ticket(ip);
     info.GetStackInfo(ticket, GetThread(), LEAF_MOST_FRAME, NULL);
 
-    // This is a special case where we return from a managed method back to an IL stub.  This can
+    // This is a special case where we return from a managed method back to an interop stub.  This can
     // only happen if there's no more managed method frames closer to the root and we want to perform
-    // a step out, or if we step-next off the end of a method called by an IL stub.  In either case,
-    // we'll get a single step in an IL stub, which we want to ignore.  We also want to enable trace
-    // call here, just in case this IL stub is about to call the managed target (in the reverse interop case).
-    if (fd->IsILStub())
+    // a step out, or if we step-next off the end of a method called by an interop stub.  In either case,
+    // we'll get a single step in an interop stub, which we want to ignore.  We also want to enable trace
+    // call here, just in case this stub is about to call the managed target (in the reverse interop case).
+    if (fd->IsInteropStub())
     {
         LOG((LF_CORDB,LL_INFO10000, "DS::TSS: not in managed code, Returning false (case 0)!\n"));
         if (this->GetDCType() == DEBUGGER_CONTROLLER_STEPPER)

src/coreclr/debug/ee/frameinfo.cpp

@@ -1560,6 +1560,13 @@ StackWalkAction DebuggerWalkStackProc(CrawlFrame *pCF, void *data)
         }
     }
     else
+    // We ignore PInvoke methods with inlined stubs in our stackwalking.
+    // These are similar to IL stubs but use PInvokeMethodDesc instead of DynamicMethodDesc.
+    if ((md != NULL) && md->IsPInvoke() && pCF->IsFrameless())
+    {
+        LOG((LF_CORDB, LL_INFO100000, "DWSP: Skip frameless PInvoke stub.\n"));
+    }
+    else
     // For frames w/o method data, send them as an internal stub frame.
     if ((md != NULL) && md->IsDynamicMethod())
     {

src/coreclr/inc/vptr_list.h

@@ -35,6 +35,7 @@ VPTR_CLASS(JumpStubStubManager)
 #endif // FEATURE_JIT
 VPTR_CLASS(RangeSectionStubManager)
 VPTR_CLASS(ILStubManager)
+VPTR_CLASS(PInvokeStubManager)
 VPTR_CLASS(InteropDispatchStubManager)
 #if defined(TARGET_X86) && !defined(UNIX_X86_ABI)
 VPTR_CLASS(TailCallStubManager)

src/coreclr/vm/appdomain.cpp

@@ -727,6 +727,7 @@ void SystemDomain::Attach()
 #endif // FEATURE_JIT
     RangeSectionStubManager::Init();
     ILStubManager::Init();
+    PInvokeStubManager::Init();
     InteropDispatchStubManager::Init();
     StubLinkStubManager::Init();
     TailCallStubManager::Init();

src/coreclr/vm/method.hpp

@@ -755,6 +755,11 @@ class MethodDesc
         return mcPInvoke == GetClassification();
     }
 
+    // Returns true if this MethodDesc represents an interop stub.
+    // This includes interop IL stubs (PInvoke, COM, reverse PInvoke, struct marshal)
+    // and PInvoke methods (PInvokeMethodDesc).
+    inline bool IsInteropStub();
+
     inline DWORD IsInterface()
     {
         WRAPPER_NO_CONTRACT;

src/coreclr/vm/method.inl

@@ -133,6 +133,32 @@ inline bool MethodDesc::IsILStub()
     return ((mcDynamic == GetClassification()) && dac_cast<PTR_DynamicMethodDesc>(this)->IsILStub());
 }
 
+inline bool MethodDesc::IsInteropStub()
+{
+    WRAPPER_NO_CONTRACT;
+
+    if (IsPInvoke())
+        return true;
+
+    if (!IsILStub())
+        return false;
+
+    switch (AsDynamicMethodDesc()->GetILStubType())
+    {
+        case DynamicMethodDesc::StubPInvoke:
+        case DynamicMethodDesc::StubPInvokeDelegate:
+        case DynamicMethodDesc::StubPInvokeCalli:
+        case DynamicMethodDesc::StubPInvokeVarArg:
+        case DynamicMethodDesc::StubReversePInvoke:
+        case DynamicMethodDesc::StubCLRToCOMInterop:
+        case DynamicMethodDesc::StubCOMToCLRInterop:
+        case DynamicMethodDesc::StubStructMarshalInterop:
+            return true;
+        default:
+            return false;
+    }
+}
+
 // This method is intended to identify methods that aren't shown in diagnostic introspection (stacktraces,
 // code viewing, stepping, etc). Partly this is a user experience consideration to preserve the
 // abstraction users would expect based on source code and assembly contents. Partly it is also a technical

src/coreclr/vm/stubmgr.cpp

@@ -1861,6 +1861,75 @@ BOOL ILStubManager::TraceManager(Thread *thread,
 }
 #endif //!DACCESS_COMPILE
 
+//
+// This is the stub manager for PInvoke stubs.
+//
+
+#ifndef DACCESS_COMPILE
+
+/* static */
+void PInvokeStubManager::Init()
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_NOTRIGGER;
+        MODE_ANY;
+    }
+    CONTRACTL_END
+
+    StubManager::AddStubManager(new PInvokeStubManager());
+}
+
+#endif // #ifndef DACCESS_COMPILE
+
+BOOL PInvokeStubManager::CheckIsStub_Internal(PCODE stubStartAddress)
+{
+    WRAPPER_NO_CONTRACT;
+    SUPPORTS_DAC;
+
+    MethodDesc *pMD = ExecutionManager::GetCodeMethodDesc(stubStartAddress);
+
+    return (pMD != NULL) && pMD->IsPInvoke();
+}
+
+BOOL PInvokeStubManager::DoTraceStub(PCODE stubStartAddress,
+                                     TraceDestination *trace)
+{
+    LIMITED_METHOD_CONTRACT;
+
+    LOG((LF_CORDB, LL_EVERYTHING, "PInvokeStubManager::DoTraceStub called\n"));
+
+#ifndef DACCESS_COMPILE
+
+    MethodDesc* pMD = ExecutionManager::GetCodeMethodDesc(stubStartAddress);
+    if (pMD == NULL || !pMD->IsPInvoke())
+    {
+        LOG((LF_CORDB, LL_INFO1000, "PISM::DoTraceStub: Not a PInvoke stub\n"));
+        return FALSE;
+    }
+
+    PInvokeMethodDesc* pNMD = reinterpret_cast<PInvokeMethodDesc*>(pMD);
+    // Note: The PInvoke target may not yet be resolved if it uses lazy binding
+    // (PRECODE_PINVOKE_IMPORT). In that case, GetPInvokeTarget() returns the
+    // precode address. There is a narrow race where the target gets resolved
+    // between this read and the debugger setting a breakpoint, but this is
+    // low priority to address.
+    PCODE target = (PCODE)pNMD->GetPInvokeTarget();
+    LOG((LF_CORDB, LL_INFO10000, "PISM::DoTraceStub: PInvoke target 0x%p\n", target));
+    trace->InitForUnmanaged(target);
+
+    LOG_TRACE_DESTINATION(trace, target, "PInvokeStubManager::DoTraceStub");
+
+    return TRUE;
+
+#else // !DACCESS_COMPILE
+    trace->InitForOther((PCODE)NULL);
+    return FALSE;
+
+#endif // !DACCESS_COMPILE
+}
+
 // This is used to recognize GenericCLRToCOMCallStub, VarargPInvokeStub, and GenericPInvokeCalliHelper.
 
 #ifndef DACCESS_COMPILE
@@ -2291,6 +2360,15 @@ ILStubManager::DoEnumMemoryRegions(CLRDataEnumMemoryFlags flags)
     EMEM_OUT(("MEM: %p ILStubManager\n", dac_cast<TADDR>(this)));
 }
 
+void
+PInvokeStubManager::DoEnumMemoryRegions(CLRDataEnumMemoryFlags flags)
+{
+    SUPPORTS_DAC;
+    WRAPPER_NO_CONTRACT;
+    DAC_ENUM_VTHIS();
+    EMEM_OUT(("MEM: %p PInvokeStubManager\n", dac_cast<TADDR>(this)));
+}
+
 void
 InteropDispatchStubManager::DoEnumMemoryRegions(CLRDataEnumMemoryFlags flags)
 {

src/coreclr/vm/stubmgr.h

@@ -643,6 +643,54 @@ class ILStubManager : public StubManager
 #endif
 };
 
+//
+// This is the stub manager for PInvoke stubs.
+// It handles addresses that map to a PInvokeMethodDesc.
+//
+typedef VPTR(class PInvokeStubManager) PTR_PInvokeStubManager;
+
+class PInvokeStubManager : public StubManager
+{
+    VPTR_VTABLE_CLASS(PInvokeStubManager, StubManager)
+
+  public:
+    static void Init();
+
+#ifndef DACCESS_COMPILE
+    PInvokeStubManager() : StubManager() {WRAPPER_NO_CONTRACT;}
+    ~PInvokeStubManager()
+    {
+        CONTRACTL
+        {
+            NOTHROW;
+            GC_NOTRIGGER;
+            CAN_TAKE_LOCK;     // StubManager::UnlinkStubManager uses a crst
+        }
+        CONTRACTL_END;
+    }
+#endif
+
+   public:
+
+#ifdef _DEBUG
+    virtual const char * DbgGetName() { LIMITED_METHOD_CONTRACT; return "PInvokeStubManager"; }
+#endif
+
+    virtual BOOL CheckIsStub_Internal(PCODE stubStartAddress);
+
+  private:
+
+    virtual BOOL DoTraceStub(PCODE stubStartAddress, TraceDestination *trace);
+
+#ifdef DACCESS_COMPILE
+    virtual void DoEnumMemoryRegions(CLRDataEnumMemoryFlags flags);
+
+  protected:
+    virtual LPCWSTR GetStubManagerName(PCODE addr)
+        { LIMITED_METHOD_CONTRACT; return W("PInvokeStub"); }
+#endif
+};
+
 // This is used to recognize
 //   GenericCLRToCOMCallStub()
 //   VarargPInvokeStub()