From abec533722ed00da3c636c08ba72ad2addb7f651 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 7 May 2025 23:13:00 -0700 Subject: [PATCH 1/8] Squashed, revised PInvoke implementation Marshaling wrappers work so enable test for them Separate call opcode for unmarshaled pinvokes Cleanup Do inlined pinvoke for CALL_PINVOKE opcode Update pinvoke test for new behavior --- src/coreclr/interpreter/compiler.cpp | 12 ++- src/coreclr/interpreter/intops.def | 1 + src/coreclr/vm/dllimport.cpp | 18 ++++- src/coreclr/vm/interpexec.cpp | 27 ++++++- src/tests/JIT/interpreter/CMakeLists.txt | 11 +++ src/tests/JIT/interpreter/Interpreter.cs | 80 +++++++++++++++++++ src/tests/JIT/interpreter/Interpreter.csproj | 3 + .../JIT/interpreter/InterpreterTester.csproj | 3 + src/tests/JIT/interpreter/pinvoke.cpp | 23 ++++++ 9 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 src/tests/JIT/interpreter/CMakeLists.txt create mode 100644 src/tests/JIT/interpreter/pinvoke.cpp diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 5c573ff0a53359..2ae6207a685ae4 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2821,6 +2821,9 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re doCallInsteadOfNew = true; } + bool isPInvoke = callInfo.methodFlags & CORINFO_FLG_PINVOKE; + bool isMarshaledPInvoke = isPInvoke && m_compHnd->pInvokeMarshalingRequired(callInfo.hMethod, &callInfo.sig); + // Process sVars int numArgsFromStack = callInfo.sig.numArgs + (newObj ? 0 : callInfo.sig.hasThis()); int newObjThisArgLocation = newObj && !doCallInsteadOfNew ? 0 : INT_MAX; @@ -3057,8 +3060,15 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re // before the call. // TODO: Add null checking behavior somewhere here! } - AddIns(INTOP_CALL); + AddIns((isPInvoke && !isMarshaledPInvoke) ? INTOP_CALL_PINVOKE : INTOP_CALL); m_pLastNewIns->data[0] = GetMethodDataItemIndex(callInfo.hMethod); + if (isPInvoke && !isMarshaledPInvoke) + { + CORINFO_CONST_LOOKUP lookup; + m_compHnd->getAddressOfPInvokeTarget(callInfo.hMethod, &lookup); + assert(lookup.accessType == IAT_PVALUE); + m_pLastNewIns->data[1] = GetDataItemIndex(lookup.addr); + } } break; diff --git a/src/coreclr/interpreter/intops.def b/src/coreclr/interpreter/intops.def index 2879e692084ebe..d0b2d4428378d8 100644 --- a/src/coreclr/interpreter/intops.def +++ b/src/coreclr/interpreter/intops.def @@ -358,6 +358,7 @@ OPDEF(INTOP_LDFLDA, "ldflda", 4, 1, 1, InterpOpInt) OPDEF(INTOP_CALL, "call", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLI, "calli", 5, 1, 2, InterpOpLdPtr) OPDEF(INTOP_CALLVIRT, "callvirt", 4, 1, 1, InterpOpMethodHandle) +OPDEF(INTOP_CALL_PINVOKE, "call.pinvoke", 5, 1, 1, InterpOpMethodHandle) // inlined (no marshaling wrapper) pinvokes only OPDEF(INTOP_NEWOBJ, "newobj", 5, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_NEWOBJ_GENERIC, "newobj.generic", 6, 1, 2, InterpOpMethodHandle) OPDEF(INTOP_NEWOBJ_VT, "newobj.vt", 5, 1, 1, InterpOpMethodHandle) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index ace24da99e6849..163278b6a0735c 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5886,8 +5886,22 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD) // INDEBUG(Thread *pThread = GetThread()); { - _ASSERTE((pThread->GetFrame() != FRAME_TOP && pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) - || pMD->ShouldSuppressGCTransition()); +#ifdef FEATURE_INTERPRETER + _ASSERTE( + ( + (pThread->GetFrame() != FRAME_TOP) && ( + (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) || + (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) + ) + ) || pMD->ShouldSuppressGCTransition() + ); +#else + _ASSERTE( + ( + (pThread->GetFrame() != FRAME_TOP) && (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) + ) || pMD->ShouldSuppressGCTransition() + ); +#endif CONSISTENCY_CHECK(pMD->IsNDirect()); // diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index ec1bc2da43aa31..1f79f044dd7e6a 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -44,7 +44,10 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet) } } - pHeader->SetTarget(pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); // The method to call + if (overrideTarget) + pHeader->SetTarget(overrideTarget); + else + pHeader->SetTarget(pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); // The method to call pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); } @@ -1827,6 +1830,28 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr break; } + case INTOP_CALL_PINVOKE: + { + // This opcode handles p/invokes that don't use a managed wrapper for marshaling. These + // calls are special in that they need an InlinedCallFrame in order for proper EH to happen + + returnOffset = ip[1]; + callArgsOffset = ip[2]; + methodSlot = ip[3]; + int32_t targetAddrSlot = ip[4]; + + ip += 5; + targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; + PCODE callTarget = *(PCODE*)pMethod->pDataItems[targetAddrSlot]; + + { + // Interpreter-FIXME: Create InlinedCallFrame. + GCX_PREEMP(); + InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); + } + break; + } + case INTOP_CALL: { returnOffset = ip[1]; diff --git a/src/tests/JIT/interpreter/CMakeLists.txt b/src/tests/JIT/interpreter/CMakeLists.txt new file mode 100644 index 00000000000000..8370c4824a523b --- /dev/null +++ b/src/tests/JIT/interpreter/CMakeLists.txt @@ -0,0 +1,11 @@ +project (pinvoke) + +include_directories(${INC_PLATFORM_DIR}) + +set(SOURCES pinvoke.cpp) + +# add the executable +add_library (pinvoke SHARED ${SOURCES}) + +# add the install targets +install (TARGETS pinvoke DESTINATION bin) diff --git a/src/tests/JIT/interpreter/Interpreter.cs b/src/tests/JIT/interpreter/Interpreter.cs index f87bb14a405aa5..68cea1ca5de4ea 100644 --- a/src/tests/JIT/interpreter/Interpreter.cs +++ b/src/tests/JIT/interpreter/Interpreter.cs @@ -969,6 +969,11 @@ public static void RunInterpreterTests() Console.WriteLine("IntPtr.Zero: {0}, UIntPtr.Zero: {1}", IntPtr.Zero, UIntPtr.Zero); + Console.WriteLine("TestPInvoke"); + if (!TestPInvoke()) + Environment.FailFast(null); + + // For stackwalking validation System.GC.Collect(); Console.WriteLine("All tests passed successfully!"); @@ -2383,6 +2388,81 @@ static object BoxedSubtraction(object lhs, object rhs) return (int)lhs - (int)rhs; } + [DllImport("pinvoke", CallingConvention = CallingConvention.Cdecl)] + public static extern int sumTwoInts(int x, int y); + [DllImport("pinvoke", CallingConvention = CallingConvention.Cdecl)] + public static extern double sumTwoDoubles(double x, double y); + [DllImport("pinvoke", CallingConvention = CallingConvention.Cdecl, CharSet = CharSet.Ansi)] + public static extern int writeToStdout(string s); + [DllImport("missingLibrary", CallingConvention = CallingConvention.Cdecl)] + public static extern void missingPInvoke(); + [DllImport("missingLibrary", CallingConvention = CallingConvention.Cdecl)] + public static extern void missingPInvokeWithMarshaling(string s); + + public static bool TestPInvoke() + { + if (sumTwoInts(1, 2) != 3) + return false; + + double summed = sumTwoDoubles(1, 2); + if (summed != 3) + return false; + + // Test marshaling wrappers + writeToStdout("Hello world from pinvoke.dll!writeToStdout\n"); + + bool caught = false; + try { + Console.WriteLine("calling missingPInvoke"); + missingPInvoke(); + return false; + } catch (DllNotFoundException) { + Console.WriteLine("caught #1"); + caught = true; + } + + if (!caught) + return false; + + /* fails, with output: +calling missingPInvokeWithMarshaling +caught #2 + +Assert failure(PID 74448 [0x000122d0], Thread: 61928 [0xf1e8]): ohThrowable + +CORECLR! PreStubWorker$catch$10 + 0x9E (0x00007ffc`5a0e31fe) +CORECLR! CallSettingFrame_LookupContinuationIndex + 0x20 (0x00007ffc`59f7eeb0) +CORECLR! _FrameHandler4::CxxCallCatchBlock + 0x1DE (0x00007ffc`59f6a7fe) +NTDLL! RtlCaptureContext2 + 0x4A6 (0x00007ffd`45ac6c56) +CORECLR! PreStubWorker + 0x503 (0x00007ffc`598d68b3) +CORECLR! ThePreStub + 0x55 (0x00007ffc`59f0de25) +CORECLR! CallJittedMethodRetVoid + 0x14 (0x00007ffc`59f0c394) +CORECLR! InvokeCompiledMethod + 0x5CD (0x00007ffc`59b376fd) +CORECLR! InterpExecMethod + 0x7286 (0x00007ffc`59b33056) +CORECLR! ExecuteInterpretedMethod + 0x11B (0x00007ffc`598d528b) + File: Z:\runtime\src\coreclr\vm\prestub.cpp:1972 + Image: Z:\runtime\artifacts\tests\coreclr\windows.x64.Debug\Tests\Core_Root\corerun.exe + +Interpreted App returned -1073740286 + */ + /* + bool caught2 = false; + try { + Console.WriteLine("calling missingPInvokeWithMarshaling"); + missingPInvokeWithMarshaling("test"); + return false; + } catch (DllNotFoundException) { + Console.WriteLine("caught #2"); + caught2 = true; + } + + if (!caught2) + return false; + */ + + return true; + } + public static bool TestArray() { // sbyte diff --git a/src/tests/JIT/interpreter/Interpreter.csproj b/src/tests/JIT/interpreter/Interpreter.csproj index 98b0af809b2826..567503852c60ca 100644 --- a/src/tests/JIT/interpreter/Interpreter.csproj +++ b/src/tests/JIT/interpreter/Interpreter.csproj @@ -8,4 +8,7 @@ + + + diff --git a/src/tests/JIT/interpreter/InterpreterTester.csproj b/src/tests/JIT/interpreter/InterpreterTester.csproj index 4bb141749f2885..e654b46510baa1 100644 --- a/src/tests/JIT/interpreter/InterpreterTester.csproj +++ b/src/tests/JIT/interpreter/InterpreterTester.csproj @@ -13,6 +13,9 @@ + + + diff --git a/src/tests/JIT/interpreter/pinvoke.cpp b/src/tests/JIT/interpreter/pinvoke.cpp new file mode 100644 index 00000000000000..149a902e6110fa --- /dev/null +++ b/src/tests/JIT/interpreter/pinvoke.cpp @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "stdio.h" +#include +#include + +#define EXPORT_IT extern "C" DLL_EXPORT + +EXPORT_IT void writeToStdout (const char *str) +{ + puts(str); +} + +EXPORT_IT int sumTwoInts (int x, int y) +{ + return x + y; +} + +EXPORT_IT double sumTwoDoubles (double x, double y) +{ + return x + y; +} From 4187c57db77406ef3ff8a045543920a0e716a3ae Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Tue, 24 Jun 2025 13:50:22 -0700 Subject: [PATCH 2/8] Repair merge damage --- src/coreclr/vm/interpexec.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 1f79f044dd7e6a..40887382cb791d 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -11,7 +11,7 @@ // for numeric_limits #include -void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet) +void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE overrideTarget = NULL) { CONTRACTL { @@ -45,6 +45,7 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet) } if (overrideTarget) + // Interpreter-FIXME: Is this a race condition? pHeader->SetTarget(overrideTarget); else pHeader->SetTarget(pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); // The method to call From 84e321aa3a1cd648466d36f733253025fb104d38 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 10 Jul 2025 05:06:15 -0700 Subject: [PATCH 3/8] Add InlinedCallFrame and remove special-casing for InterpreterFrame callers in NDirectImportWorker --- src/coreclr/vm/dllimport.cpp | 11 ----------- src/coreclr/vm/interpexec.cpp | 12 ++++++++++++ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 163278b6a0735c..52a0f803895a7c 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5886,22 +5886,11 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD) // INDEBUG(Thread *pThread = GetThread()); { -#ifdef FEATURE_INTERPRETER - _ASSERTE( - ( - (pThread->GetFrame() != FRAME_TOP) && ( - (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) || - (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InterpreterFrame) - ) - ) || pMD->ShouldSuppressGCTransition() - ); -#else _ASSERTE( ( (pThread->GetFrame() != FRAME_TOP) && (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) ) || pMD->ShouldSuppressGCTransition() ); -#endif CONSISTENCY_CHECK(pMD->IsNDirect()); // diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 40887382cb791d..77faa81226f2c0 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -7,6 +7,7 @@ #include "gcenv.h" #include "interpexec.h" #include "callstubgenerator.h" +#include "frames.h" // for numeric_limits #include @@ -1845,11 +1846,22 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; PCODE callTarget = *(PCODE*)pMethod->pDataItems[targetAddrSlot]; + InlinedCallFrame inlinedCallFrame; + inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip; + inlinedCallFrame.m_pCallSiteSP = stack; + inlinedCallFrame.m_pCalleeSavedFP = (TADDR)pFrame; + inlinedCallFrame.m_pThread = GetThread(); + inlinedCallFrame.m_Datum = NULL; + inlinedCallFrame.Push(); + { // Interpreter-FIXME: Create InlinedCallFrame. GCX_PREEMP(); InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); } + + inlinedCallFrame.Pop(); + break; } From 6227cf69f623e0e1cdf3a5efbe81f6d9ed4d55f5 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 10 Jul 2025 16:37:20 -0700 Subject: [PATCH 4/8] Switch from 'indirect helper tag' to 'direct helper tag' so tagged helpers are compatible with arm32 thumb Checkpoint pinvoke changes --- src/coreclr/interpreter/compiler.cpp | 18 ++++--- src/coreclr/interpreter/interpretershared.h | 4 +- src/coreclr/interpreter/intops.def | 2 +- src/coreclr/vm/interpexec.cpp | 18 ++++--- src/tests/JIT/interpreter/Interpreter.cs | 56 ++++++++++++++------- 5 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 2ae6207a685ae4..373c26bffee711 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2163,9 +2163,11 @@ int32_t InterpCompiler::GetDataItemIndexForHelperFtn(CorInfoHelpFunc ftn) CORINFO_CONST_LOOKUP ftnLookup; m_compHnd->getHelperFtn(ftn, &ftnLookup); void* addr = ftnLookup.addr; - if (ftnLookup.accessType == IAT_PVALUE) + if (ftnLookup.accessType != IAT_PVALUE) { - addr = (void*)((size_t)addr | INTERP_INDIRECT_HELPER_TAG); + // We can't use the 1 bit to mark indirect addresses because it is used for real code on arm32 (the thumb bit) + // So instead, we mark direct addresses with a 1 and then on the other end we will clear the 1 and re-set it as needed for thumb + addr = (void*)((size_t)addr | INTERP_DIRECT_HELPER_TAG); } #ifdef DEBUG @@ -3066,8 +3068,8 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re { CORINFO_CONST_LOOKUP lookup; m_compHnd->getAddressOfPInvokeTarget(callInfo.hMethod, &lookup); - assert(lookup.accessType == IAT_PVALUE); m_pLastNewIns->data[1] = GetDataItemIndex(lookup.addr); + m_pLastNewIns->data[2] = lookup.accessType == IAT_PVALUE; } } break; @@ -6306,13 +6308,15 @@ void InterpCompiler::PrintHelperFtn(void* helperDirectOrIndirect) { void* helperAddr = helperDirectOrIndirect; - if (((size_t)helperDirectOrIndirect) & INTERP_INDIRECT_HELPER_TAG) + if (((size_t)helperDirectOrIndirect) & INTERP_DIRECT_HELPER_TAG) { - helperAddr = (void*)(((size_t)helperDirectOrIndirect) & ~INTERP_INDIRECT_HELPER_TAG); - printf(" (indirect)"); + helperAddr = (void*)(((size_t)helperDirectOrIndirect) & ~INTERP_DIRECT_HELPER_TAG); + printf(" (direct)"); } else - printf(" (direct)"); + { + printf(" (indirect)"); + } PrintPointer(helperAddr); } diff --git a/src/coreclr/interpreter/interpretershared.h b/src/coreclr/interpreter/interpretershared.h index 388fa3502e8065..210bc1c1ad36db 100644 --- a/src/coreclr/interpreter/interpretershared.h +++ b/src/coreclr/interpreter/interpretershared.h @@ -17,7 +17,7 @@ #define INTERP_STACK_SLOT_SIZE 8 // Alignment of each var offset on the interpreter stack #define INTERP_STACK_ALIGNMENT 16 // Alignment of interpreter stack at the start of a frame -#define INTERP_INDIRECT_HELPER_TAG 1 // When a helper ftn's address is indirect we tag it with this tag bit +#define INTERP_DIRECT_HELPER_TAG 1 // When a helper ftn's address is direct we tag it with this tag bit struct CallStubHeader; @@ -137,7 +137,7 @@ struct InterpGenericLookup void* signature; // Here is the helper you must call. It is one of CORINFO_HELP_RUNTIMEHANDLE_* helpers. - + InterpGenericLookupType lookupType; // Number of indirections to get there diff --git a/src/coreclr/interpreter/intops.def b/src/coreclr/interpreter/intops.def index d0b2d4428378d8..8de811f7b52c1c 100644 --- a/src/coreclr/interpreter/intops.def +++ b/src/coreclr/interpreter/intops.def @@ -358,7 +358,7 @@ OPDEF(INTOP_LDFLDA, "ldflda", 4, 1, 1, InterpOpInt) OPDEF(INTOP_CALL, "call", 4, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_CALLI, "calli", 5, 1, 2, InterpOpLdPtr) OPDEF(INTOP_CALLVIRT, "callvirt", 4, 1, 1, InterpOpMethodHandle) -OPDEF(INTOP_CALL_PINVOKE, "call.pinvoke", 5, 1, 1, InterpOpMethodHandle) // inlined (no marshaling wrapper) pinvokes only +OPDEF(INTOP_CALL_PINVOKE, "call.pinvoke", 6, 1, 1, InterpOpMethodHandle) // inlined (no marshaling wrapper) pinvokes only OPDEF(INTOP_NEWOBJ, "newobj", 5, 1, 1, InterpOpMethodHandle) OPDEF(INTOP_NEWOBJ_GENERIC, "newobj.generic", 6, 1, 2, InterpOpMethodHandle) OPDEF(INTOP_NEWOBJ_VT, "newobj.vt", 5, 1, 1, InterpOpMethodHandle) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 77faa81226f2c0..09e3489da19421 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -168,10 +168,11 @@ static OBJECTREF CreateMultiDimArray(MethodTable* arrayClass, int8_t* stack, int template static THelper GetPossiblyIndirectHelper(void* dataItem) { size_t helperDirectOrIndirect = (size_t)dataItem; - if (helperDirectOrIndirect & INTERP_INDIRECT_HELPER_TAG) - return *(THelper *)(helperDirectOrIndirect & ~INTERP_INDIRECT_HELPER_TAG); + if (helperDirectOrIndirect & INTERP_DIRECT_HELPER_TAG) + // Clear the direct flag and then raise the thumb bit as needed + return (THelper)PINSTRToPCODE((TADDR)(helperDirectOrIndirect & ~INTERP_DIRECT_HELPER_TAG)); else - return (THelper)helperDirectOrIndirect; + return *(THelper *)helperDirectOrIndirect; } // At present our behavior for float to int conversions is to perform a saturating conversion down to either 32 or 64 bits @@ -1841,15 +1842,18 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr callArgsOffset = ip[2]; methodSlot = ip[3]; int32_t targetAddrSlot = ip[4]; + int32_t indirectFlag = ip[5]; - ip += 5; + ip += 6; targetMethod = (MethodDesc*)pMethod->pDataItems[methodSlot]; - PCODE callTarget = *(PCODE*)pMethod->pDataItems[targetAddrSlot]; + PCODE callTarget = indirectFlag + ? *(PCODE *)pMethod->pDataItems[targetAddrSlot] + : (PCODE)pMethod->pDataItems[targetAddrSlot]; InlinedCallFrame inlinedCallFrame; inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip; - inlinedCallFrame.m_pCallSiteSP = stack; - inlinedCallFrame.m_pCalleeSavedFP = (TADDR)pFrame; + inlinedCallFrame.m_pCallSiteSP = pFrame; + inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; inlinedCallFrame.m_pThread = GetThread(); inlinedCallFrame.m_Datum = NULL; inlinedCallFrame.Push(); diff --git a/src/tests/JIT/interpreter/Interpreter.cs b/src/tests/JIT/interpreter/Interpreter.cs index 68cea1ca5de4ea..61c02d4bb61e6d 100644 --- a/src/tests/JIT/interpreter/Interpreter.cs +++ b/src/tests/JIT/interpreter/Interpreter.cs @@ -2411,6 +2411,25 @@ public static bool TestPInvoke() // Test marshaling wrappers writeToStdout("Hello world from pinvoke.dll!writeToStdout\n"); + /* fails, with output: + Assert failure(PID 32748 [0x00007fec], Thread: 24256 [0x5ec0]): pMD == codeInfo.GetMethodDesc() + + CORECLR! AppendExceptionStackFrame + 0x331 (0x00007ff9`85879b71) + SYSTEM.PRIVATE.CORELIB! + 0x0 (0x00007ff9`80d91f30) + SYSTEM.PRIVATE.CORELIB! + 0x0 (0x00007ff9`80d926b7) + SYSTEM.PRIVATE.CORELIB! + 0x0 (0x00007ff9`80d92289) + CORECLR! CallDescrWorkerInternal + 0x83 (0x00007ff9`859811c3) + CORECLR! CallDescrWorkerWithHandler + 0x130 (0x00007ff9`854755c0) + CORECLR! DispatchCallSimple + 0x26C (0x00007ff9`8547655c) + CORECLR! DispatchManagedException + 0x388 (0x00007ff9`85872998) + CORECLR! DispatchManagedException + 0x67 (0x00007ff9`858725a7) + CORECLR! UnwindAndContinueRethrowHelperAfterCatch + 0x1F8 (0x00007ff9`851be5e8) + File: Z:\runtime\src\coreclr\vm\exceptionhandling.cpp:3032 + Image: Z:\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\corerun.exe + + pMD is TestPInvoke (correct) and codeInfo.GetMethodDesc() is Main (wrong) + */ + /* bool caught = false; try { Console.WriteLine("calling missingPInvoke"); @@ -2423,27 +2442,26 @@ public static bool TestPInvoke() if (!caught) return false; + */ /* fails, with output: -calling missingPInvokeWithMarshaling -caught #2 - -Assert failure(PID 74448 [0x000122d0], Thread: 61928 [0xf1e8]): ohThrowable - -CORECLR! PreStubWorker$catch$10 + 0x9E (0x00007ffc`5a0e31fe) -CORECLR! CallSettingFrame_LookupContinuationIndex + 0x20 (0x00007ffc`59f7eeb0) -CORECLR! _FrameHandler4::CxxCallCatchBlock + 0x1DE (0x00007ffc`59f6a7fe) -NTDLL! RtlCaptureContext2 + 0x4A6 (0x00007ffd`45ac6c56) -CORECLR! PreStubWorker + 0x503 (0x00007ffc`598d68b3) -CORECLR! ThePreStub + 0x55 (0x00007ffc`59f0de25) -CORECLR! CallJittedMethodRetVoid + 0x14 (0x00007ffc`59f0c394) -CORECLR! InvokeCompiledMethod + 0x5CD (0x00007ffc`59b376fd) -CORECLR! InterpExecMethod + 0x7286 (0x00007ffc`59b33056) -CORECLR! ExecuteInterpretedMethod + 0x11B (0x00007ffc`598d528b) - File: Z:\runtime\src\coreclr\vm\prestub.cpp:1972 - Image: Z:\runtime\artifacts\tests\coreclr\windows.x64.Debug\Tests\Core_Root\corerun.exe - -Interpreted App returned -1073740286 + calling missingPInvokeWithMarshaling + caught #2 + + Assert failure(PID 59772 [0x0000e97c], Thread: 24864 [0x6120]): ohThrowable + + CORECLR! PreStubWorker$catch$10 + 0x93 (0x00007ff9`580972b3) + CORECLR! CallSettingFrame_LookupContinuationIndex + 0x20 (0x00007ff9`57f32e70) + CORECLR! _FrameHandler4::CxxCallCatchBlock + 0x1DE (0x00007ff9`57f1e83e) + NTDLL! RtlCaptureContext2 + 0x4A6 (0x00007ffa`b7e46606) + CORECLR! PreStubWorker + 0x4F8 (0x00007ff9`5789dd78) + CORECLR! ThePreStub + 0x55 (0x00007ff9`57ec29c5) + CORECLR! CallJittedMethodRetVoid + 0x14 (0x00007ff9`57ec0f34) + CORECLR! InvokeCompiledMethod + 0x5D7 (0x00007ff9`57afaf67) + CORECLR! InterpExecMethod + 0x84BB (0x00007ff9`57af68cb) + CORECLR! ExecuteInterpretedMethod + 0x11B (0x00007ff9`5789c77b) + File: Z:\runtime\src\coreclr\vm\prestub.cpp:1965 + Image: Z:\runtime\artifacts\tests\coreclr\windows.x64.Checked\Tests\Core_Root\corerun.exe */ /* bool caught2 = false; From bb27d7982d3adf31610d691f3c239a6ff00d6dc1 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 10 Jul 2025 19:08:15 -0700 Subject: [PATCH 5/8] Cleanup --- src/coreclr/interpreter/compiler.cpp | 6 +++++- src/coreclr/vm/dllimport.cpp | 7 ++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 373c26bffee711..cb585f782687d9 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2163,12 +2163,14 @@ int32_t InterpCompiler::GetDataItemIndexForHelperFtn(CorInfoHelpFunc ftn) CORINFO_CONST_LOOKUP ftnLookup; m_compHnd->getHelperFtn(ftn, &ftnLookup); void* addr = ftnLookup.addr; - if (ftnLookup.accessType != IAT_PVALUE) + if (ftnLookup.accessType == IAT_VALUE) { // We can't use the 1 bit to mark indirect addresses because it is used for real code on arm32 (the thumb bit) // So instead, we mark direct addresses with a 1 and then on the other end we will clear the 1 and re-set it as needed for thumb addr = (void*)((size_t)addr | INTERP_DIRECT_HELPER_TAG); } + else if (ftnLookup.accessType == IAT_PPVALUE) + NO_WAY("IAT_PPVALUE helpers not implemented in interpreter"); #ifdef DEBUG if (!PointerInNameMap(addr)) @@ -3070,6 +3072,8 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re m_compHnd->getAddressOfPInvokeTarget(callInfo.hMethod, &lookup); m_pLastNewIns->data[1] = GetDataItemIndex(lookup.addr); m_pLastNewIns->data[2] = lookup.accessType == IAT_PVALUE; + if (lookup.accessType == IAT_PPVALUE) + NO_WAY("IAT_PPVALUE pinvokes not implemented in interpreter"); } } break; diff --git a/src/coreclr/vm/dllimport.cpp b/src/coreclr/vm/dllimport.cpp index 52a0f803895a7c..ace24da99e6849 100644 --- a/src/coreclr/vm/dllimport.cpp +++ b/src/coreclr/vm/dllimport.cpp @@ -5886,11 +5886,8 @@ EXTERN_C LPVOID STDCALL NDirectImportWorker(NDirectMethodDesc* pMD) // INDEBUG(Thread *pThread = GetThread()); { - _ASSERTE( - ( - (pThread->GetFrame() != FRAME_TOP) && (pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) - ) || pMD->ShouldSuppressGCTransition() - ); + _ASSERTE((pThread->GetFrame() != FRAME_TOP && pThread->GetFrame()->GetFrameIdentifier() == FrameIdentifier::InlinedCallFrame) + || pMD->ShouldSuppressGCTransition()); CONSISTENCY_CHECK(pMD->IsNDirect()); // From 41b3cf0c9856988cb70f13168a67ff6e5161980a Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 10 Jul 2025 19:09:20 -0700 Subject: [PATCH 6/8] Fix musl ci build --- src/coreclr/vm/interpexec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 09e3489da19421..003d40b08dc022 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -12,7 +12,7 @@ // for numeric_limits #include -void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE overrideTarget = NULL) +void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE overrideTarget = (PCODE)0) { CONTRACTL { From de173d51cf16766b78ed776ab826dac7027822ca Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Mon, 14 Jul 2025 07:21:21 -0700 Subject: [PATCH 7/8] Address PR feedback Encapsulate inlined callframe creation --- src/coreclr/vm/interpexec.cpp | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 003d40b08dc022..bae0b011e3f5f7 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -12,7 +12,7 @@ // for numeric_limits #include -void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE overrideTarget = (PCODE)0) +void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE target) { CONTRACTL { @@ -45,11 +45,8 @@ void InvokeCompiledMethod(MethodDesc *pMD, int8_t *pArgs, int8_t *pRet, PCODE ov } } - if (overrideTarget) - // Interpreter-FIXME: Is this a race condition? - pHeader->SetTarget(overrideTarget); - else - pHeader->SetTarget(pMD->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); // The method to call + // Interpreter-FIXME: Potential race condition if a single CallStubHeader is reused for multiple targets. + pHeader->SetTarget(target); // The method to call pHeader->Invoke(pHeader->Routines, pArgs, pRet, pHeader->TotalStackSize); } @@ -164,6 +161,20 @@ static OBJECTREF CreateMultiDimArray(MethodTable* arrayClass, int8_t* stack, int #define LOCAL_VAR_ADDR(offset,type) ((type*)(stack + (offset))) #define LOCAL_VAR(offset,type) (*LOCAL_VAR_ADDR(offset, type)) #define NULL_CHECK(o) do { if ((o) == NULL) { COMPlusThrow(kNullReferenceException); } } while (0) +#define WITH_INLINED_CALL_FRAME(block) \ + do { \ + InlinedCallFrame inlinedCallFrame; \ + inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip; \ + inlinedCallFrame.m_pCallSiteSP = pFrame; \ + inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; \ + inlinedCallFrame.m_pThread = GetThread(); \ + inlinedCallFrame.m_Datum = NULL; \ + inlinedCallFrame.Push(); \ + \ + block\ + \ + inlinedCallFrame.Pop(); \ + } while (0) template static THelper GetPossiblyIndirectHelper(void* dataItem) { @@ -1850,21 +1861,10 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr ? *(PCODE *)pMethod->pDataItems[targetAddrSlot] : (PCODE)pMethod->pDataItems[targetAddrSlot]; - InlinedCallFrame inlinedCallFrame; - inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip; - inlinedCallFrame.m_pCallSiteSP = pFrame; - inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; - inlinedCallFrame.m_pThread = GetThread(); - inlinedCallFrame.m_Datum = NULL; - inlinedCallFrame.Push(); - - { - // Interpreter-FIXME: Create InlinedCallFrame. + WITH_INLINED_CALL_FRAME({ GCX_PREEMP(); InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); - } - - inlinedCallFrame.Pop(); + }); break; } @@ -1902,7 +1902,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr if (targetIp == NULL) { // If we didn't get the interpreter code pointer setup, then this is a method we need to invoke as a compiled method. - InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset); + InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, targetMethod->GetMultiCallableAddrOfCode(CORINFO_ACCESS_ANY)); break; } } From 860e39b7f4981efcba522528d4807a184f1ff51b Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Tue, 15 Jul 2025 08:33:46 -0700 Subject: [PATCH 8/8] Remove macro --- src/coreclr/vm/interpexec.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index bae0b011e3f5f7..927bd812a4af0b 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -161,20 +161,6 @@ static OBJECTREF CreateMultiDimArray(MethodTable* arrayClass, int8_t* stack, int #define LOCAL_VAR_ADDR(offset,type) ((type*)(stack + (offset))) #define LOCAL_VAR(offset,type) (*LOCAL_VAR_ADDR(offset, type)) #define NULL_CHECK(o) do { if ((o) == NULL) { COMPlusThrow(kNullReferenceException); } } while (0) -#define WITH_INLINED_CALL_FRAME(block) \ - do { \ - InlinedCallFrame inlinedCallFrame; \ - inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip; \ - inlinedCallFrame.m_pCallSiteSP = pFrame; \ - inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; \ - inlinedCallFrame.m_pThread = GetThread(); \ - inlinedCallFrame.m_Datum = NULL; \ - inlinedCallFrame.Push(); \ - \ - block\ - \ - inlinedCallFrame.Pop(); \ - } while (0) template static THelper GetPossiblyIndirectHelper(void* dataItem) { @@ -1861,10 +1847,20 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr ? *(PCODE *)pMethod->pDataItems[targetAddrSlot] : (PCODE)pMethod->pDataItems[targetAddrSlot]; - WITH_INLINED_CALL_FRAME({ + InlinedCallFrame inlinedCallFrame; + inlinedCallFrame.m_pCallerReturnAddress = (TADDR)ip; + inlinedCallFrame.m_pCallSiteSP = pFrame; + inlinedCallFrame.m_pCalleeSavedFP = (TADDR)stack; + inlinedCallFrame.m_pThread = GetThread(); + inlinedCallFrame.m_Datum = NULL; + inlinedCallFrame.Push(); + + { GCX_PREEMP(); InvokeCompiledMethod(targetMethod, stack + callArgsOffset, stack + returnOffset, callTarget); - }); + } + + inlinedCallFrame.Pop(); break; }