Fix COM reentrancy contract violations and re-enable the ExtensionPoints test

src/coreclr/vm/comcallablewrapper.cpp

@@ -292,7 +292,7 @@ ComCallMethodDesc* ComMethodTable::ComCallMethodDescFromSlot(unsigned i)
 //--------------------------------------------------------------------------
 extern "C" PCODE ComPreStubWorker(UMEntryThunkData* pEntryThunk)
 {
-    STATIC_CONTRACT_THROWS;
+    STATIC_CONTRACT_NOTHROW;
     STATIC_CONTRACT_GC_TRIGGERS;
     STATIC_CONTRACT_MODE_ANY;
 
@@ -306,6 +306,11 @@ extern "C" PCODE ComPreStubWorker(UMEntryThunkData* pEntryThunk)
         return pMarshalInfo->GetReturnStubForHResult(E_OUTOFMEMORY);
     }
 
+    // The below "INSTALL_" macros ensure exceptions don't escape,
+    // but the macros do not update the contract state for the thread, so
+    // we manually indicate that here.
+    BEGIN_CONTRACT_VIOLATION(ThrowsViolation);
+
     INSTALL_MANAGED_EXCEPTION_DISPATCHER;
     INSTALL_UNWIND_AND_CONTINUE_HANDLER;
 
@@ -327,6 +332,8 @@ extern "C" PCODE ComPreStubWorker(UMEntryThunkData* pEntryThunk)
     UNINSTALL_UNWIND_AND_CONTINUE_HANDLER;
     UNINSTALL_MANAGED_EXCEPTION_DISPATCHER;
 
+    END_CONTRACT_VIOLATION;
+
     return pStub;
 }
 
