Status and incomplete fat Precode pointer.

experiment_status.md

@@ -1,3 +1,19 @@
+# Status 15-NOV-2024
+
+## Aaron's CoreCLR learnings
+
+- `PCODE` manifests itself from a myriad of places.
+  - The `MethodDesc::DoPrestub()` entry point should be referenced for many of the APIs that can produce `PCODE`. (for example, `GetStubForInteropMethod()`).
+- The ILStubResolver deletes the associated IL post JIT - `ILStubResolver::ClearCompileTimeState`.
+  - This means that when an IL stub will be the interpreted, clean-up of generated IL needs to be avoided.
+    - If a future CoreCLR interpreter uses a converted byte code approach, then getting rid of the IL may not be an issue.
+  - See `CEEInfo::getMethodInfo()` for how IL stubs are impacted, see `MethodDesc::IsWrapperStub()`.
+- Created a new `Precode` type to represent the interpreter entry point.
+  - See `MethodDescCallSite` for the common ways CoreCLR calls from C++ into managed code. Specifically, `CallTargetWorker()` is of note.
+  - The `Precode` approach seems to be a valid way to loop into the execution engine without too much disruption.
+- Passing around stub context to P/Invokes needs to be considered in the interpreter case.
+  - Disabling P/Invoke stub sharing would avoid this need. ReadyToRun is prior art for disabling stub sharing.
+
 # Status 01-NOV-2024
 
 Audit of W^X locations in CoreCLR has yielded locations that need investigation. The section below links to all locations where generated code is manipulated at run-time and indicates a scenario that needs consideration. Jan Vorlicek did a quick check on the list and added a high level thought on relevance to the CoreCLR interpreter track.

src/coreclr/vm/amd64/CallDescrWorkerAMD64.asm

@@ -42,6 +42,11 @@ StackCopyLoop:                          ; copy the arguments to stack top-down t
 
         mov     rax, [rbx + CallDescrData__dwRegTypeMap] ; save the reg (arg) type map
 
+        ; Store stub context
+        cmp     QWORD PTR [rbx + CallDescrData__pStubContextMD], 0
+        jz      NoStubContext
+        mov     r10, [rbx + CallDescrData__pStubContextMD]
+NoStubContext:
         mov     rcx, 0[rsp]             ; load first four argument registers
         movss   xmm0, real4 ptr 0[rsp]  ;
         cmp     al, ASM_ELEMENT_TYPE_R8 ;

src/coreclr/vm/amd64/asmconstants.h

