Fix unboxing stubs on wasm (#120153)

* Fix unboxing stubs on wasm

* Apply suggestions from code review

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>

* Feedback

Remove GenerateShuffleArray and s_pShuffleThunkCache for FEATURE_PORTABLE_ENTRYPOINTS

* Feedback

Handle all pStubs for FEATURE_PORTABLE_ENTRYPOINTS

* Do not double free

* Add thunk for the IL stub

* Update src/coreclr/vm/wasm/helpers.cpp

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

* Update src/coreclr/vm/prestub.cpp

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

* Finish the feedback

* Feedback

* Feedback

* Feedback

* Feedback

---------

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Aaron Robinson <arobins@microsoft.com>

Author: 3 people

src/coreclr/clrdefinitions.cmake

@@ -60,14 +60,6 @@ if(CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64)
 add_compile_definitions(OUT_OF_PROCESS_SETTHREADCONTEXT)
 endif(CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64)
 
-if(NOT CLR_CMAKE_TARGET_ARCH_I386)
-  add_definitions(-DFEATURE_PORTABLE_SHUFFLE_THUNKS)
-endif()
-
-if(CLR_CMAKE_TARGET_UNIX OR NOT CLR_CMAKE_TARGET_ARCH_I386)
-  add_definitions(-DFEATURE_INSTANTIATINGSTUB_AS_IL)
-endif()
-
 add_compile_definitions(FEATURE_CODE_VERSIONING)
 add_definitions(-DFEATURE_COLLECTIBLE_TYPES)
 

src/coreclr/inc/switches.h

@@ -165,6 +165,14 @@
 #define CHAIN_LOOKUP
 #endif // FEATURE_VIRTUAL_STUB_DISPATCH
 
+#if !defined(FEATURE_PORTABLE_ENTRYPOINTS) && !defined(TARGET_X86)
+#define FEATURE_PORTABLE_SHUFFLE_THUNKS
+#endif
+
+#if defined(TARGET_UNIX) || !defined(TARGET_X86)
+#define FEATURE_INSTANTIATINGSTUB_AS_IL
+#endif
+
 // If this is uncommented, leaves a file "StubLog_<pid>.log" with statistics on the behavior
 // of stub-based interface dispatch.
 //#define STUB_LOGGING

src/coreclr/vm/comdelegate.cpp

@@ -635,6 +635,7 @@ BOOL GenerateShuffleArrayPortable(MethodDesc* pMethodSrc, MethodDesc *pMethodDst
 }
 #endif // FEATURE_PORTABLE_SHUFFLE_THUNKS
 
+#ifndef FEATURE_PORTABLE_ENTRYPOINTS
 BOOL GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, SArray<ShuffleEntry> * pShuffleEntryArray)
 {
     STANDARD_VM_CONTRACT;
@@ -773,8 +774,8 @@ BOOL GenerateShuffleArray(MethodDesc* pInvoke, MethodDesc *pTargetMeth, SArray<S
     }
     return TRUE;
 }
-
 static ShuffleThunkCache* s_pShuffleThunkCache = NULL;
+#endif // !FEATURE_PORTABLE_ENTRYPOINTS
 
 // One time init.
 void COMDelegate::Init()
@@ -786,8 +787,9 @@ void COMDelegate::Init()
         MODE_ANY;
     }
     CONTRACTL_END;
-
+#ifndef FEATURE_PORTABLE_ENTRYPOINTS
     s_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetStubHeap());
+#endif
 }
 
 #ifdef FEATURE_COMINTEROP
@@ -913,6 +915,7 @@ static PCODE SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth)
 
     MethodDesc *pMD = pClass->GetInvokeMethod();
 
+#ifndef FEATURE_PORTABLE_ENTRYPOINTS
     // We haven't already setup a shuffle thunk, go do it now (which will cache the result automatically).
     StackSArray<ShuffleEntry> rShuffleEntryArray;
     if (GenerateShuffleArray(pMD, pTargetMeth, &rShuffleEntryArray))
@@ -928,6 +931,7 @@ static PCODE SetupShuffleThunk(MethodTable * pDelMT, MethodDesc *pTargetMeth)
         pShuffleThunk = pShuffleThunkCache->Canonicalize((const BYTE *)&rShuffleEntryArray[0], "DelegateShuffleThunk");
     }
     else