@@ -2250,7 +2257,7 @@ static IUnknown * GetComIPFromCCW_ForIID_Worker(
         {
             // Make sure the all the base classes of the class this IClassX corresponds to
             // are visible to COM.
-            pIntfComMT->CheckParentComVisibility(FALSE);
+            pIntfComMT->CheckParentComVisibility();
 
             // Giveout IClassX of this class because the IID matches one of the IClassX in the hierarchy
             // This assumes any IClassX implementation must be derived from base class IClassX's implementation
@@ -2298,7 +2305,7 @@ static IUnknown *GetComIPFromCCW_ForIntfMT_Worker(ComCallWrapper *pWrap, MethodT
             {
                 // Make sure the all the base classes of the class this IClassX corresponds to
                 // are visible to COM.
-                pIntfComMT->CheckParentComVisibility(FALSE);
+                pIntfComMT->CheckParentComVisibility();
 
                 // Giveout IClassX
                 IUnknown * pIntf = pWrap->GetIClassXIP();
@@ -2584,7 +2591,7 @@ IDispatch* ComCallWrapper::GetIDispatchIP()
         // Make sure we release the BasicIP we're about to get.
         SafeComHolder<IUnknown> pBasic = GetBasicIP();
         ComMethodTable* pCMT = ComMethodTable::ComMethodTableFromIP(pBasic);
-        pCMT->CheckParentComVisibility(TRUE);
+        pCMT->CheckParentComVisibility();
     }
 
     // If the class implements IReflect then use the IDispatchEx implementation.
@@ -3789,7 +3796,7 @@ BOOL ComCallWrapperTemplate::IsSafeTypeForMarshalling()
 // Checks to see if the parent of the current class interface is visible to COM.
 // Throws an InvalidOperationException if not.
 //--------------------------------------------------------------------------
-void ComCallWrapperTemplate::CheckParentComVisibility(BOOL fForIDispatch)
+void ComCallWrapperTemplate::CheckParentComVisibility()
 {
     CONTRACTL
     {
@@ -3799,9 +3806,8 @@ void ComCallWrapperTemplate::CheckParentComVisibility(BOOL fForIDispatch)
     }
     CONTRACTL_END;
 
-
     // Throw an exception to report the error.
-    if (!CheckParentComVisibilityNoThrow(fForIDispatch))
+    if (HasInvisibleParent())
     {
         ComCallWrapperTemplate *invisParent = FindInvisibleParent();
         _ASSERTE(invisParent != NULL);
@@ -3814,24 +3820,6 @@ void ComCallWrapperTemplate::CheckParentComVisibility(BOOL fForIDispatch)
     }
 }
 
-BOOL ComCallWrapperTemplate::CheckParentComVisibilityNoThrow(BOOL fForIDispatch)
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        MODE_ANY;
-    }
-    CONTRACTL_END;
-
-
-    // If the parent is visible to COM then everything is ok.
-    if (!HasInvisibleParent())
-        return TRUE;
-
-    return FALSE;
-}
-
 DefaultInterfaceType ComCallWrapperTemplate::GetDefaultInterface(MethodTable **ppDefaultItf)
 {
     CONTRACTL

src/coreclr/vm/comcallablewrapper.h

@@ -226,8 +226,7 @@ class ComCallWrapperTemplate
     ComMethodTable* GetBasicComMT();
     ULONG           GetNumInterfaces();
     SLOT*           GetVTableSlot(ULONG index);
-    void            CheckParentComVisibility(BOOL fForIDispatch);
-    BOOL            CheckParentComVisibilityNoThrow(BOOL fForIDispatch);
+    void            CheckParentComVisibility();
 
     // Calls GetDefaultInterfaceForClassInternal and caches the result.
     DefaultInterfaceType GetDefaultInterface(MethodTable **ppDefaultItf);
@@ -661,18 +660,11 @@ struct ComMethodTable
     }
 #endif // DACCESS_COMPILE
 
-    void CheckParentComVisibility(BOOL fForIDispatch)
+    void CheckParentComVisibility()
     {
         WRAPPER_NO_CONTRACT;
 
-        ((ComCallWrapperTemplate*)m_pMT->GetComCallWrapperTemplate())->CheckParentComVisibility(fForIDispatch);
-    }
-
-    BOOL CheckParentComVisibilityNoThrow(BOOL fForIDispatch)
-    {
-        WRAPPER_NO_CONTRACT;
-
-        return ((ComCallWrapperTemplate*)m_pMT->GetComCallWrapperTemplate())->CheckParentComVisibilityNoThrow(fForIDispatch);
+        ((ComCallWrapperTemplate*)m_pMT->GetComCallWrapperTemplate())->CheckParentComVisibility();
     }
 
 private:

src/coreclr/vm/olevariant.cpp

@@ -1608,6 +1608,7 @@ void OleVariant::ClearLPWSTRArray(void *oleArray, SIZE_T cElements, MethodTable
     LPWSTR *pOle = (LPWSTR *) oleArray;
     LPWSTR *pOleEnd = pOle + cElements;
 
+    PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonRuntimeReentrancy); // IMallocSpy in managed
     while (pOle < pOleEnd)
     {
         LPWSTR lpwstr = *pOle++;
@@ -1743,6 +1744,7 @@ void OleVariant::ClearLPSTRArray(void *oleArray, SIZE_T cElements, MethodTable *
     LPSTR *pOle = (LPSTR *) oleArray;
     LPSTR *pOleEnd = pOle + cElements;
 
+    PERMANENT_CONTRACT_VIOLATION(ThrowsViolation, ReasonRuntimeReentrancy); // IMallocSpy in managed
     while (pOle < pOleEnd)
     {
         LPSTR lpstr = *pOle++;
@@ -4161,4 +4163,3 @@ extern "C" void QCALLTYPE Variant_ConvertValueTypeToRecord(QCall::ObjectHandleOn
     END_QCALL;
 }
 #endif // FEATURE_COMINTEROP
-

src/coreclr/vm/stdinterfaces.cpp

@@ -1348,7 +1348,7 @@ InternalDispatchImpl_GetIDsOfNames (
 
         ComMethodTable* pCMT = ComMethodTable::ComMethodTableFromIP(pDisp);
         if (pCMT->IsIClassXOrBasicItf() && pCMT->GetClassInterfaceType() != clsIfNone)
-            pCMT->CheckParentComVisibility(FALSE);
+            pCMT->CheckParentComVisibility();
 
         pSimpleWrap = pCCW->GetSimpleWrapper();
         pDispInfo = ComMethodTable::ComMethodTableFromIP(pDisp)->GetDispatchInfo();
@@ -1420,7 +1420,7 @@ InternalDispatchImpl_Invoke
 
         ComMethodTable* pCMT = ComMethodTable::ComMethodTableFromIP(pDisp);
         if (pCMT->IsIClassXOrBasicItf() && pCMT->GetClassInterfaceType() != clsIfNone)
-            pCMT->CheckParentComVisibility(FALSE);
+            pCMT->CheckParentComVisibility();
 
         pSimpleWrap = pCCW->GetSimpleWrapper();
 

src/coreclr/vm/stubhelpers.cpp

@@ -476,7 +476,7 @@ extern "C" void QCALLTYPE InterfaceMarshaler_ValidateComVisibilityForIUnknown(IU
 
     if (pComMT->IsIClassX())
     {
-        pComMT->CheckParentComVisibility(FALSE);
+        pComMT->CheckParentComVisibility();
     }
 
     END_QCALL;

src/tests/Interop/COM/ExtensionPoints/ExtensionPoints.cs

@@ -42,7 +42,6 @@ public virtual void PostHeapMinimize() { }
 
     [Fact]
     [Xunit.SkipOnCoreClrAttribute("Depends on marshalled calli", RuntimeTestModes.InterpreterActive)]
-    [ActiveIssue("https://github.com/dotnet/runtime/issues/126567")]
     public static unsafe void Validate_Managed_IMallocSpy()
     {
         Console.WriteLine($"Running {nameof(Validate_Managed_IMallocSpy)}...");