@@ -477,12 +477,14 @@ ASMCONSTANTS_C_ASSERT(OFFSETOF__InlinedCallFrame__m_pThread
 #define CallDescrData__pFloatArgumentRegisters 0x18
 #define CallDescrData__fpReturnSize        0x20
 #define CallDescrData__pTarget             0x28
-#define CallDescrData__returnValue         0x30
+#define CallDescrData__pStubContextMD      0x30
+#define CallDescrData__returnValue         0x38
 #else
 #define CallDescrData__dwRegTypeMap        0x10
 #define CallDescrData__fpReturnSize        0x18
 #define CallDescrData__pTarget             0x20
-#define CallDescrData__returnValue         0x28
+#define CallDescrData__pStubContextMD      0x28
+#define CallDescrData__returnValue         0x30
 #endif
 
 ASMCONSTANTS_C_ASSERT(CallDescrData__pSrc                 == offsetof(CallDescrData, pSrc))
@@ -495,6 +497,7 @@ ASMCONSTANTS_C_ASSERT(CallDescrData__dwRegTypeMap         == offsetof(CallDescrD
 #endif
 ASMCONSTANTS_C_ASSERT(CallDescrData__fpReturnSize         == offsetof(CallDescrData, fpReturnSize))
 ASMCONSTANTS_C_ASSERT(CallDescrData__pTarget              == offsetof(CallDescrData, pTarget))
+ASMCONSTANTS_C_ASSERT(CallDescrData__pStubContextMD       == offsetof(CallDescrData, pStubContextMD))
 ASMCONSTANTS_C_ASSERT(CallDescrData__returnValue          == offsetof(CallDescrData, returnValue))
 
 #ifdef UNIX_AMD64_ABI

src/coreclr/vm/callhelpers.cpp

@@ -33,6 +33,10 @@ void AssertMulticoreJitAllowedModule(PCODE pTarget)
 
 #endif
 
+#ifdef FEATURE_INTERPRETER
+extern void InterpretCallTarget(PCODE pCallTarget, const ARG_SLOT* pArguments, ARG_SLOT* pReturnValue, int cbReturnValue);
+#endif // FEATURE_INTERPRETER
+
 // For X86, INSTALL_COMPLUS_EXCEPTION_HANDLER grants us sufficient protection to call into
 // managed code.
 //
@@ -209,7 +213,7 @@ void * DispatchCallSimple(
         g_pDebugInterface->TraceCall((const BYTE *)pTargetAddress);
 #endif // DEBUGGING_SUPPORTED
 
-    CallDescrData callDescrData;
+    CallDescrData callDescrData{};
 
 #ifdef CALLDESCR_ARGREGS
     callDescrData.pSrc = pSrc + NUM_ARGUMENT_REGISTERS;
@@ -234,18 +238,22 @@ void * DispatchCallSimple(
     callDescrData.fpReturnSize = 0;
     callDescrData.pTarget = pTargetAddress;
 
-    if ((dwDispatchCallSimpleFlags & DispatchCallSimple_CatchHandlerFoundNotification) != 0)
-    {
-        DispatchCallDebuggerWrapper(
-            &callDescrData,
-            dwDispatchCallSimpleFlags & DispatchCallSimple_CriticalCall);
-    }
-    else
-    {
-        CallDescrWorkerWithHandler(&callDescrData, dwDispatchCallSimpleFlags & DispatchCallSimple_CriticalCall);
-    }
-
-    return *(void **)(&callDescrData.returnValue);
+    ARG_SLOT result;
+    InterpretCallTarget(pTargetAddress, pSrc, &result, sizeof(result));
+    return (void*)result;
+
+    //if ((dwDispatchCallSimpleFlags & DispatchCallSimple_CatchHandlerFoundNotification) != 0)
+    //{
+    //    DispatchCallDebuggerWrapper(
+    //        &callDescrData,
+    //        dwDispatchCallSimpleFlags & DispatchCallSimple_CriticalCall);
+    //}
+    //else
+    //{
+    //    CallDescrWorkerWithHandler(&callDescrData, dwDispatchCallSimpleFlags & DispatchCallSimple_CriticalCall);
+    //}
+
+    //return *(void **)(&callDescrData.returnValue);
 }
 
 #ifdef CALLDESCR_REGTYPEMAP
@@ -553,7 +561,7 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *
 
     } // END GCX_FORBID & ENABLE_FORBID_GC_LOADER_USE_IN_THIS_SCOPE
 
-    CallDescrData callDescrData;
+    CallDescrData callDescrData{};
 
     callDescrData.pSrc = pTransitionBlock + sizeof(TransitionBlock);
     _ASSERTE((nStackBytes % TARGET_POINTER_SIZE) == 0);
@@ -574,49 +582,65 @@ void MethodDescCallSite::CallTargetWorker(const ARG_SLOT *pArguments, ARG_SLOT *
     callDescrData.pTarget = m_pCallTarget;
 
 #ifdef FEATURE_INTERPRETER
-    if (transitionToPreemptive)
+    callDescrData.pStubContextMD = m_pMD;
+    if (InterpreterPrecode::IsInstance(m_pCallTarget))
     {
-        GCPreemp transitionIfILStub(transitionToPreemptive);
-        CallDescrWorkerInternal(&callDescrData);
+        if (transitionToPreemptive)
+        {
+            GCPreemp transitionIfILStub(transitionToPreemptive);
+            InterpretCallTarget(m_pCallTarget, pArguments, pReturnValue, cbReturnValue);
+        }
+        else
+        {
+            InterpretCallTarget(m_pCallTarget, pArguments, pReturnValue, cbReturnValue);
+        }
     }
     else
 #endif // FEATURE_INTERPRETER
     {
-        CallDescrWorkerWithHandler(&callDescrData);
-    }
-
-#ifdef FEATURE_HFA
-    if (pvRetBuff != NULL)
-    {
-        memcpyNoGCRefs(pvRetBuff, &callDescrData.returnValue, sizeof(callDescrData.returnValue));
-    }
-#endif // FEATURE_HFA
-
-    if (pReturnValue != NULL)
-    {
-        _ASSERTE((DWORD)cbReturnValue <= sizeof(callDescrData.returnValue));
-#if defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
-        if (callDescrData.fpReturnSize != FpStruct::UseIntCallConv)
+        if (transitionToPreemptive)
         {
-            FpStructInRegistersInfo info = m_argIt.GetReturnFpStructInRegistersInfo();
-            CopyReturnedFpStructFromRegisters(pReturnValue, callDescrData.returnValue, info, false);
+            GCPreemp transitionIfILStub(transitionToPreemptive);
+            CallDescrWorkerInternal(&callDescrData);
         }
         else
-#endif // defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
         {
-            memcpyNoGCRefs(pReturnValue, &callDescrData.returnValue, cbReturnValue);
+            CallDescrWorkerWithHandler(&callDescrData);
         }
 
-#if !defined(HOST_64BIT) && BIGENDIAN
+#ifdef FEATURE_HFA
+        if (pvRetBuff != NULL)
         {
-            GCX_FORBID();
+            memcpyNoGCRefs(pvRetBuff, &callDescrData.returnValue, sizeof(callDescrData.returnValue));
+        }
+#endif // FEATURE_HFA
 
-            if (!m_methodSig.Is64BitReturn())
+        if (pReturnValue != NULL)
+        {
+            _ASSERTE((DWORD)cbReturnValue <= sizeof(callDescrData.returnValue));
+#if defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
+            if (callDescrData.fpReturnSize != FpStruct::UseIntCallConv)
             {
-                pReturnValue[0] >>= 32;
+                FpStructInRegistersInfo info = m_argIt.GetReturnFpStructInRegistersInfo();
+                CopyReturnedFpStructFromRegisters(pReturnValue, callDescrData.returnValue, info, false);
+            }
+            else
+#endif // defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
+            {
+                memcpyNoGCRefs(pReturnValue, &callDescrData.returnValue, cbReturnValue);
+            }
+
+#if !defined(HOST_64BIT) && BIGENDIAN
+            {
+                GCX_FORBID();
+
+                if (!m_methodSig.Is64BitReturn())
+                {
+                    pReturnValue[0] >>= 32;
+                }
             }
-        }
 #endif // !defined(HOST_64BIT) && BIGENDIAN
+        }
     }
 }
 

src/coreclr/vm/callhelpers.h

@@ -29,6 +29,10 @@ struct CallDescrData
     UINT32                      fpReturnSize;
     PCODE                       pTarget;
 
+#ifdef FEATURE_INTERPRETER
+    LPVOID                      pStubContextMD;
+#endif // FEATURE_INTERPRETER
+
 #ifdef CALLDESCR_RETBUFFARGREG
     // Pointer to return buffer arg location
     UINT64*                     pRetBuffArg;

src/coreclr/vm/interpreter.cpp

@@ -13,6 +13,7 @@
 #include "openum.h"
 #include "fcall.h"
 #include "frames.h"
+#include "dllimport.h"
 #include "gcheaputilities.h"
 #include <float.h>
 #include "jitinterface.h"
@@ -10050,7 +10051,12 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T
         }
 
         // Compile the target in advance of calling.
-        if (exactMethToCall->IsPointingToPrestub())
+        if (exactMethToCall->IsNDirect())
+        {
+            GCX_PREEMP();
+            target = GetStubForInteropMethod(exactMethToCall);
+        }
+        else if (exactMethToCall->IsPointingToPrestub())
         {
             MethodTable* dispatchingMT = NULL;
             if (exactMethToCall->IsVtableMethod())
@@ -10087,10 +10093,19 @@ void Interpreter::DoCallWork(bool virtualCall, void* thisArg, CORINFO_RESOLVED_T
             bool b = CycleTimer::GetThreadCyclesS(&startCycles); _ASSERTE(b);
 #endif // INTERP_ILCYCLE_PROFILE
 
+            // If the current method being interpreted is an IL stub, we're calling native code, so
+            // change the GC mode.  (We'll only do this at the call if the calling convention turns out
+            // to be a managed calling convention.)
+            bool transitionToPreemptive = false;
+            {
+                //GCX_PREEMP();
+                //transitionToPreemptive = exactMethToCall != NULL && exactMethToCall->IsNDirect();
+            }
+
 #if defined(UNIX_AMD64_ABI) || defined(TARGET_RISCV64)
-            mdcs.CallTargetWorker(args, retVals, HasTwoSlotBuf ? 16: 8);
+            mdcs.CallTargetWorker(args, retVals, HasTwoSlotBuf ? 16: 8, transitionToPreemptive);
 #else
-            mdcs.CallTargetWorker(args, retVals, 8);
+            mdcs.CallTargetWorker(args, retVals, 8, transitionToPreemptive);
 #endif
 
             if (pCscd != NULL)
@@ -10534,12 +10549,30 @@ void Interpreter::CallI()
     PCODE ftnPtr = OpStackGet<PCODE>(ftnInd);
 
     {
-        MethodDesc* methToCall;
+        MethodDesc* methToCall = NULL;
         // If we're interpreting the target, simply call it directly.
-        if ((methToCall = InterpretationStubToMethodInfo((PCODE)ftnPtr)) != NULL)
+        if (InterpreterPrecode::IsInstance(ftnPtr)
+            || (methToCall = InterpretationStubToMethodInfo((PCODE)ftnPtr)) != NULL)
         {
-            InterpreterMethodInfo* methInfo = MethodHandleToInterpreterMethInfoPtr(CORINFO_METHOD_HANDLE(methToCall));
+            InterpreterMethodInfo* methInfo = methToCall != NULL
+                ? MethodHandleToInterpreterMethInfoPtr(CORINFO_METHOD_HANDLE(methToCall))
+                : NULL;
+
+            // Allocate a new jitInfo and also a new InterpreterMethodInfo.
+            if (methInfo == NULL)
+            {
+                methToCall = InterpreterPrecode::Get(ftnPtr)->GetMethodDesc();
+                CEEInfo* jitInfo = new CEEInfo(methToCall, true);
+
+                CORINFO_METHOD_INFO methInfoLocal;
+
+                GCX_PREEMP();
+                jitInfo->getMethodInfo(CORINFO_METHOD_HANDLE(methToCall), &methInfoLocal, NULL);
+                GenerateInterpreterStub(jitInfo, &methInfoLocal, NULL, 0, &methInfo, true);
+                delete jitInfo;
+            }
             _ASSERTE(methInfo != NULL);
+
 #if INTERP_ILCYCLE_PROFILE
             bool b = CycleTimer::GetThreadCyclesS(&startCycles); _ASSERTE(b);
 #endif // INTERP_ILCYCLE_PROFILE
@@ -12869,4 +12902,32 @@ void Interpreter::PrintILProfile(Interpreter::InstrExecRecord *recs, unsigned in
 }
 #endif // INTERP_ILINSTR_PROFILE
 
+void InterpretCallTarget(PCODE pCallTarget, const ARG_SLOT* pArguments, ARG_SLOT* pReturnValue, int cbReturnValue)
+{
+    _ASSERTE(InterpreterPrecode::IsInstance(pCallTarget));
+
+    InterpreterPrecode* interpPrecode = InterpreterPrecode::Get(pCallTarget);
+
+    MethodDesc* pMD = interpPrecode->GetMethodDesc();
+
+    InterpreterMethodInfo* methInfo;
+    CORINFO_METHOD_INFO jitMethInfo;
+    {
+        CEEInfo* jitInfo = new CEEInfo(pMD, true);
+
+        GCX_PREEMP();
+        if (!jitInfo->getMethodInfo(CORINFO_METHOD_HANDLE(pMD), &jitMethInfo, NULL))
+        {
+            _ASSERTE(false && "getMethodInfo failure");
+        }
+
+        Interpreter::GenerateInterpreterStub(jitInfo, &jitMethInfo, NULL, 0, &methInfo, true);
+        delete jitInfo;
+    }
+
+    ARG_SLOT retVal = Interpreter::InterpretMethodBody(methInfo, true, reinterpret_cast<BYTE*>(const_cast<ARG_SLOT*>(pArguments)), NULL);
+    if (pReturnValue != NULL)
+        *pReturnValue = retVal;
+}
+
 #endif // FEATURE_INTERPRETER

src/coreclr/vm/interpreter.h

@@ -746,9 +746,11 @@ class Interpreter
     friend float F_CALL_CONV InterpretMethodFloat(InterpreterMethodInfo* methInfo, BYTE* ilArgs, void* stubContext);
     friend double F_CALL_CONV InterpretMethodDouble(InterpreterMethodInfo* methInfo, BYTE* ilArgs, void* stubContext);
 
+public:
     // This will be inlined into the bodies of the methods above
     static inline ARG_SLOT InterpretMethodBody(InterpreterMethodInfo* interpMethInfo, bool directCall, BYTE* ilArgs, void* stubContext);
 
+private:
     // The local frame size of the method being interpreted.
     static size_t GetFrameSize(InterpreterMethodInfo* interpMethInfo);
 

src/coreclr/vm/jitinterface.cpp

@@ -7697,7 +7697,7 @@ CEEInfo::getMethodInfo(
         getMethodInfoHelper(cxt, methInfo, context);
         result = true;
     }
-    else if (!ftn->IsWrapperStub() && ftn->HasILHeader())
+    else if (ftn->HasILHeader()) // !ftn->IsWrapperStub() && 
     {
         COR_ILMETHOD_DECODER header(ftn->GetILHeader(), ftn->GetMDImport(), NULL);
         cxt.Header = &header;

src/coreclr/vm/method.cpp

@@ -2658,6 +2658,14 @@ MethodDesc* MethodDesc::GetMethodDescFromStubAddr(PCODE addr, BOOL fSpeculative
 
     MethodDesc* pMD = NULL;
 
+#ifdef FEATURE_INTERPRETER
+    if (InterpreterPrecode::IsInstance(addr))
+    {
+        pMD = InterpreterPrecode::Get(addr)->GetMethodDesc();
+        RETURN(pMD);
+    }
+#endif // FEATURE_INTERPRETER
+
     // Otherwise this must be some kind of precode
     //
     PTR_Precode pPrecode = Precode::GetPrecodeFromEntryPoint(addr, fSpeculative);
@@ -3879,7 +3887,9 @@ PrecodeType MethodDesc::GetPrecodeType()
     LIMITED_METHOD_CONTRACT;
 
     PrecodeType precodeType = PRECODE_INVALID;
-
+#ifdef FEATURE_INTERPRETER
+    precodeType = PRECODE_INTERPRETER;
+#else // !FEATURE_INTERPRETER
 #ifdef HAS_FIXUP_PRECODE
     if (!RequiresMethodDescCallingConvention())
     {
@@ -3891,6 +3901,7 @@ PrecodeType MethodDesc::GetPrecodeType()
     {
         precodeType = PRECODE_STUB;
     }
+#endif // FEATURE_INTERPRETER
 
     return precodeType;
 }