Fix stack walking and reporting of default interface methods (#21525)

Default interface methods in their unresolved state don't have a generic context. The generic context is only added once the method is resolved to its implementation.

8 files changed, 81 insertions, 11 deletions

diff --git a/src/inc/corcompile.h b/src/inc/corcompile.h
index e04da79365..a644dd8026 100644
--- a/src/inc/corcompile.h
+++ b/src/inc/corcompile.h
@@ -1711,7 +1711,8 @@ class ICorCompileInfo
     //
     virtual void GetCallRefMap(
             CORINFO_METHOD_HANDLE hMethod,
-            GCRefMapBuilder * pBuilder) = 0;
+            GCRefMapBuilder * pBuilder,
+            bool isDispatchCell) = 0;
 
     // Returns a compressed block of debug information
     //
diff --git a/src/vm/compile.cpp b/src/vm/compile.cpp
index 1bd75fcffd..3cec66e833 100644
--- a/src/vm/compile.cpp
+++ b/src/vm/compile.cpp
@@ -964,7 +964,7 @@ void FakeGcScanRoots(MetaSig& msig, ArgIterator& argit, MethodDesc * pMD, BYTE *
     }
 }
 
-void CEECompileInfo::GetCallRefMap(CORINFO_METHOD_HANDLE hMethod, GCRefMapBuilder * pBuilder)
+void CEECompileInfo::GetCallRefMap(CORINFO_METHOD_HANDLE hMethod, GCRefMapBuilder * pBuilder, bool isDispatchCell)
 {
 #ifdef _DEBUG
     DWORD dwInitialLength = pBuilder->GetBlobLength();
@@ -973,7 +973,25 @@ void CEECompileInfo::GetCallRefMap(CORINFO_METHOD_HANDLE hMethod, GCRefMapBuilde
 
     MethodDesc *pMD = (MethodDesc *)hMethod;
 
-    MetaSig msig(pMD);
+    SigTypeContext typeContext(pMD);
+    PCCOR_SIGNATURE pSig;
+    DWORD cbSigSize;
+    pMD->GetSig(&pSig, &cbSigSize);
+    MetaSig msig(pSig, cbSigSize, pMD->GetModule(), &typeContext);
+
+    //
+    // Shared default interface methods (i.e. virtual interface methods with an implementation) require
+    // an instantiation argument. But if we're in a situation where we haven't resolved the method yet
+    // we need to pretent that unresolved default interface methods are like any other interface
+    // methods and don't have an instantiation argument.
+    // See code:CEEInfo::getMethodSigInternal
+    //
+    assert(!isDispatchCell || !pMD->RequiresInstArg() || pMD->GetMethodTable()->IsInterface());
+    if (pMD->RequiresInstArg() && !isDispatchCell)
+    {
+        msig.SetHasParamTypeArg();
+    }
+
     ArgIterator argit(&msig);
 
     UINT nStackBytes = argit.SizeOfFrameArgumentArray();
diff --git a/src/vm/compile.h b/src/vm/compile.h
index 4307d97147..79991ddee6 100644
--- a/src/vm/compile.h
+++ b/src/vm/compile.h
@@ -338,7 +338,8 @@ class CEECompileInfo : public ICorCompileInfo
                              SString                &result);
 
     void GetCallRefMap(CORINFO_METHOD_HANDLE hMethod, 
-                       GCRefMapBuilder * pBuilder);
+                       GCRefMapBuilder * pBuilder,
+                       bool isDispatchCell);
 
     void CompressDebugInfo(
                                     IN ICorDebugInfo::OffsetMapping * pOffsetMapping,
diff --git a/src/vm/frames.cpp b/src/vm/frames.cpp
index 4ca2e92610..cea5241ef5 100644
--- a/src/vm/frames.cpp
+++ b/src/vm/frames.cpp
@@ -1238,7 +1238,16 @@ void TransitionFrame::PromoteCallerStack(promote_func* fn, ScanContext* sc)
     //If not "vararg" calling convention, assume "default" calling convention
     if (!MetaSig::IsVarArg(pFunction->GetModule(), callSignature))
     {
-        MetaSig msig(pFunction);
+        SigTypeContext typeContext(pFunction);
+        PCCOR_SIGNATURE pSig;
+        DWORD cbSigSize;
+        pFunction->GetSig(&pSig, &cbSigSize);
+
+        MetaSig msig(pSig, cbSigSize, pFunction->GetModule(), &typeContext);
+
+        if (pFunction->RequiresInstArg() && !SuppressParamTypeArg())
+            msig.SetHasParamTypeArg();
+
         PromoteCallerStackHelper (fn, sc, pFunction, &msig);
     }
     else
diff --git a/src/vm/frames.h b/src/vm/frames.h
index a77ad63d12..f8bd4bec79 100644
--- a/src/vm/frames.h
+++ b/src/vm/frames.h
@@ -958,6 +958,15 @@ public:
     //---------------------------------------------------------------
     PTR_VOID GetParamTypeArg();
 
+    //---------------------------------------------------------------
+    // Gets value indicating whether the generic parameter type
+    // argument should be supressed.
+    //---------------------------------------------------------------
+    virtual BOOL SuppressParamTypeArg()
+    {
+        return FALSE;
+    }
+
 protected:  // we don't want people using this directly
     //---------------------------------------------------------------
     // Get the address of the "this" object. WARNING!!! Whether or not "this"
@@ -2267,6 +2276,22 @@ public:
 
     Interception GetInterception();
 
+    virtual BOOL SuppressParamTypeArg()
+    {
+        //
+        // Shared default interface methods (i.e. virtual interface methods with an implementation) require
+        // an instantiation argument. But if we're in the stub dispatch frame, we haven't actually resolved
+        // the method yet (we could end up in class's override of this method, for example).
+        //
+        // So we need to pretent that unresolved default interface methods are like any other interface
+        // methods and don't have an instantiation argument.
+        //
+        // See code:CEEInfo::getMethodSigInternal
+        //
+        assert(GetFunction()->GetMethodTable()->IsInterface());
+        return TRUE;
+    }
+
 private:
     friend class VirtualCallStubManager;
 
diff --git a/src/vm/gcenv.ee.common.cpp b/src/vm/gcenv.ee.common.cpp
index 8f247b7f26..8ce6709a61 100644
--- a/src/vm/gcenv.ee.common.cpp
+++ b/src/vm/gcenv.ee.common.cpp
@@ -81,7 +81,9 @@ unsigned FindFirstInterruptiblePoint(CrawlFrame* pCF, unsigned offs, unsigned en
 //-----------------------------------------------------------------------------
 // Determine whether we should report the generic parameter context
 // 
-// This is meant to detect the situation where a ThreadAbortException is raised
+// This is meant to detect following situations:
+//
+// When a ThreadAbortException is raised
 // in the prolog of a managed method, before the location for the generics 
 // context has been initialized; when such a TAE is raised, we are open to a
 // race with the GC (e.g. while creating the managed object for the TAE).
@@ -90,9 +92,23 @@ unsigned FindFirstInterruptiblePoint(CrawlFrame* pCF, unsigned offs, unsigned en
 // The long term solution is to avoid raising TAEs in any non-GC safe points, 
 // and to additionally ensure that we do not expose the runtime to TAE 
 // starvation.
+//
+// When we're in the process of resolution of an interface method and the
+// interface method happens to have a default implementation. Normally,
+// such methods require a generic context, but since we didn't resolve the
+// method to an implementation yet, we don't have the right context (in fact,
+// there's no context provided by the caller).
+// See code:CEEInfo::getMethodSigInternal
+//
 inline bool SafeToReportGenericParamContext(CrawlFrame* pCF)
 {
     LIMITED_METHOD_CONTRACT;
+
+    if (!pCF->IsFrameless() && pCF->GetFrame()->GetVTablePtr() == StubDispatchFrame::GetMethodFrameVPtr())
+    {
+        return !((StubDispatchFrame*)pCF->GetFrame())->SuppressParamTypeArg();
+    }
+
     if (!pCF->IsFrameless() || !(pCF->IsActiveFrame() || pCF->IsInterrupted()))
     {
         return true;
@@ -353,7 +369,7 @@ StackWalkAction GcStackCrawlCallBack(CrawlFrame* pCF, VOID* pData)
                         paramContextType = GENERIC_PARAM_CONTEXT_METHODTABLE;
                 }
 
-                if (SafeToReportGenericParamContext(pCF))
+                if (paramContextType != GENERIC_PARAM_CONTEXT_NONE && SafeToReportGenericParamContext(pCF))
                 {
                     // Handle the case where the method is a static shared generic method and we need to keep the type 
                     // of the generic parameters alive
diff --git a/src/zap/zapimport.cpp b/src/zap/zapimport.cpp
index 7b847e9430..5e1a42c119 100644
--- a/src/zap/zapimport.cpp
+++ b/src/zap/zapimport.cpp
@@ -658,7 +658,7 @@ void ZapImportSectionSignatures::PlaceStubDispatchCell(ZapImport * pImport)
 
     m_pImage->GetImportTable()->PlaceImportBlob(pCell);
 
-    m_pGCRefMapTable->Append(pCell->GetMethod());
+    m_pGCRefMapTable->Append(pCell->GetMethod(), true);
 }
 
 //
@@ -871,9 +871,9 @@ ZapImport * ZapImportTable::GetVirtualImportThunk(CORINFO_METHOD_HANDLE handle,
 // GCRefMapTable is used to encode for GC references locations for lazily resolved calls
 //
 
-void ZapGCRefMapTable::Append(CORINFO_METHOD_HANDLE handle)
+void ZapGCRefMapTable::Append(CORINFO_METHOD_HANDLE handle, bool isDispatchCell)
 {
-    m_pImage->GetCompileInfo()->GetCallRefMap(handle, &m_GCRefMapBuilder);
+    m_pImage->GetCompileInfo()->GetCallRefMap(handle, &m_GCRefMapBuilder, isDispatchCell);
     m_nCount++;
 }
 
diff --git a/src/zap/zapimport.h b/src/zap/zapimport.h
index 058cb0b145..d9359e9df8 100644
--- a/src/zap/zapimport.h
+++ b/src/zap/zapimport.h
@@ -533,7 +533,7 @@ public:
     {
     }
 
-    void Append(CORINFO_METHOD_HANDLE handle);
+    void Append(CORINFO_METHOD_HANDLE handle, bool isDispatchCell = false);
 
     virtual DWORD GetSize();