From 2af0c303dafbb5848ec55ccf50c66e0dc6e92ecb Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Fri, 22 Aug 2025 11:58:01 -0700 Subject: [PATCH 1/3] Fix passing Vector3s from the interpreter to native code Add DOTNET_InterpHaltOnCall to make the interpreter assert when compiling a call to a specified method --- src/coreclr/interpreter/compiler.cpp | 7 +++++-- src/coreclr/interpreter/interpconfigvalues.h | 1 + src/coreclr/vm/callstubgenerator.cpp | 15 +++++++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 1b5310b9c85b61..b25ba01fc65658 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2848,6 +2848,9 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re m_pLastNewIns->SetDVar(pThisStackInfo->var); } } + + if (InterpConfig.InterpHaltOnCall().contains(m_compHnd, resolvedCallToken.hMethod, resolvedCallToken.hClass, nullptr)) + assert(!"HaltOnCall"); } if (newObj && (callInfo.classFlags & CORINFO_FLG_VAROBJSIZE)) @@ -5315,7 +5318,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) int byrefOfTypedRefVar = m_pStackPointer[-1].var; m_pLastNewIns->SetDVar(byrefOfTypedRefVar); m_pStackPointer--; - + AddIns(GetStindForType(InterpTypeByRef)); m_pLastNewIns->data[0] = OFFSETOF__CORINFO_TypedReference__dataPtr; m_pLastNewIns->SetSVars2(byrefOfTypedRefVar, addressVar); @@ -5380,7 +5383,7 @@ void InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) m_pLastNewIns->data[2] = 0; m_pLastNewIns->SetSVar(typedByRefVar); m_pLastNewIns->SetDVar(classHandleVar); - + AddIns(INTOP_CALL_HELPER_P_S); m_pLastNewIns->data[0] = GetDataForHelperFtn(CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE_MAYBENULL); m_pLastNewIns->SetSVar(classHandleVar); diff --git a/src/coreclr/interpreter/interpconfigvalues.h b/src/coreclr/interpreter/interpconfigvalues.h index a76685a1fc7011..2e729dedd4af69 100644 --- a/src/coreclr/interpreter/interpconfigvalues.h +++ b/src/coreclr/interpreter/interpconfigvalues.h @@ -20,6 +20,7 @@ #endif RELEASE_CONFIG_METHODSET(Interpreter, "Interpreter") +CONFIG_METHODSET(InterpHaltOnCall, "InterpHaltOnCall"); // Assert in the compiler when compiling a call to these method(s) CONFIG_METHODSET(InterpHalt, "InterpHalt"); CONFIG_METHODSET(InterpDump, "InterpDump"); CONFIG_INTEGER(InterpList, "InterpList", 0); // List the methods which are compiled by the interpreter JIT diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index 02f275d9b9506a..d07a414bf28455 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -1685,17 +1685,28 @@ void CallStubGenerator::ProcessArgument(ArgIterator *pArgIt, ArgLocDesc& argLocD // we always process single argument passed by reference using single routine. if (pArgIt != NULL && pArgIt->IsArgPassedByRef()) { + int unalignedArgSize = pArgIt->GetArgSize(); + // For the interpreter-to-native transition we need to make sure that we properly align the offsets + // to interpreter stack slots. Otherwise a VT of i.e. size 12 will misalign the stack offset during + // loads and we will start loading garbage into registers. + // We don't need to do this for native-to-interpreter transitions because the Store_Ref_xxx helpers + // automatically do alignment of the stack offset themselves when updating the stack offset, + // and if we were to pass them aligned sizes they would potentially read bytes past the end of the VT. + int alignedArgSize = m_interpreterToNative + ? (unalignedArgSize + 7) & ~7 + : unalignedArgSize; + if (argLocDesc.m_cGenReg == 1) { pRoutines[m_routineIndex++] = GetGPRegRefRoutine(argLocDesc.m_idxGenReg); - pRoutines[m_routineIndex++] = pArgIt->GetArgSize(); + pRoutines[m_routineIndex++] = alignedArgSize; m_r1 = NoRange; } else { _ASSERTE(argLocDesc.m_byteStackIndex != -1); pRoutines[m_routineIndex++] = GetStackRefRoutine(); - pRoutines[m_routineIndex++] = ((int64_t)pArgIt->GetArgSize() << 32) | argLocDesc.m_byteStackIndex; + pRoutines[m_routineIndex++] = ((int64_t)alignedArgSize << 32) | argLocDesc.m_byteStackIndex; m_s1 = NoRange; } } From ad9d8b5f005d491773709d5fa17963a700d3840e Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Fri, 22 Aug 2025 12:10:23 -0700 Subject: [PATCH 2/3] Address PR feedback --- src/coreclr/vm/callstubgenerator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/callstubgenerator.cpp b/src/coreclr/vm/callstubgenerator.cpp index d07a414bf28455..a8342e19c09496 100644 --- a/src/coreclr/vm/callstubgenerator.cpp +++ b/src/coreclr/vm/callstubgenerator.cpp @@ -1693,7 +1693,7 @@ void CallStubGenerator::ProcessArgument(ArgIterator *pArgIt, ArgLocDesc& argLocD // automatically do alignment of the stack offset themselves when updating the stack offset, // and if we were to pass them aligned sizes they would potentially read bytes past the end of the VT. int alignedArgSize = m_interpreterToNative - ? (unalignedArgSize + 7) & ~7 + ? ALIGN_UP(unalignedArgSize, 8) : unalignedArgSize; if (argLocDesc.m_cGenReg == 1) From 6973365514b846eb6811659dbee79b5e8d756185 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Fri, 22 Aug 2025 13:47:08 -0700 Subject: [PATCH 3/3] Fix build on CI --- src/coreclr/interpreter/compiler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index b25ba01fc65658..3919e3400d1a08 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2849,8 +2849,10 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool re } } +#if DEBUG if (InterpConfig.InterpHaltOnCall().contains(m_compHnd, resolvedCallToken.hMethod, resolvedCallToken.hClass, nullptr)) assert(!"HaltOnCall"); +#endif } if (newObj && (callInfo.classFlags & CORINFO_FLG_VAROBJSIZE))