Exclude runtime-async methods from built-in COM vtables and GUID calculations (#122195)

Author: jkoritzinsky

src/coreclr/vm/asyncthunks.cpp

@@ -65,7 +65,7 @@ void MethodDesc::EmitTaskReturningThunk(MethodDesc* pAsyncCallVariant, MetaSig&
     _ASSERTE(!pAsyncCallVariant->IsAsyncThunkMethod());
 
     // Emits roughly the following code:
-    // 
+    //
     // ExecutionAndSyncBlockStore store = default;
     // store.Push();
     // try
@@ -569,7 +569,15 @@ void MethodDesc::EmitAsyncMethodThunk(MethodDesc* pTaskReturningVariant, MetaSig
     }
 
     // other(arg)
-    pCode->EmitCALL(userFuncToken, localArg, 1);
+    if (pTaskReturningVariant->IsAbstract())
+    {
+        _ASSERTE(pTaskReturningVariant->IsCLRToCOMCall());
+        pCode->EmitCALLVIRT(userFuncToken, localArg, 1);
+    }
+    else
+    {
+        pCode->EmitCALL(userFuncToken, localArg, 1);
+    }
 
     TypeHandle thLogicalRetType = msig.GetRetTypeHandleThrowing();
     if (IsValueTaskAsyncThunk())

src/coreclr/vm/class.cpp

@@ -2828,13 +2828,15 @@ void SparseVTableMap::AllocOrExpand()
 }
 
 //*******************************************************************************
-// While building mapping list, record a gap in VTable slot numbers.
+// While building mapping list, record a gap in VTable slot numbers or MT slots.
+// A positive number indicates a gap in the VTable slot numbers.
+// A negative number indicates a gap in the MT slots.
 void SparseVTableMap::RecordGap(WORD StartMTSlot, WORD NumSkipSlots)
 {
     STANDARD_VM_CONTRACT;
 
     _ASSERTE((StartMTSlot == 0) || (StartMTSlot > m_MTSlot));
-    _ASSERTE(NumSkipSlots > 0);
+    _ASSERTE(NumSkipSlots != 0);
 
     // We use the information about the current gap to complete a map entry for
     // the last non-gap. There is a special case where the vtable begins with a
@@ -2860,6 +2862,14 @@ void SparseVTableMap::RecordGap(WORD StartMTSlot, WORD NumSkipSlots)
     m_MapEntries++;
 }
 
+//*******************************************************************************
+// While building mapping list, record an excluded MT slot.
+void SparseVTableMap::RecordExcludedMethod(WORD MTSlot)
+{
+    WRAPPER_NO_CONTRACT;
+    return RecordGap(MTSlot, -1);
+}
+
 //*******************************************************************************
 // Finish creation of mapping list.
 void SparseVTableMap::FinalizeMapping(WORD TotalMTSlots)

src/coreclr/vm/class.h

@@ -289,6 +289,9 @@ class SparseVTableMap
     // occurs.
     void RecordGap(WORD StartMTSlot, WORD NumSkipSlots);
 
+    // Record that the method table slot at MTSlot is excluded from the VT slots.
+    void RecordExcludedMethod(WORD MTSlot);
+
     // Then call FinalizeMapping to create the actual mapping list.
     void FinalizeMapping(WORD TotalMTSlots);
 

src/coreclr/vm/clrtocomcall.cpp

@@ -212,6 +212,7 @@ I4ARRAYREF SetUpWrapperInfo(MethodDesc *pMD)
         MODE_COOPERATIVE;
         INJECT_FAULT(COMPlusThrowOM());
         PRECONDITION(CheckPointer(pMD));
+        PRECONDITION(!pMD->IsAsyncMethod());
     }
     CONTRACTL_END;
 
@@ -229,13 +230,6 @@ I4ARRAYREF SetUpWrapperInfo(MethodDesc *pMD)
         WrapperTypeArr = (I4ARRAYREF)AllocatePrimitiveArray(ELEMENT_TYPE_I4, numArgs);
 
         GCX_PREEMP();
