From c5cb7aa4ed02f10e976821b8256f30036d4dbd69 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 16 Jun 2025 18:12:49 +0300 Subject: [PATCH 1/3] Add full constructor to StackInfo To prevent having uninitialized fields --- src/coreclr/interpreter/compiler.cpp | 3 +-- src/coreclr/interpreter/compiler.h | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 99850a6cad3bcc..c81e12cf6999f9 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -1285,9 +1285,8 @@ void InterpCompiler::EmitConv(StackInfo *sp, StackType type, InterpOpcode convOp InterpInst *newInst = AddIns(convOp); newInst->SetSVar(sp->var); - new (sp) StackInfo(type); int32_t var = CreateVarExplicit(g_interpTypeFromStackType[type], NULL, INTERP_STACK_SLOT_SIZE); - sp->var = var; + new (sp) StackInfo(type, NULL, INTERP_STACK_SLOT_SIZE, var); newInst->SetDVar(var); // NOTE: We rely on m_pLastNewIns == newInst upon return from this function. Make sure you preserve that if you change anything. diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 6348dae993b5f3..34c110fefb4cef 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -274,12 +274,12 @@ struct StackInfo // the stack a new var is created. int var; - StackInfo(StackType type) + StackInfo(StackType type, CORINFO_CLASS_HANDLE clsHnd, int size, int var) { this->type = type; - clsHnd = NULL; - size = 0; - var = -1; + this->clsHnd = clsHnd; + this->size = size; + this->var = var; } }; From 8da7c1fa46dd6891f3da704088a726b877de54d8 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 16 Jun 2025 18:16:18 +0300 Subject: [PATCH 2/3] Add constrained support for calls Rename constrainedClass to pConstrainedToken for clarity. Extract box code emit into separate method. --- src/coreclr/interpreter/compiler.cpp | 81 ++++++++++++++++++---------- src/coreclr/interpreter/compiler.h | 3 +- src/coreclr/interpreter/intops.def | 1 + src/coreclr/vm/interpexec.cpp | 11 ++-- 4 files changed, 64 insertions(+), 32 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index c81e12cf6999f9..4ddf40fa8aab6c 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -2311,7 +2311,7 @@ int InterpCompiler::EmitGenericHandleAsVar(const CORINFO_GENERICHANDLE_RESULT &e return resultVar; } -void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall, bool newObj, bool isCalli) +void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool readonly, bool tailcall, bool newObj, bool isCalli) { uint32_t token = getU4LittleEndian(m_ip + 1); bool isVirtual = (*m_ip == CEE_CALLVIRT); @@ -2341,19 +2341,38 @@ void InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool rea } else { - ResolveToken(token, newObj ? CORINFO_TOKENKIND_NewObj : CORINFO_TOKENKIND_Method, &resolvedCallToken); - + CORINFO_CALLINFO_FLAGS flags = (CORINFO_CALLINFO_FLAGS)(CORINFO_CALLINFO_ALLOWINSTPARAM | CORINFO_CALLINFO_SECURITYCHECKS | CORINFO_CALLINFO_DISALLOW_STUB); if (isVirtual) flags = (CORINFO_CALLINFO_FLAGS)(flags | CORINFO_CALLINFO_CALLVIRT); - m_compHnd->getCallInfo(&resolvedCallToken, constrainedClass, m_methodInfo->ftn, flags, &callInfo); + m_compHnd->getCallInfo(&resolvedCallToken, pConstrainedToken, m_methodInfo->ftn, flags, &callInfo); if (EmitCallIntrinsics(callInfo.hMethod, callInfo.sig)) { m_ip += 5; return; } + + if (callInfo.thisTransform != CORINFO_NO_THIS_TRANSFORM) + { + assert(pConstrainedToken != NULL); + StackInfo *pThisStackInfo = m_pStackPointer - callInfo.sig.numArgs - 1; + if (callInfo.thisTransform == CORINFO_BOX_THIS) + { + EmitBox(pThisStackInfo, pConstrainedToken->hClass, true); + } + else + { + assert(callInfo.thisTransform == CORINFO_DEREF_THIS); + AddIns(INTOP_LDIND_I); + m_pLastNewIns->SetSVar(pThisStackInfo->var); + m_pLastNewIns->data[0] = 0; + int32_t var = CreateVarExplicit(InterpTypeO, pConstrainedToken->hClass, INTERP_STACK_SLOT_SIZE); + new (pThisStackInfo) StackInfo(StackTypeO, pConstrainedToken->hClass, var); + m_pLastNewIns->SetDVar(pThisStackInfo->var); + } + } } if (newObj && (callInfo.classFlags & CORINFO_FLG_VAROBJSIZE)) @@ -2885,12 +2904,27 @@ void InterpCompiler::EmitLdLocA(int32_t var) m_pLastNewIns->SetDVar(m_pStackPointer[-1].var); } +void InterpCompiler::EmitBox(StackInfo* pStackInfo, CORINFO_CLASS_HANDLE clsHnd, bool argByRef) +{ + CORINFO_CLASS_HANDLE boxedClsHnd = m_compHnd->getTypeForBox(clsHnd); + CorInfoHelpFunc helpFunc = m_compHnd->getBoxHelper(clsHnd); + AddIns(argByRef ? INTOP_BOX_PTR : INTOP_BOX); + m_pLastNewIns->SetSVar(pStackInfo->var); + + int32_t var = CreateVarExplicit(InterpTypeO, boxedClsHnd, INTERP_STACK_SLOT_SIZE); + new (pStackInfo) StackInfo(StackTypeO, boxedClsHnd, INTERP_STACK_SLOT_SIZE, var); + + m_pLastNewIns->SetDVar(pStackInfo->var); + m_pLastNewIns->data[0] = GetDataItemIndex(clsHnd); + m_pLastNewIns->data[1] = GetDataItemIndexForHelperFtn(helpFunc); +} + int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) { bool readonly = false; bool tailcall = false; bool volatile_ = false; - CORINFO_RESOLVED_TOKEN* constrainedClass = NULL; + CORINFO_RESOLVED_TOKEN* pConstrainedToken = NULL; CORINFO_RESOLVED_TOKEN constrainedToken; uint8_t *codeEnd; int numArgs = m_methodInfo->args.hasThis() + m_methodInfo->args.numArgs; @@ -4037,21 +4071,21 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) break; case CEE_CALLVIRT: case CEE_CALL: - EmitCall(constrainedClass, readonly, tailcall, false /*newObj*/, false /*isCalli*/); - constrainedClass = NULL; + EmitCall(pConstrainedToken, readonly, tailcall, false /*newObj*/, false /*isCalli*/); + pConstrainedToken = NULL; readonly = false; tailcall = false; break; case CEE_CALLI: - EmitCall(NULL /*constrainedClass*/, false /* readonly*/, false /* tailcall*/, false /*newObj*/, true /*isCalli*/); - constrainedClass = NULL; + EmitCall(NULL /*pConstrainedToken*/, false /* readonly*/, false /* tailcall*/, false /*newObj*/, true /*isCalli*/); + pConstrainedToken = NULL; readonly = false; tailcall = false; break; case CEE_NEWOBJ: { - EmitCall(NULL /*constrainedClass*/, false /* readonly*/, false /* tailcall*/, true /*newObj*/, false /*isCalli*/); - constrainedClass = NULL; + EmitCall(NULL /*pConstrainedToken*/, false /* readonly*/, false /* tailcall*/, true /*newObj*/, false /*isCalli*/); + pConstrainedToken = NULL; readonly = false; tailcall = false; break; @@ -4428,12 +4462,9 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) { uint32_t token = getU4LittleEndian(m_ip + 1); - constrainedToken.tokenScope = m_compScopeHnd; - constrainedToken.tokenContext = METHOD_BEING_COMPILED_CONTEXT(); - constrainedToken.token = token; - constrainedToken.tokenType = CORINFO_TOKENKIND_Constrained; - m_compHnd->resolveToken(&constrainedToken); - constrainedClass = &constrainedToken; + ResolveToken(token, CORINFO_TOKENKIND_Constrained, &constrainedToken); + + pConstrainedToken = &constrainedToken; m_ip += 5; break; } @@ -4526,8 +4557,8 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) ResolveToken(token, CORINFO_TOKENKIND_Method, &resolvedToken); CORINFO_CALL_INFO callInfo; - m_compHnd->getCallInfo(&resolvedToken, constrainedClass, m_methodInfo->ftn, (CORINFO_CALLINFO_FLAGS)(CORINFO_CALLINFO_SECURITYCHECKS| CORINFO_CALLINFO_LDFTN), &callInfo); - constrainedClass = NULL; + m_compHnd->getCallInfo(&resolvedToken, pConstrainedToken, m_methodInfo->ftn, (CORINFO_CALLINFO_FLAGS)(CORINFO_CALLINFO_SECURITYCHECKS| CORINFO_CALLINFO_LDFTN), &callInfo); + pConstrainedToken = NULL; if (callInfo.kind == CORINFO_CALL) { @@ -4627,17 +4658,11 @@ int InterpCompiler::GenerateCode(CORINFO_METHOD_INFO* methodInfo) case CEE_BOX: { + CORINFO_CLASS_HANDLE clsHnd = ResolveClassToken(getU4LittleEndian(m_ip + 1)); CHECK_STACK(1); m_pStackPointer -= 1; - CORINFO_CLASS_HANDLE clsHnd = ResolveClassToken(getU4LittleEndian(m_ip + 1)); - CORINFO_CLASS_HANDLE boxedClsHnd = m_compHnd->getTypeForBox(clsHnd); - CorInfoHelpFunc helpFunc = m_compHnd->getBoxHelper(clsHnd); - AddIns(INTOP_BOX); - m_pLastNewIns->SetSVar(m_pStackPointer[0].var); - PushStackType(StackTypeO, boxedClsHnd); - m_pLastNewIns->SetDVar(m_pStackPointer[-1].var); - m_pLastNewIns->data[0] = GetDataItemIndex(clsHnd); - m_pLastNewIns->data[1] = GetDataItemIndexForHelperFtn(helpFunc); + EmitBox(m_pStackPointer, clsHnd, false); + m_pStackPointer++; m_ip += 5; break; } diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index 34c110fefb4cef..c2a05b8ed35fdb 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -491,7 +491,7 @@ class InterpCompiler void EmitUnaryArithmeticOp(int32_t opBase); void EmitShiftOp(int32_t opBase); void EmitCompareOp(int32_t opBase); - void EmitCall(CORINFO_RESOLVED_TOKEN* constrainedClass, bool readonly, bool tailcall, bool newObj, bool isCalli); + void EmitCall(CORINFO_RESOLVED_TOKEN* pConstrainedToken, bool readonly, bool tailcall, bool newObj, bool isCalli); bool EmitCallIntrinsics(CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO sig); void EmitLdind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset); void EmitStind(InterpType type, CORINFO_CLASS_HANDLE clsHnd, int32_t offset, bool reverseSVarOrder); @@ -500,6 +500,7 @@ class InterpCompiler void EmitStaticFieldAddress(CORINFO_FIELD_INFO *pFieldInfo, CORINFO_RESOLVED_TOKEN *pResolvedToken); void EmitStaticFieldAccess(InterpType interpFieldType, CORINFO_FIELD_INFO *pFieldInfo, CORINFO_RESOLVED_TOKEN *pResolvedToken, bool isLoad); void EmitLdLocA(int32_t var); + void EmitBox(StackInfo* pStackInfo, CORINFO_CLASS_HANDLE clsHnd, bool argByRef); // Var Offset allocator TArray *m_pActiveCalls; diff --git a/src/coreclr/interpreter/intops.def b/src/coreclr/interpreter/intops.def index ac75b30d256229..399d1a18603d50 100644 --- a/src/coreclr/interpreter/intops.def +++ b/src/coreclr/interpreter/intops.def @@ -207,6 +207,7 @@ OPDEF(INTOP_CONV_OVF_U8_R4, "conv.ovf.u8.r4", 3, 1, 1, InterpOpNoArgs) OPDEF(INTOP_CONV_OVF_U8_R8, "conv.ovf.u8.r8", 3, 1, 1, InterpOpNoArgs) OPDEF(INTOP_BOX, "box", 5, 1, 1, InterpOpClassHandle) // [class handle data item] [helper data item] +OPDEF(INTOP_BOX_PTR, "box.ptr", 5, 1, 1, InterpOpClassHandle) // [class handle data item] [helper data item] OPDEF(INTOP_UNBOX, "unbox", 5, 1, 1, InterpOpClassHandle) // [class handle data item] [helper data item] OPDEF(INTOP_UNBOX_ANY, "unbox.any", 5, 1, 1, InterpOpClassHandle) // [class handle data item] [helper data item] // Unary operations end diff --git a/src/coreclr/vm/interpexec.cpp b/src/coreclr/vm/interpexec.cpp index 4d7b0b53b15da7..8df97d9e073211 100644 --- a/src/coreclr/vm/interpexec.cpp +++ b/src/coreclr/vm/interpexec.cpp @@ -1674,6 +1674,7 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr ip += 2; break; case INTOP_BOX: + case INTOP_BOX_PTR: case INTOP_UNBOX: case INTOP_UNBOX_ANY: { @@ -1683,10 +1684,14 @@ void InterpExecMethod(InterpreterFrame *pInterpreterFrame, InterpMethodContextFr MethodTable *pMT = (MethodTable*)pMethod->pDataItems[ip[3]]; HELPER_FTN_BOX_UNBOX helper = GetPossiblyIndirectHelper(pMethod->pDataItems[ip[4]]); - if (opcode == INTOP_BOX) { + if (opcode == INTOP_BOX || opcode == INTOP_BOX_PTR) { // internal static object Box(MethodTable* typeMT, ref byte unboxedData) - void *unboxedData = LOCAL_VAR_ADDR(sreg, void); - LOCAL_VAR(dreg, Object*) = (Object*)helper(pMT, unboxedData); + void *unboxedData; + if (opcode == INTOP_BOX) + unboxedData = LOCAL_VAR_ADDR(sreg, void); + else + unboxedData = LOCAL_VAR(sreg, void*); + LOCAL_VAR(dreg, OBJECTREF) = ObjectToOBJECTREF((Object*)helper(pMT, unboxedData)); } else { // private static ref byte Unbox(MethodTable* toTypeHnd, object obj) Object *src = LOCAL_VAR(sreg, Object*); From 220d60e1565d072d1fdd04367c1f6b418fd080ef Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 16 Jun 2025 18:27:55 +0300 Subject: [PATCH 3/3] Get rid of StackInfo.size It doesn't seem like we will be needing it. Use constructor in more places --- src/coreclr/interpreter/compiler.cpp | 21 ++++++--------------- src/coreclr/interpreter/compiler.h | 6 +----- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/coreclr/interpreter/compiler.cpp b/src/coreclr/interpreter/compiler.cpp index 4ddf40fa8aab6c..050f2e644b825d 100644 --- a/src/coreclr/interpreter/compiler.cpp +++ b/src/coreclr/interpreter/compiler.cpp @@ -578,11 +578,8 @@ bool InterpCompiler::CheckStackHelper(int n) void InterpCompiler::PushTypeExplicit(StackType stackType, CORINFO_CLASS_HANDLE clsHnd, int size) { EnsureStack(1); - m_pStackPointer->type = stackType; - m_pStackPointer->clsHnd = clsHnd; - m_pStackPointer->size = ALIGN_UP_TO(size, INTERP_STACK_SLOT_SIZE); - int var = CreateVarExplicit(g_interpTypeFromStackType[stackType], clsHnd, size); - m_pStackPointer->var = var; + int32_t var = CreateVarExplicit(g_interpTypeFromStackType[stackType], clsHnd, size); + new (m_pStackPointer) StackInfo(stackType, clsHnd, var); m_pStackPointer++; } @@ -1286,7 +1283,7 @@ void InterpCompiler::EmitConv(StackInfo *sp, StackType type, InterpOpcode convOp newInst->SetSVar(sp->var); int32_t var = CreateVarExplicit(g_interpTypeFromStackType[type], NULL, INTERP_STACK_SLOT_SIZE); - new (sp) StackInfo(type, NULL, INTERP_STACK_SLOT_SIZE, var); + new (sp) StackInfo(type, NULL, var); newInst->SetDVar(var); // NOTE: We rely on m_pLastNewIns == newInst upon return from this function. Make sure you preserve that if you change anything. @@ -1704,10 +1701,7 @@ bool InterpCompiler::InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodI // Initialize the filter stack state. It initially contains the exception object. pFilterBB->stackHeight = 1; pFilterBB->pStackState = (StackInfo*)AllocMemPool(sizeof (StackInfo)); - pFilterBB->pStackState[0].type = StackTypeO; - pFilterBB->pStackState[0].size = INTERP_STACK_SLOT_SIZE; - pFilterBB->pStackState[0].clsHnd = NULL; - pFilterBB->pStackState[0].var = pFilterBB->clauseVarIndex; + new (pFilterBB->pStackState) StackInfo(StackTypeO, NULL, pFilterBB->clauseVarIndex); // Find and mark all basic blocks that are part of the filter region. for (uint32_t j = clause.FilterOffset; j < clause.HandlerOffset; j++) @@ -1736,10 +1730,7 @@ bool InterpCompiler::InitializeClauseBuildingBlocks(CORINFO_METHOD_INFO* methodI // Initialize the catch / filtered handler stack state. It initially contains the exception object. pCatchBB->stackHeight = 1; pCatchBB->pStackState = (StackInfo*)AllocMemPool(sizeof (StackInfo)); - pCatchBB->pStackState[0].type = StackTypeO; - pCatchBB->pStackState[0].size = INTERP_STACK_SLOT_SIZE; - pCatchBB->pStackState[0].var = pCatchBB->clauseVarIndex; - pCatchBB->pStackState[0].clsHnd = NULL; + new (pCatchBB->pStackState) StackInfo(StackTypeO, NULL, pCatchBB->clauseVarIndex); } } @@ -2912,7 +2903,7 @@ void InterpCompiler::EmitBox(StackInfo* pStackInfo, CORINFO_CLASS_HANDLE clsHnd, m_pLastNewIns->SetSVar(pStackInfo->var); int32_t var = CreateVarExplicit(InterpTypeO, boxedClsHnd, INTERP_STACK_SLOT_SIZE); - new (pStackInfo) StackInfo(StackTypeO, boxedClsHnd, INTERP_STACK_SLOT_SIZE, var); + new (pStackInfo) StackInfo(StackTypeO, boxedClsHnd, var); m_pLastNewIns->SetDVar(pStackInfo->var); m_pLastNewIns->data[0] = GetDataItemIndex(clsHnd); diff --git a/src/coreclr/interpreter/compiler.h b/src/coreclr/interpreter/compiler.h index c2a05b8ed35fdb..6ce45505357212 100644 --- a/src/coreclr/interpreter/compiler.h +++ b/src/coreclr/interpreter/compiler.h @@ -266,19 +266,15 @@ struct StackInfo { StackType type; CORINFO_CLASS_HANDLE clsHnd; - // Size that this value will occupy on the interpreter stack. It is a multiple - // of INTERP_STACK_SLOT_SIZE - int size; // The var associated with the value of this stack entry. Every time we push on // the stack a new var is created. int var; - StackInfo(StackType type, CORINFO_CLASS_HANDLE clsHnd, int size, int var) + StackInfo(StackType type, CORINFO_CLASS_HANDLE clsHnd, int var) { this->type = type; this->clsHnd = clsHnd; - this->size = size; this->var = var; } };