+#endif // !FEATURE_PORTABLE_ENTRYPOINTS
     {
 #if defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
         pShuffleThunk = CreateILDelegateShuffleThunk(pMD, isInstRetBuff);

src/coreclr/vm/comdelegate.h

@@ -170,6 +170,7 @@ struct ShuffleEntry
 
 #include <poppack.h>
 
+#ifndef FEATURE_PORTABLE_ENTRYPOINTS
 class ShuffleThunkCache : public StubCacheBase
 {
 public:
@@ -204,5 +205,6 @@ class ShuffleThunkCache : public StubCacheBase
         return sizeof(ShuffleEntry) * (UINT)(1 + (pse - (ShuffleEntry*)pRawStub));
     }
 };
+#endif // !FEATURE_PORTABLE_ENTRYPOINTS
 
 #endif  // _COMDELEGATE_H_

src/coreclr/vm/loaderallocator.cpp

@@ -1767,6 +1767,7 @@ void AssemblyLoaderAllocator::Init()
     m_dependentHandleToNativeObjectSetCrst.Init(CrstLeafLock, CRST_UNSAFE_ANYMODE);
 
     LoaderAllocator::Init(NULL /*pExecutableHeapMemory*/);
+#ifndef FEATURE_PORTABLE_ENTRYPOINTS
     if (IsCollectible())
     {
         // TODO: the ShuffleThunkCache should really be using the m_pStubHeap, however the unloadability support
@@ -1775,6 +1776,7 @@ void AssemblyLoaderAllocator::Init()
         // https://github.com/dotnet/runtime/issues/55697 tracks this issue.
         m_pShuffleThunkCache = new ShuffleThunkCache(SystemDomain::GetGlobalLoaderAllocator()->GetExecutableHeap());
     }
+#endif // !FEATURE_PORTABLE_ENTRYPOINTS
 }
 
 
@@ -1786,8 +1788,10 @@ AssemblyLoaderAllocator::~AssemblyLoaderAllocator()
         m_binderToRelease = NULL;
     }
 
+#ifndef FEATURE_PORTABLE_ENTRYPOINTS
     delete m_pShuffleThunkCache;
     m_pShuffleThunkCache = NULL;
+#endif // !FEATURE_PORTABLE_ENTRYPOINTS
 }
 
 void AssemblyLoaderAllocator::RegisterBinder(CustomAssemblyBinder* binderToRelease)

src/coreclr/vm/prestub.cpp

@@ -1459,7 +1459,7 @@ void MethodDesc::CreateDerivedTargetSigWithExtraParams(MetaSig& msig, SigBuilder
 
 #ifdef FEATURE_INSTANTIATINGSTUB_AS_IL
 
-Stub * CreateUnboxingILStubForSharedGenericValueTypeMethods(MethodDesc* pTargetMD)
+Stub * CreateUnboxingILStubForValueTypeMethods(MethodDesc* pTargetMD)
 {
 
     CONTRACT(Stub*)
@@ -1510,13 +1510,16 @@ Stub * CreateUnboxingILStubForSharedGenericValueTypeMethods(MethodDesc* pTargetM
     }
 #endif
 
-    // Push the hidden context param
-    // The context is going to be captured from the thisptr
-    pCode->EmitLoadThis();
-    pCode->EmitLDFLDA(tokRawData);
-    pCode->EmitLDC(Object::GetOffsetOfFirstField());
-    pCode->EmitSUB();
-    pCode->EmitLDIND_I();
+    if (pTargetMD->RequiresInstMethodTableArg())
+    {
+        // Push the hidden context param
+        // The context is going to be captured from the thisptr
+        pCode->EmitLoadThis();
+        pCode->EmitLDFLDA(tokRawData);
+        pCode->EmitLDC(Object::GetOffsetOfFirstField());
+        pCode->EmitSUB();
+        pCode->EmitLDIND_I();
+    }
 
 #ifndef TARGET_X86
     if (msig.HasAsyncContinuation())
@@ -1727,13 +1730,16 @@ Stub * MakeUnboxingStubWorker(MethodDesc *pMD)
     else
 #endif
     {
+#ifdef FEATURE_PORTABLE_ENTRYPOINTS
+        pstub = CreateUnboxingILStubForValueTypeMethods(pUnboxedMD);
+#else // !FEATURE_PORTABLE_ENTRYPOINTS
 #ifdef FEATURE_INSTANTIATINGSTUB_AS_IL
 #ifndef FEATURE_PORTABLE_SHUFFLE_THUNKS
         if (pUnboxedMD->RequiresInstMethodTableArg())
 #endif // !FEATURE_PORTABLE_SHUFFLE_THUNKS
         {
             _ASSERTE(pUnboxedMD->RequiresInstMethodTableArg());
-            pstub = CreateUnboxingILStubForSharedGenericValueTypeMethods(pUnboxedMD);
+            pstub = CreateUnboxingILStubForValueTypeMethods(pUnboxedMD);
         }
 #ifndef FEATURE_PORTABLE_SHUFFLE_THUNKS
         else
@@ -1746,6 +1752,7 @@ Stub * MakeUnboxingStubWorker(MethodDesc *pMD)
             pstub = sl.Link(pMD->GetLoaderAllocator()->GetStubHeap(), NEWSTUB_FL_NONE, "UnboxingStub");
         }
 #endif // !FEATURE_PORTABLE_SHUFFLE_THUNKS
+#endif // FEATURE_PORTABLE_ENTRYPOINTS
     }
     RETURN pstub;
 }