-        
-        
-        // TODO: (async) revisit and examine if this needs to be supported somehow
-        if (pMD->IsAsyncMethod())
-        {
-            ThrowHR(COR_E_NOTSUPPORTED);
-        }
 
         // Collects ParamDef information in an indexed array where element 0 represents
         // the return type.
@@ -511,11 +505,9 @@ UINT32 CLRToCOMLateBoundWorker(
     LPCUTF8 strMemberName;
     ULONG uSemantic;
 
-    // TODO: (async) revisit and examine if this needs to be supported somehow
-    if (pItfMD->IsAsyncMethod())
-    {
-        ThrowHR(COR_E_NOTSUPPORTED);
-    }
+    // We should never see an async method here, as the async variant should go down
+    // the async stub path and call the non-async variant (which ends up here).
+    _ASSERTE(!pItfMD->IsAsyncMethod());
 
     // See if there is property information for this member.
     hr = pItfMT->GetMDImport()->GetPropertyInfoForMethodDef(pItfMD->GetMemberDef(), &propToken, &strMemberName, &uSemantic);

src/coreclr/vm/comcallablewrapper.cpp

@@ -3313,9 +3313,10 @@ void ComMethodTable::LayOutClassMethodTable()
                 pCurrParentInteropMD = &pCurrParentInteropMT->pVTable[i];
                 pParentMD = pCurrParentInteropMD->pMD;
 
-                if (pMD &&
-                        !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i)) &&
-                    IsOverloadedComVisibleMember(pMD, pParentMD))
+                if (pMD
+                    && !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i))
+                    && IsOverloadedComVisibleMember(pMD, pParentMD)
+                    && !pMD->IsAsyncMethod())
                 {
                     // some bytes are reserved for CALL xxx before the method desc
                     ComCallMethodDesc* pNewMD = (ComCallMethodDesc *) (pMethodDescMemory + COMMETHOD_PREPAD);
@@ -3346,9 +3347,10 @@ void ComMethodTable::LayOutClassMethodTable()
                 pCurrInteropMD = &pCurrInteropMT->pVTable[i];
                 pMD = pCurrInteropMD->pMD;
 
-                if (pMD &&
-                        !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i)) &&
-                    IsNewComVisibleMember(pMD))
+                if (pMD
+                    && !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i))
+                    && IsNewComVisibleMember(pMD)
+                    && !pMD->IsAsyncMethod())
                 {
                     // some bytes are reserved for CALL xxx before the method desc
                     ComCallMethodDesc* pNewMD = (ComCallMethodDesc *) (pMethodDescMemory + COMMETHOD_PREPAD);
@@ -3376,8 +3378,12 @@ void ComMethodTable::LayOutClassMethodTable()
                 if (!it.IsVirtual()) {
                     MethodDesc* pMD = it.GetMethodDesc();
 
-                    if (pMD != NULL && !IsDuplicateClassItfMD(pMD, it.GetSlotNumber()) &&
-                        IsNewComVisibleMember(pMD) && !pMD->IsStatic() && !pMD->IsCtor()
+                    if (pMD != NULL
+                        && !IsDuplicateClassItfMD(pMD, it.GetSlotNumber())
+                        && IsNewComVisibleMember(pMD)
+                        && !pMD->IsStatic()
+                        && !pMD->IsCtor()
+                        && !pMD->IsAsyncMethod()
                         && (!pCurrMT->IsValueType() || (GetClassInterfaceType() != clsIfAutoDual && IsStrictlyUnboxed(pMD))))
                     {
                         // some bytes are reserved for CALL xxx before the method desc
@@ -3558,6 +3564,8 @@ BOOL ComMethodTable::LayOutInterfaceMethodTable(MethodTable* pClsMT)
     ArrayList NewCOMMethodDescs;
     ComCallMethodDescArrayHolder NewCOMMethodDescsHolder(&NewCOMMethodDescs);
 
+    unsigned numVtableSlots = 0;
+
     for (i = 0; i < cbSlots; i++)
     {
         // Some space for a CALL xx xx xx xx stub is reserved before the beginning of the MethodDesc
@@ -3566,6 +3574,15 @@ BOOL ComMethodTable::LayOutInterfaceMethodTable(MethodTable* pClsMT)
 
         MethodDesc* pIntfMD = m_pMT->GetMethodDescForSlot(i);
 
+        if (pIntfMD->IsAsyncMethod())
+        {
+            // Async methods are not supported on COM interfaces
+            // And we don't include them in the calculation of COM vtable slots.
+            continue;
+        }
+
+        numVtableSlots++;
+
         if (m_pMT->HasInstantiation())
         {
             pIntfMD = MethodDesc::FindOrCreateAssociatedMethodDesc(
@@ -3616,23 +3633,36 @@ BOOL ComMethodTable::LayOutInterfaceMethodTable(MethodTable* pClsMT)
         SLOT *pComVtableRW = (SLOT*)((BYTE*)pComVtable + writeableOffset);
 
         // Method descs are at the end of the vtable
-        // m_cbSlots interfaces methods + IUnk methods
+        // numVtableSlots interfaces methods + IUnk methods
+        unsigned cbEmittedSlots = 0;
         pMethodDescMemory = (BYTE *)&pComVtable[m_cbSlots];
+        _ASSERTE(numVtableSlots <= m_cbSlots);
         for (i = 0; i < cbSlots; i++)
         {
             ComCallMethodDesc* pNewMD = (ComCallMethodDesc *) (pMethodDescMemory + COMMETHOD_PREPAD);
             ComCallMethodDesc* pNewMDRW = (ComCallMethodDesc *) (pMethodDescMemory + writeableOffset + COMMETHOD_PREPAD);
 
             MethodDesc* pIntfMD  = m_pMT->GetMethodDescForSlot(i);
 
+            if (pIntfMD->IsAsyncMethod())
+            {
+                // Async methods are not supported on COM interfaces
+                // We skip them above in the vtable calculation
+                // so don't fill in the COM vtable slot here.
+                continue;
+            }
+
             emitCOMStubCall(pNewMD, pNewMDRW, GetEEFuncEntryPoint(ComCallPreStub));
 
             UINT slotIndex = (pIntfMD->GetComSlot() - cbExtraSlots);
             FillInComVtableSlot(pComVtableRW, slotIndex, pNewMD);
 
             pMethodDescMemory += (COMMETHOD_PREPAD + sizeof(ComCallMethodDesc));
+            cbEmittedSlots++;
         }
 
+        _ASSERTE(numVtableSlots == cbEmittedSlots);
+
         // Set the layout complete flag and release the lock.
         comMTWriterHolder.GetRW()->m_Flags |= enum_LayoutComplete;
         NewCOMMethodDescsHolder.SuppressRelease();
@@ -4248,9 +4278,10 @@ ComMethodTable* ComCallWrapperTemplate::CreateComMethodTableForClass(MethodTable
                 pCurrParentInteropMD = &pCurrParentInteropMT->pVTable[i];
                 pParentMD = pCurrParentInteropMD->pMD;
 
-                if (pMD &&
-                    !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i)) &&
-                    IsOverloadedComVisibleMember(pMD, pParentMD))
+                if (pMD
+                    && !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i))
+                    && IsOverloadedComVisibleMember(pMD, pParentMD)
+                    && !pMD->IsAsyncMethod())
                 {
                     cbNewPublicMethods++;
                 }
@@ -4267,9 +4298,10 @@ ComMethodTable* ComCallWrapperTemplate::CreateComMethodTableForClass(MethodTable
                 pCurrInteropMD = &pCurrInteropMT->pVTable[i];
                 pMD = pCurrInteropMD->pMD;
 
-                if (pMD &&
-                        !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i)) &&
-                    IsNewComVisibleMember(pMD))
+                if (pMD
+                    && !(pCurrInteropMD ? IsDuplicateClassItfMD(pCurrInteropMD, i) : IsDuplicateClassItfMD(pMD, i))
+                    && IsNewComVisibleMember(pMD)
+                    && !pMD->IsAsyncMethod())
                 {
                     cbNewPublicMethods++;
                 }
@@ -4282,9 +4314,13 @@ ComMethodTable* ComCallWrapperTemplate::CreateComMethodTableForClass(MethodTable
                 if (!it.IsVirtual())
                 {
                     MethodDesc* pMD = it.GetMethodDesc();
-                        if (pMD && !IsDuplicateClassItfMD(pMD, it.GetSlotNumber()) && IsNewComVisibleMember(pMD) &&
-                        !pMD->IsStatic() && !pMD->IsCtor() &&
-                        (!pCurrMT->IsValueType() || (ClassItfType != clsIfAutoDual && IsStrictlyUnboxed(pMD))))
+                        if (pMD
+                            && !IsDuplicateClassItfMD(pMD, it.GetSlotNumber())
+                            && IsNewComVisibleMember(pMD)
+                            && !pMD->IsStatic()
+                            && !pMD->IsCtor()
+                            && !pMD->IsAsyncMethod()
+                            && (!pCurrMT->IsValueType() || (ClassItfType != clsIfAutoDual && IsStrictlyUnboxed(pMD))))
                     {
                         cbNewPublicMethods++;
                     }

src/coreclr/vm/commtmemberinfomap.cpp

@@ -361,6 +361,7 @@ void ComMTMemberInfoMap::SetupPropsForIClassX(size_t sizeOfPtr)
             // Retrieve the method desc on the current class. This involves looking up the method
             // desc in the vtable if it is a virtual method.
             pMeth = pCMT->GetMethodDescForSlot(i);
+            _ASSERTE(!pMeth->IsAsyncMethod());
             if (pMeth->IsVirtual())
             {
                 WORD wSlot = InteropMethodTableData::GetSlotForMethodDesc(m_pMT, pMeth);
@@ -541,6 +542,15 @@ void ComMTMemberInfoMap::SetupPropsForInterface(size_t sizeOfPtr)
     {
         MethodDesc* pMD = m_pMT->GetMethodDescForSlot(iMD);
         _ASSERTE(pMD != NULL);
+
+        if (pMD->IsAsyncMethod())
+        {
+            // Async methods introduce mismatches in the .NET and COM vtables.
+            // We will need to remap slots.
+            bSlotRemap = true;
+            continue;
+        }
+
         ULONG tmp = pMD->GetComSlot();
 
         if (tmp < ulComSlotMin)
@@ -552,10 +562,13 @@ void ComMTMemberInfoMap::SetupPropsForInterface(size_t sizeOfPtr)
     // Used a couple of times.
     MethodTable::MethodIterator it(m_pMT);
 
-    if (ulComSlotMax-ulComSlotMin >= nSlots)
+    if (ulComSlotMax - ulComSlotMin >= nSlots)
     {
         bSlotRemap = true;
+    }
 
+    if (bSlotRemap)
+    {
         // Resize the array.
         rSlotMap.ReSizeThrows(ulComSlotMax+1);
 
@@ -566,7 +579,7 @@ void ComMTMemberInfoMap::SetupPropsForInterface(size_t sizeOfPtr)
         it.MoveToBegin();
         for (; it.IsValid(); it.Next())
         {
-            if (it.IsVirtual())
+            if (it.IsVirtual() && !it.GetMethodDesc()->IsAsyncMethod())
             {
                 MethodDesc* pMD = it.GetMethodDesc();
                 _ASSERTE(pMD != NULL);
@@ -590,7 +603,7 @@ void ComMTMemberInfoMap::SetupPropsForInterface(size_t sizeOfPtr)
         if (it.IsVirtual())
         {
             pMeth = it.GetMethodDesc();
-            if (pMeth != NULL)
+            if (pMeth != NULL && !pMeth->IsAsyncMethod())
             {
                 ULONG ixSlot = pMeth->GetComSlot();
                 if (bSlotRemap)
@@ -607,6 +620,22 @@ void ComMTMemberInfoMap::SetupPropsForInterface(size_t sizeOfPtr)
     for (iMD=0; iMD < nSlots; ++iMD)
     {
         pMeth = m_MethodProps[iMD].pMeth;
+        if (pMeth == nullptr)
+        {
+            // For async methods, we skip .NET methods when building the COM vtable.
+            // So at some point we hit the end of the .NET methods before we run
+            // through all possible vtable slots.
+            // Record when we ran out here.
+#ifdef _DEBUG
+            // In debug, validate that all remaining slots are null.
+            for (unsigned j = iMD; j < nSlots; ++j)
+            {
+                _ASSERTE(m_MethodProps[j].pMeth == nullptr);
+            }
+#endif
+            nSlots = iMD;
+            break;
+        }
         GetMethodPropsForMeth(pMeth, iMD, m_MethodProps, m_sNames);
     }
 
@@ -688,9 +717,7 @@ void ComMTMemberInfoMap::GetMethodPropsForMeth(
     // Generally don't munge function into a getter.
     rProps[ix].bFunction2Getter = FALSE;
 
-    // TODO: (async) revisit and examine if this needs to be supported somehow
-    if (pMeth->IsAsyncMethod())
-        ThrowHR(COR_E_NOTSUPPORTED);
+    _ASSERTE(!pMeth->IsAsyncMethod());
 
     // See if there is property information for this member.
     hr = pMeth->GetMDImport()->GetPropertyInfoForMethodDef(pMeth->GetMemberDef(), &pd, &pPropName, &uSemantic);
@@ -1608,11 +1635,7 @@ void ComMTMemberInfoMap::PopulateMemberHashtable()
 
             // We are dealing with a method.
             MethodDesc *pMD = pProps->pMeth;
-            // TODO: (async) revisit and examine if this needs to be supported somehow
-            if (pMD->IsAsyncMethod())
-            {
-                ThrowHR(COR_E_NOTSUPPORTED); // Probably this isn't right, and instead should be a skip, but a throw makes it easier to find if this is wrong
-            }
+            _ASSERTE(!pMD->IsAsyncMethod());
             EEModuleTokenPair Key(pMD->GetMemberDef(), pMD->GetModule());
             m_TokenToComMTMethodPropsMap.InsertValue(&Key, (HashDatum)pProps);
         }

src/coreclr/vm/comtoclrcall.cpp

@@ -263,6 +263,8 @@ OBJECTREF COMToCLRGetObjectAndTarget_NonVirtual(ComCallWrapper * pWrap, MethodDe
     }
     CONTRACTL_END;
 
+    CONTRACT_VIOLATION(ThrowsViolation);
+
     //NOTE: No need to optimize for stub dispatch since non-virtuals are retrieved quickly.
     *ppManagedTargetOut = pRealMD->GetSingleCallableAddrOfCode();
 
@@ -850,6 +852,7 @@ void ComCallMethodDesc::InitMethod(MethodDesc *pMD, MethodDesc *pInterfaceMD)
         GC_TRIGGERS;
         MODE_ANY;
         PRECONDITION(CheckPointer(pMD));
+        PRECONDITION(!pMD->IsAsyncMethod());
     }
     CONTRACTL_END;
 
@@ -974,6 +977,7 @@ void ComCallMethodDesc::InitNativeInfo()
         else
         {
             MethodDesc *pMD = GetCallMethodDesc();
+            _ASSERTE(!pMD->IsAsyncMethod()); // Async methods should never have a ComCallMethodDesc.
 
 #ifdef _DEBUG
             LPCUTF8         szDebugName = pMD->m_pszDebugMethodName;
@@ -985,9 +989,6 @@ void ComCallMethodDesc::InitNativeInfo()
 
             MethodTable * pMT = pMD->GetMethodTable();
             IMDInternalImport * pInternalImport = pMT->GetMDImport();
-            // TODO: (async) revisit and examine if this needs to be supported somehow
-            if (pMD->IsAsyncMethod())
-                ThrowHR(COR_E_NOTSUPPORTED);
 
             mdMethodDef md = pMD->GetMemberDef();