@@ -2377,6 +2384,15 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo
     }
     else
     {
+#ifdef FEATURE_PORTABLE_ENTRYPOINTS
+        pCode = pStub->GetEntryPoint();
+        pStub->DecRef();
+
+        void* ilStubInterpData = PortableEntryPoint::GetInterpreterData(pCode);
+        _ASSERTE(ilStubInterpData != NULL);
+        SetInterpreterCode((InterpByteCodeStart*)ilStubInterpData);
+        SetCodeEntryPoint(pCode);
+#else
         if (!GetOrCreatePrecode()->SetTargetInterlocked(pStub->GetEntryPoint()))
         {
             if (pStub->HasExternalEntryPoint())
@@ -2396,12 +2412,12 @@ PCODE MethodDesc::DoPrestub(MethodTable *pDispatchingMT, CallerGCMode callerGCMo
             // need to free the Stub allocation now.
             pStub->DecRef();
         }
+#endif // !FEATURE_PORTABLE_ENTRYPOINTS
     }
 
     _ASSERTE(!IsPointingToPrestub());
     _ASSERTE(HasStableEntryPoint());
 
-
     pCode = DoBackpatch(pMT, pDispatchingMT, FALSE);
 
 Return:

src/coreclr/vm/wasm/cgencpu.h

@@ -74,12 +74,6 @@ class StubLinkerCPU : public StubLinker
 {
 public:
     static void Init() { /* no-op on wasm */ }
-    inline void EmitShuffleThunk(struct ShuffleEntry *pShuffleEntryArray) {
-        _ASSERTE("The EmitShuffleThunk is not implemented on wasm");
-    }
-    inline VOID EmitComputedInstantiatingMethodStub(MethodDesc* pSharedMD, struct ShuffleEntry *pShuffleEntryArray, void* extraArg) {
-        _ASSERTE("The EmitComputedInstantiatingMethodStub is not implemented on wasm");
-    }
 };
 
 //**********************************************************************

src/coreclr/vm/wasm/helpers.cpp

@@ -553,6 +553,12 @@ namespace
         *(int32_t*)pRet = (*fptr)(ARG_IND(0), ARG(1));
     }
 
+    void CallFunc_I32_I32IND_I32_I32IND_I32_RetI32(PCODE pcode, int8_t *pArgs, int8_t *pRet)
+    {
+        int32_t (*fptr)(int32_t, int32_t, int32_t, int32_t, int32_t) = (int32_t (*)(int32_t, int32_t, int32_t, int32_t, int32_t))pcode;
+        *(int32_t*)pRet = (*fptr)(ARG(0), ARG_IND(1), ARG(2), ARG_IND(3), ARG(4));
+    }
+
     void CallFunc_I32IND_I32_I32_I32_I32_I32_RetI32(PCODE pcode, int8_t *pArgs, int8_t *pRet)
     {
         int32_t (*fptr)(int32_t, int32_t, int32_t, int32_t, int32_t, int32_t) = (int32_t (*)(int32_t, int32_t, int32_t, int32_t, int32_t, int32_t))pcode;
@@ -690,6 +696,16 @@ namespace
                         return (void*)&CallFunc_I32IND_I32_RetI32;
                     }
                     break;
+                case 5:
+                    if (args[0] == ConvertType::ToI32 &&
+                        args[1] == ConvertType::ToI32Indirect &&
+                        args[2] == ConvertType::ToI32 &&
+                        args[3] == ConvertType::ToI32Indirect &&
+                        args[4] == ConvertType::ToI32)
+                    {
+                        return (void*)&CallFunc_I32_I32IND_I32_I32IND_I32_RetI32;
+                    }
+                    break;
                 case 6:
                     if (args[0] == ConvertType::ToI32Indirect &&
                         args[1] == ConvertType::ToI32 &&