diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 6b2e65004192bd..0e97164d884ec8 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4553,10 +4553,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl }; DoPhase(this, PHASE_POST_MORPH, postMorphPhase); - // GS security checks for unsafe buffers - // - DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase); - if (opts.OptimizationEnabled()) { // Compute the block weights @@ -4948,6 +4944,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl DoPhase(this, PHASE_ASYNC, &Compiler::TransformAsync); } + // GS security checks for unsafe buffers + // + DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase); + #ifdef TARGET_WASM // Transform any strongly connected components into reducible flow. // diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index c59041eb655972..fb2d5aa3f998ab 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -11247,18 +11247,23 @@ class Compiler #endif }; - GSCookie* gsGlobalSecurityCookieAddr; // Address of global cookie for unsafe buffer checks - GSCookie gsGlobalSecurityCookieVal; // Value of global cookie if addr is NULL - ShadowParamVarInfo* gsShadowVarInfo = nullptr; // Table used by shadow param analysis code + GSCookie* gsGlobalSecurityCookieAddr; // Address of global cookie for unsafe buffer checks + GSCookie gsGlobalSecurityCookieVal; // Value of global cookie if addr is NULL + ShadowParamVarInfo* gsShadowVarInfo = nullptr; // Table used by shadow param analysis code + unsigned gsShadowVarInfoCount = 0; PhaseStatus gsPhase(); void gsGSChecksInitCookie(); // Grabs cookie variable void gsCopyShadowParams(); // Identify vulnerable params and create dhadow copies bool gsFindVulnerableParams(); // Shadow param analysis code - void gsParamsToShadows(); // Insert copy code and replave param uses by shadow - - static fgWalkPreFn gsMarkPtrsAndAssignGroups; // Shadow param analysis tree-walk - static fgWalkPreFn gsReplaceShadowParams; // Shadow param replacement tree-walk + template + void gsVisitDependentLocals(GenTree* tree, TVisit visit); + void gsMarkPointers(GenTree* tree); + void gsUnionAssignGroups(unsigned lclNum1, unsigned lclNum2, GenTree* reason); + void gsParamsToShadows(); // Insert copy code and replace param uses by shadow + void gsCopyIntoShadow(unsigned lclNum, unsigned shadowLclNum, LIR::Range& range); + bool gsCreateShadowingLocals(); + void gsRewriteTreeForShadowParam(GenTree* tree); #define DEFAULT_MAX_INLINE_SIZE \ 100 // Methods with > DEFAULT_MAX_INLINE_SIZE IL bytes will never be inlined. diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp index 4d7a54fcdcc2ff..604ab0d7df3a07 100644 --- a/src/coreclr/jit/gschecks.cpp +++ b/src/coreclr/jit/gschecks.cpp @@ -45,10 +45,10 @@ PhaseStatus Compiler::gsPhase() return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } -/***************************************************************************** - * gsGSChecksInitCookie - * Grabs the cookie for detecting overflow of unsafe buffers. - */ +//------------------------------------------------------------------------ +// gsGSChecksInitCookie: +// Initialize the GS cookie local and value. +// void Compiler::gsGSChecksInitCookie() { var_types type = TYP_I_IMPL; @@ -62,19 +62,19 @@ void Compiler::gsGSChecksInitCookie() info.compCompHnd->getGSCookie(&gsGlobalSecurityCookieVal, &gsGlobalSecurityCookieAddr); } -/***************************************************************************** - * gsCopyShadowParams - * The current function has an unsafe buffer on the stack. Search for vulnerable - * parameters which could be used to modify a code address and take over the process - * in the case of a buffer overrun. Create a safe local copy for each vulnerable parameter, - * which will be allocated bellow the unsafe buffer. Change uses of the param to the - * shadow copy. - * - * A pointer under indirection is considered vulnerable. A malicious user could read from - * protected memory or write to it. If a parameter is assigned/computed into another variable, - * and is a pointer (i.e., under indirection), then we consider the variable to be part of the - * equivalence class with the parameter. All parameters in the equivalence class are shadowed. - */ +//------------------------------------------------------------------------ +// gsCopyShadowParams: +// The current function has an unsafe buffer on the stack. Search for vulnerable +// parameters which could be used to modify a code address and take over the process +// in the case of a buffer overrun. Create a safe local copy for each vulnerable parameter, +// which will be allocated below the unsafe buffer. Change uses of the param to the +// shadow copy. +// +// A pointer under indirection is considered vulnerable. A malicious user could read from +// protected memory or write to it. If a parameter is assigned/computed into another variable, +// and is a pointer (i.e., under indirection), then we consider the variable to be part of the +// equivalence class with the parameter. All parameters in the equivalence class are shadowed. +// void Compiler::gsCopyShadowParams() { if (info.compIsVarArgs) @@ -84,7 +84,8 @@ void Compiler::gsCopyShadowParams() // Allocate array for shadow param info // - gsShadowVarInfo = new (this, CMK_Unknown) ShadowParamVarInfo[lvaCount](); + gsShadowVarInfo = new (this, CMK_Unknown) ShadowParamVarInfo[lvaCount](); + gsShadowVarInfoCount = lvaCount; // Find groups of variables assigned to each other, and also // tracks variables which are dereferenced and marks them as ptrs. @@ -101,200 +102,207 @@ void Compiler::gsCopyShadowParams() // There are no vulnerable params. // Clear out the info to avoid looking at stale data. // - gsShadowVarInfo = nullptr; + gsShadowVarInfo = nullptr; + gsShadowVarInfoCount = 0; } } -// This struct tracks how a tree is being used - -struct MarkPtrsInfo +//------------------------------------------------------------------------ +// gsUnionAssignGroups: +// Unify the assign groups of lclNum1 and lclNum2, indicating that these may +// take the same value during the function's execution. +// +// Parameters: +// lclNum1 - The first local +// lclNum2 - The second local +// reason - The tree that is the reason for the union (for debugging only) +// +void Compiler::gsUnionAssignGroups(unsigned lclNum1, unsigned lclNum2, GenTree* reason) { - Compiler* comp; - unsigned lvStoreDef; // Which local variable is the tree being assigned to? - bool isStoreSrc; // Is this the source value for a local store? - bool isUnderIndir; // Is this a pointer value tree that is being dereferenced? - bool skipNextNode; // Skip a single node during the tree-walk + ShadowParamVarInfo& info1 = gsShadowVarInfo[lclNum1]; + ShadowParamVarInfo& info2 = gsShadowVarInfo[lclNum2]; #ifdef DEBUG - void Print() + if (info1.assignGroup != info2.assignGroup) { - printf("[MarkPtrsInfo] = {comp = %p, lvStoreDef = %d, isStoreSrc = %d, isUnderIndir = %d, skipNextNode = %d}\n", - comp, lvStoreDef, isStoreSrc, isUnderIndir, skipNextNode); + JITDUMP("Unifying assign groups of V%02u and V%02u because of [%06u]\n", lclNum1, lclNum2, dspTreeID(reason)); } #endif -}; - -/***************************************************************************** - * gsMarkPtrsAndAssignGroups - * Walk a tree looking for assignment groups, variables whose value is used - * in a *p store or use, and variable passed to calls. This info is then used - * to determine parameters which are vulnerable. - * This function carries a state to know if it is under an assign node, call node - * or indirection node. It starts a new tree walk for it's subtrees when the state - * changes. - */ -Compiler::fgWalkResult Compiler::gsMarkPtrsAndAssignGroups(GenTree** pTree, fgWalkData* data) -{ - struct MarkPtrsInfo* pState = (MarkPtrsInfo*)data->pCallbackData; - struct MarkPtrsInfo newState = *pState; - Compiler* comp = data->compiler; - GenTree* tree = *pTree; - ShadowParamVarInfo* shadowVarInfo = pState->comp->gsShadowVarInfo; - assert(shadowVarInfo); - assert(!pState->isStoreSrc || (pState->lvStoreDef != BAD_VAR_NUM)); + // + // Add lvAssignDef and lclNum to a common assign group + if (info1.assignGroup != nullptr) + { + if (info2.assignGroup != nullptr) + { + // OR both bit vector + info1.assignGroup->bitVectOr(info2.assignGroup); + } + else + { + info1.assignGroup->bitVectSet(lclNum2); + } + + // Point both to the same bit vector + info2.assignGroup = info1.assignGroup; + } + else if (info2.assignGroup != nullptr) + { + info2.assignGroup->bitVectSet(lclNum1); - if (pState->skipNextNode) + // Point both to the same bit vector + info1.assignGroup = info2.assignGroup; + } + else { - pState->skipNextNode = false; - return WALK_CONTINUE; + FixedBitVect* bv = FixedBitVect::bitVectInit(lvaCount, this); + + // Neither of them has an assign group yet. Make a new one. + info1.assignGroup = bv; + info2.assignGroup = bv; + bv->bitVectSet(lclNum1); + bv->bitVectSet(lclNum2); } +} - switch (tree->OperGet()) +//------------------------------------------------------------------------ +// gsVisitDependentLocals: +// Visit the local number of all locals that a specified node's value may +// depend on. +// +// Parameters: +// node - The node +// visit - A functor void(unsigned lclNum) called for each local number that +// directly contributes to node's value. +// +template +void Compiler::gsVisitDependentLocals(GenTree* node, TVisit visit) +{ + class Visitor : public GenTreeVisitor { - // Indirections - look for *p uses and defs - case GT_IND: - case GT_BLK: - case GT_ARR_ELEM: - case GT_MDARR_LENGTH: - case GT_MDARR_LOWER_BOUND: - newState.isUnderIndir = true; - newState.skipNextNode = true; // Don't have to worry about which kind of node we're dealing with - comp->fgWalkTreePre(&tree, comp->gsMarkPtrsAndAssignGroups, &newState); - return WALK_SKIP_SUBTREES; - - case GT_STOREIND: - case GT_STORE_BLK: - newState.isUnderIndir = true; - comp->fgWalkTreePre(&tree->AsIndir()->Addr(), comp->gsMarkPtrsAndAssignGroups, &newState); - comp->fgWalkTreePre(&tree->AsIndir()->Data(), comp->gsMarkPtrsAndAssignGroups, pState); - return WALK_SKIP_SUBTREES; - - // local vars and param uses - case GT_LCL_VAR: - case GT_LCL_FLD: + TVisit& m_visit; + public: + enum { - unsigned lclNum = tree->AsLclVarCommon()->GetLclNum(); + DoPreOrder = true, + }; + + Visitor(Compiler* compiler, TVisit& visit) + : GenTreeVisitor(compiler) + , m_visit(visit) + { + } - if (pState->isUnderIndir) + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* node = *use; + if (node->OperIs(GT_IND, GT_BLK, GT_MDARR_LENGTH, GT_MDARR_LOWER_BOUND, GT_CALL)) { - // The variable is being dereferenced for a read or a write. - comp->lvaTable[lclNum].lvIsPtr = 1; + return WALK_SKIP_SUBTREES; } - if (pState->isStoreSrc) + if ((user != nullptr) && user->OperIs(GT_SELECT) && (node == user->AsConditional()->gtCond)) { - // - // Add lvAssignDef and lclNum to a common assign group - if (shadowVarInfo[pState->lvStoreDef].assignGroup) - { - if (shadowVarInfo[lclNum].assignGroup) - { - // OR both bit vector - shadowVarInfo[pState->lvStoreDef].assignGroup->bitVectOr(shadowVarInfo[lclNum].assignGroup); - } - else - { - shadowVarInfo[pState->lvStoreDef].assignGroup->bitVectSet(lclNum); - } - - // Point both to the same bit vector - shadowVarInfo[lclNum].assignGroup = shadowVarInfo[pState->lvStoreDef].assignGroup; - } - else if (shadowVarInfo[lclNum].assignGroup) - { - shadowVarInfo[lclNum].assignGroup->bitVectSet(pState->lvStoreDef); + // The condition of a select does not contribute to the value + return WALK_SKIP_SUBTREES; + } - // Point both to the same bit vector - shadowVarInfo[pState->lvStoreDef].assignGroup = shadowVarInfo[lclNum].assignGroup; - } - else - { - FixedBitVect* bv = FixedBitVect::bitVectInit(pState->comp->lvaCount, pState->comp); - - // (shadowVarInfo[pState->lvAssignDef] == NULL && shadowVarInfo[lclNew] == NULL); - // Neither of them has an assign group yet. Make a new one. - shadowVarInfo[pState->lvStoreDef].assignGroup = bv; - shadowVarInfo[lclNum].assignGroup = bv; - bv->bitVectSet(pState->lvStoreDef); - bv->bitVectSet(lclNum); - } + if (node->OperIs(GT_LCL_VAR, GT_LCL_FLD)) + { + m_visit(node->AsLclVarCommon()->GetLclNum()); } + return WALK_CONTINUE; } + }; - // Calls - Mark arg variables - case GT_CALL: + Visitor visitor(this, visit); + visitor.WalkTree(&node, nullptr); +} - newState.isUnderIndir = false; - newState.isStoreSrc = false; +//------------------------------------------------------------------------ +// gsMarkPointers: +// Mark that dependent locals of the specified tree are pointers. +// +// Parameters: +// tree - The tree, typically an indirection +// +void Compiler::gsMarkPointers(GenTree* tree) +{ + gsVisitDependentLocals(tree, [=](unsigned lclNum) { + LclVarDsc* varDsc = lvaGetDesc(lclNum); + +#ifdef DEBUG + if (!varDsc->lvIsPtr) + { + JITDUMP("Marking V%02u as a pointer because of [%06u]\n", lclNum, dspTreeID(tree)); + } +#endif + + varDsc->lvIsPtr = 1; + }); +} + +//------------------------------------------------------------------------ +// gsFindVulnerableParams: +// Walk all the trees looking for ptrs, args, assign groups, *p stores, etc. +// Then use that info to figure out vulnerable pointers. +// +// Returns: +// Returns true if it found atleast one vulnerable pointer parameter that +// needs to be shadow-copied. +// +bool Compiler::gsFindVulnerableParams() +{ + for (BasicBlock* block : Blocks()) + { + for (GenTree* node : LIR::AsRange(block)) + { + switch (node->OperGet()) { - CallArg* thisArg = tree->AsCall()->gtArgs.GetThisArg(); - if (thisArg != nullptr) + case GT_IND: + case GT_BLK: + case GT_MDARR_LENGTH: + case GT_MDARR_LOWER_BOUND: + case GT_STOREIND: + case GT_STORE_BLK: { - // TODO-ARGS: This is a quirk for previous behavior where - // we set this to true for the 'this' arg. The flag can - // then remain set after the recursive call, depending on - // what the child node is, e.g. GT_ARGPLACE did not clear - // the flag, so when processing the second arg we would - // also have isUnderIndir = true. - newState.isUnderIndir = true; + gsMarkPointers(node->gtGetOp1()); + break; } - - for (CallArg& arg : tree->AsCall()->gtArgs.EarlyArgs()) + case GT_STORE_LCL_VAR: + case GT_STORE_LCL_FLD: { - comp->fgWalkTreePre(&arg.EarlyNodeRef(), gsMarkPtrsAndAssignGroups, &newState); + GenTreeLclVarCommon* lcl = node->AsLclVarCommon(); + gsVisitDependentLocals(lcl->Data(), [=](unsigned lclNum) { + gsUnionAssignGroups(lcl->GetLclNum(), lclNum, lcl); + }); + + break; } - for (CallArg& arg : tree->AsCall()->gtArgs.LateArgs()) + case GT_CALL: { - comp->fgWalkTreePre(&arg.LateNodeRef(), gsMarkPtrsAndAssignGroups, &newState); - } + CallArg* thisArg = node->AsCall()->gtArgs.GetThisArg(); + if (thisArg != nullptr) + { + gsMarkPointers(thisArg->GetNode()); + } - if (tree->AsCall()->gtCallType == CT_INDIRECT) - { - newState.isUnderIndir = true; + if (node->AsCall()->gtCallType == CT_INDIRECT) + { + // A function pointer is treated like a write-through pointer since + // it controls what code gets executed, and so indirectly can cause + // a write to memory. + gsMarkPointers(node->AsCall()->gtCallAddr); + } - // A function pointer is treated like a write-through pointer since - // it controls what code gets executed, and so indirectly can cause - // a write to memory. - comp->fgWalkTreePre(&tree->AsCall()->gtCallAddr, gsMarkPtrsAndAssignGroups, &newState); + break; } + default: + break; } - return WALK_SKIP_SUBTREES; - - case GT_STORE_LCL_VAR: - case GT_STORE_LCL_FLD: - newState.lvStoreDef = tree->AsLclVarCommon()->GetLclNum(); - newState.isStoreSrc = true; - comp->fgWalkTreePre(&tree->AsLclVarCommon()->Data(), gsMarkPtrsAndAssignGroups, &newState); - return WALK_SKIP_SUBTREES; - - default: - return WALK_CONTINUE; + } } -} - -/***************************************************************************** - * gsFindVulnerableParams - * Walk all the trees looking for ptrs, args, assign groups, *p stores, etc. - * Then use that info to figure out vulnerable pointers. - * - * It returns true if it found atleast one vulnerable pointer parameter that - * needs to be shadow-copied. - */ - -bool Compiler::gsFindVulnerableParams() -{ - MarkPtrsInfo info; - - info.comp = this; - info.lvStoreDef = (unsigned)-1; - info.isUnderIndir = false; - info.isStoreSrc = false; - info.skipNextNode = false; - - // Walk all the trees setting lvIsPtr and assignGroup. - fgWalkAllTreesPre(gsMarkPtrsAndAssignGroups, &info); // Compute has vulnerable at the end of the loop. bool hasOneVulnerable = false; @@ -303,7 +311,7 @@ bool Compiler::gsFindVulnerableParams() // some assign group. FixedBitVect* propagated = (lvaCount > 0) ? FixedBitVect::bitVectInit(lvaCount, this) : nullptr; - for (UINT lclNum = 0; lclNum < lvaCount; lclNum++) + for (unsigned lclNum = 0; lclNum < lvaCount; lclNum++) { LclVarDsc* varDsc = lvaGetDesc(lclNum); ShadowParamVarInfo* shadowInfo = &gsShadowVarInfo[lclNum]; @@ -323,12 +331,12 @@ bool Compiler::gsFindVulnerableParams() // Propagate lvIsPtr, so that: // 1. Any parameter in the equivalence class can be identified as lvIsPtr and hence shadowed. // 2. Buffers with pointers are placed at lower memory addresses than buffers without pointers. - UINT isUnderIndir = varDsc->lvIsPtr; + bool isUnderIndir = varDsc->lvIsPtr; // First pass -- find if any variable is vulnerable. FixedBitVect* assignGroup = shadowInfo->assignGroup; - for (UINT lclNum = assignGroup->bitVectGetFirst(); lclNum != (unsigned)-1 && !isUnderIndir; - lclNum = assignGroup->bitVectGetNext(lclNum)) + for (unsigned lclNum = assignGroup->bitVectGetFirst(); lclNum != (unsigned)-1 && !isUnderIndir; + lclNum = assignGroup->bitVectGetNext(lclNum)) { isUnderIndir |= lvaTable[lclNum].lvIsPtr; } @@ -346,8 +354,8 @@ bool Compiler::gsFindVulnerableParams() // Second pass -- mark all are vulnerable. assert(isUnderIndir); - for (UINT lclNum = assignGroup->bitVectGetFirst(); lclNum != (unsigned)-1; - lclNum = assignGroup->bitVectGetNext(lclNum)) + for (unsigned lclNum = assignGroup->bitVectGetFirst(); lclNum != (unsigned)-1; + lclNum = assignGroup->bitVectGetNext(lclNum)) { lvaTable[lclNum].lvIsPtr = true; propagated->bitVectSet(lclNum); @@ -357,8 +365,8 @@ bool Compiler::gsFindVulnerableParams() if (verbose) { printf("Equivalence assign group %s: ", isUnderIndir ? "isPtr " : ""); - for (UINT lclNum = assignGroup->bitVectGetFirst(); lclNum != (unsigned)-1; - lclNum = assignGroup->bitVectGetNext(lclNum)) + for (unsigned lclNum = assignGroup->bitVectGetFirst(); lclNum != (unsigned)-1; + lclNum = assignGroup->bitVectGetNext(lclNum)) { gtDispLclVar(lclNum, false); printf(" "); @@ -372,16 +380,145 @@ bool Compiler::gsFindVulnerableParams() } //------------------------------------------------------------------------------- -// gsParamsToShadows: Copy each vulnerable param ptr or buffer to a local shadow -// copy and replace uses of the param by the shadow copy. +// gsParamsToShadows: +// Copy each vulnerable param ptr or buffer to a local shadow copy and +// replace uses of the param by the shadow copy. +// void Compiler::gsParamsToShadows() { - // Cache old count since we'll add new variables, and - // gsShadowVarInfo will not grow to accommodate the new ones. - UINT lvaOldCount = lvaCount; + // Create the locals that shadow the parameters + if (!gsCreateShadowingLocals()) + { + return; + } + + // Redirect uses in the IR to the shadowed versions. + for (BasicBlock* block : Blocks()) + { + for (GenTree* tree : LIR::AsRange(block)) + { + gsRewriteTreeForShadowParam(tree); + } + } + + // Now insert code to copy the params to their shadow copy at the beginning of the function. + LIR::Range range; + for (unsigned lclNum = 0; lclNum < gsShadowVarInfoCount; lclNum++) + { + const LclVarDsc* varDsc = lvaGetDesc(lclNum); + + const unsigned shadowLclNum = gsShadowVarInfo[lclNum].shadowCopy; + if (shadowLclNum == BAD_VAR_NUM) + { + continue; + } + + gsCopyIntoShadow(lclNum, shadowLclNum, range); + } + + // Mark some of the new locals as having explicit inits. + if (opts.OptimizationEnabled()) + { + bool hasGCSafePoint = false; + for (GenTree* node : range) + { + hasGCSafePoint |= IsPotentialGCSafePoint(node); + if (node->OperIsLocalStore()) + { + LclVarDsc* lclDsc = lvaGetDesc(node->AsLclVarCommon()); + // Keep this in sync with optRemoveRedundantZeroInits + if (!lclDsc->HasGCPtr() || (!GetInterruptible() && !hasGCSafePoint)) + { + lclDsc->lvHasExplicitInit = true; + node->gtFlags |= GTF_VAR_EXPLICIT_INIT; + JITDUMP("Marking V%02u has having an explicit init\n", node->AsLclVarCommon()->GetLclNum()); + } + } + } + } + + // Insert the IR + if (opts.IsReversePInvoke()) + { + GenTree* insertAfter = nullptr; + // If we are in a reverse P/Invoke then insert after the GC transition. + // Struct stores can turn into calls, and x86 can use special copy + // helpers, so take care to transition to coop first. + // + // TODO-Cleanup: We should be inserting reverse pinvoke transitions way + // later in the JIT to avoid having to search like this. + + for (GenTree* node : LIR::AsRange(fgFirstBB)) + { + if (node->IsHelperCall(this, CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER) || + node->IsHelperCall(this, CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER_TRACK_TRANSITIONS)) + { + insertAfter = node; + break; + } + } + + noway_assert(insertAfter != nullptr); + + JITDUMP("Inserting IR after Reverse P/Invoke transition [%06u]\n", dspTreeID(insertAfter)); + LIR::AsRange(fgFirstBB).InsertAfter(insertAfter, std::move(range)); + } + else + { + JITDUMP("Inserting IR at beginning of fgFirstBB\n"); + LIR::AsRange(fgFirstBB).InsertAtBeginning(std::move(range)); + } + + // If the method has "Jmp CalleeMethod", then we need to copy shadow params back to original + // params before "jmp" to CalleeMethod. + if (compJmpOpUsed) + { + // There could be more than one basic block ending with a "Jmp" type tail call. + // We would have to insert stores in all such blocks, just before GT_JMP stmnt. + for (BasicBlock* const block : Blocks()) + { + if (!block->KindIs(BBJ_RETURN)) + { + continue; + } + + GenTree* lastNode = LIR::AsRange(block).LastNode(); + if ((lastNode == nullptr) || !lastNode->OperIs(GT_JMP)) + { + continue; + } + + for (unsigned lclNum = 0; lclNum < info.compArgsCount; lclNum++) + { + const LclVarDsc* varDsc = lvaGetDesc(lclNum); + const unsigned shadowVarNum = gsShadowVarInfo[lclNum].shadowCopy; + if (shadowVarNum == BAD_VAR_NUM) + { + continue; + } + + GenTree* src = gtNewLclVarNode(shadowVarNum); + GenTree* store = gtNewStoreLclVarNode(lclNum, src); + + LIR::AsRange(fgFirstBB).InsertBefore(lastNode, LIR::SeqTree(this, store)); + } + } + } +} + +//------------------------------------------------------------------------------- +// gsCreateShadowingLocals: +// For each parameter that should be shadowed, create a local variable to hold its copy. +// +// Returns: +// True if any shadowing locals were created.. +// +bool Compiler::gsCreateShadowingLocals() +{ + bool createdAny = false; // Create shadow copy for each param candidate - for (UINT lclNum = 0; lclNum < lvaOldCount; lclNum++) + for (unsigned lclNum = 0; lclNum < gsShadowVarInfoCount; lclNum++) { LclVarDsc* varDsc = lvaGetDesc(lclNum); gsShadowVarInfo[lclNum].shadowCopy = BAD_VAR_NUM; @@ -397,7 +534,7 @@ void Compiler::gsParamsToShadows() continue; } - int shadowVarNum = lvaGrabTemp(false DEBUGARG("shadowVar")); + int shadowVarNum = lvaGrabTemp(false DEBUGARG(printfAlloc("V%02u shadow", lclNum))); // reload varDsc as lvaGrabTemp may realloc the lvaTable[] varDsc = lvaGetDesc(lclNum); LclVarDsc* shadowVarDsc = lvaGetDesc(shadowVarNum); @@ -407,7 +544,10 @@ void Compiler::gsParamsToShadows() shadowVarDsc->lvType = type; shadowVarDsc->lvRegStruct = varDsc->lvRegStruct; shadowVarDsc->SetAddressExposed(varDsc->IsAddressExposed() DEBUGARG(varDsc->GetAddrExposedReason())); - shadowVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister; + shadowVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister; + shadowVarDsc->lvSingleDefRegCandidate = varDsc->lvSingleDefRegCandidate; + // The old variable will not be used in handlers anymore, allow it to stay enregistered + varDsc->lvLiveInOutOfHndlr = false; #ifdef DEBUG shadowVarDsc->SetDoNotEnregReason(varDsc->GetDoNotEnregReason()); shadowVarDsc->SetDefinedViaAddress(varDsc->IsDefinedViaAddress()); @@ -438,185 +578,101 @@ void Compiler::gsParamsToShadows() #endif gsShadowVarInfo[lclNum].shadowCopy = shadowVarNum; + createdAny = true; } - class ReplaceShadowParamsVisitor final : public GenTreeVisitor - { - // Walk the locals of the method (i.e. GT_LCL_FLD and GT_LCL_VAR nodes) and replace the ones that correspond to - // "vulnerable" parameters with their shadow copies. If an original local variable has small type then replace - // the GT_LCL_VAR node type with TYP_INT. - public: - enum - { - DoPostOrder = true - }; - - ReplaceShadowParamsVisitor(Compiler* compiler) - : GenTreeVisitor(compiler) - { - } - - Compiler::fgWalkResult PostOrderVisit(GenTree** use, GenTree* user) - { - GenTree* tree = *use; - - if (tree->OperIsAnyLocal()) - { - unsigned int lclNum = tree->AsLclVarCommon()->GetLclNum(); - unsigned int shadowLclNum = m_compiler->gsShadowVarInfo[lclNum].shadowCopy; - - if (shadowLclNum != BAD_VAR_NUM) - { - LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum); - assert(ShadowParamVarInfo::mayNeedShadowCopy(varDsc)); - - tree->AsLclVarCommon()->SetLclNum(shadowLclNum); - - if (varTypeIsSmall(varDsc)) - { - if (tree->OperIsScalarLocal()) - { - tree->gtType = TYP_INT; - } - else if (tree->OperIs(GT_STORE_LCL_FLD) && tree->IsPartialLclFld(m_compiler)) - { - tree->gtFlags |= GTF_VAR_USEASG; - } - } - } - } - - return WALK_CONTINUE; - } - }; + return createdAny; +} - for (BasicBlock* const block : Blocks()) +//------------------------------------------------------------------------------- +// gsRewriteTreeForShadowParam: +// If necessary, rewrite the given tree to act on the shadowed version of a local. +// +// Parameters: +// tree - The tree +// +void Compiler::gsRewriteTreeForShadowParam(GenTree* tree) +{ + if (!tree->OperIsAnyLocal()) { - for (Statement* const stmt : block->Statements()) - { - ReplaceShadowParamsVisitor replaceShadowParamsVisitor(this); - replaceShadowParamsVisitor.WalkTree(stmt->GetRootNodePointer(), nullptr); - } + return; } - compCurBB = fgFirstBB; - // Now insert code to copy the params to their shadow copy. - for (UINT lclNum = 0; lclNum < lvaOldCount; lclNum++) + unsigned int lclNum = tree->AsLclVarCommon()->GetLclNum(); + if (lclNum >= gsShadowVarInfoCount) { - const LclVarDsc* varDsc = lvaGetDesc(lclNum); - - const unsigned shadowVarNum = gsShadowVarInfo[lclNum].shadowCopy; - if (shadowVarNum == BAD_VAR_NUM) - { - continue; - } - -#if defined(TARGET_X86) && defined(FEATURE_IJW) - if (lclNum < info.compArgsCount && argRequiresSpecialCopy(lclNum) && varDsc->TypeIs(TYP_STRUCT)) - { - JITDUMP("arg%02u requires special copy, using special copy helper to copy to shadow var V%02u\n", lclNum, - shadowVarNum); - CORINFO_METHOD_HANDLE copyHelper = - info.compCompHnd->getSpecialCopyHelper(varDsc->GetLayout()->GetClassHandle()); - GenTreeCall* call = gtNewCallNode(CT_USER_FUNC, copyHelper, TYP_VOID); - - GenTree* src = gtNewLclVarAddrNode(lclNum); - GenTree* dst = gtNewLclVarAddrNode(shadowVarNum); - - call->gtArgs.PushBack(this, NewCallArg::Primitive(dst)); - call->gtArgs.PushBack(this, NewCallArg::Primitive(src)); - - compCurBB = fgFirstBB; // Needed by some morphing - if (opts.IsReversePInvoke()) - { - // If we are in a reverse P/Invoke, then we need to insert - // the call at the end of the first block as we need to do the GC transition - // before we can call the helper. - // - // TODO-Cleanup: These gymnastics indicate that we are - // inserting reverse pinvoke transitions way too early in the - // JIT. - - auto isReversePInvoke = [=](GenTree* tree) { - return tree->IsHelperCall(this, CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER) || - tree->IsHelperCall(this, CORINFO_HELP_JIT_REVERSE_PINVOKE_ENTER_TRACK_TRANSITIONS); - }; - - Statement* reversePInvokeStmt = nullptr; - for (Statement* const stmt : fgFirstBB->Statements()) - { - // assert that we don't have any uses of the local variable - // at the point before we insert the shadow copy statement. - assert(!gtHasRef(stmt->GetRootNode(), lclNum)); + return; + } - if (gtFindNodeInTree(stmt->GetRootNode(), isReversePInvoke) != nullptr) - { - reversePInvokeStmt = stmt; - break; - } - } + unsigned int shadowLclNum = gsShadowVarInfo[lclNum].shadowCopy; + if (shadowLclNum == BAD_VAR_NUM) + { + return; + } - noway_assert(reversePInvokeStmt != nullptr); + LclVarDsc* varDsc = lvaGetDesc(lclNum); + assert(ShadowParamVarInfo::mayNeedShadowCopy(varDsc)); - JITDUMP("Inserting special copy helper call after Reverse P/Invoke transition " FMT_STMT "\n", - reversePInvokeStmt->GetID()); + tree->AsLclVarCommon()->SetLclNum(shadowLclNum); - (void)fgInsertStmtAfter(fgFirstBB, reversePInvokeStmt, gtNewStmt(fgMorphTree(call))); - } - else - { - JITDUMP("Inserting special copy helper call at the beginning of the first block\n"); - (void)fgNewStmtAtBeg(fgFirstBB, fgMorphTree(call)); - } + if (varTypeIsSmall(varDsc)) + { + if (tree->OperIsScalarLocal()) + { + tree->gtType = TYP_INT; } - else -#endif // TARGET_X86 && FEATURE_IJW + else if (tree->OperIs(GT_STORE_LCL_FLD) && tree->IsPartialLclFld(this)) { - GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet()); - src->gtFlags |= GTF_DONT_CSE; - - GenTree* store = gtNewStoreLclVarNode(shadowVarNum, src); - - compCurBB = fgFirstBB; // Needed by some morphing - fgNewStmtAtBeg(fgFirstBB, fgMorphTree(store)); + tree->gtFlags |= GTF_VAR_USEASG; } } - compCurBB = nullptr; +} - // If the method has "Jmp CalleeMethod", then we need to copy shadow params back to original - // params before "jmp" to CalleeMethod. - if (compJmpOpUsed) +//------------------------------------------------------------------------------- +// gsCopyIntoShadow: +// Insert IR to copy the value of "lclNum" into "shadowLclNum" at the +// beginning of the function. +// +// Parameters: +// lclNum - The original vulnerable local +// shadowLclNum - The shadowing local +// +void Compiler::gsCopyIntoShadow(unsigned lclNum, unsigned shadowLclNum, LIR::Range& range) +{ + LclVarDsc* varDsc = lvaGetDesc(lclNum); + if (varDsc->lvPromoted && !varDsc->lvDoNotEnregister) { - // There could be more than one basic block ending with a "Jmp" type tail call. - // We would have to insert stores in all such blocks, just before GT_JMP stmnt. - for (BasicBlock* const block : Blocks()) - { - if (!block->KindIs(BBJ_RETURN)) - { - continue; - } - - if (!block->HasFlag(BBF_HAS_JMP)) - { - continue; - } - - for (UINT lclNum = 0; lclNum < info.compArgsCount; lclNum++) - { - const LclVarDsc* varDsc = lvaGetDesc(lclNum); + lvaSetVarDoNotEnregister(lclNum DEBUGARG(DoNotEnregisterReason::BlockOp)); + } - const unsigned shadowVarNum = gsShadowVarInfo[lclNum].shadowCopy; - if (shadowVarNum == BAD_VAR_NUM) - { - continue; - } +#if defined(TARGET_X86) && defined(FEATURE_IJW) + if (lclNum < info.compArgsCount && argRequiresSpecialCopy(lclNum) && varDsc->TypeIs(TYP_STRUCT)) + { + JITDUMP("arg%02u requires special copy, using special copy helper to copy to shadow var V%02u\n", lclNum, + shadowLclNum); + CORINFO_METHOD_HANDLE copyHelper = + info.compCompHnd->getSpecialCopyHelper(varDsc->GetLayout()->GetClassHandle()); + GenTreeCall* call = gtNewCallNode(CT_USER_FUNC, copyHelper, TYP_VOID); + + GenTree* src = gtNewLclVarAddrNode(lclNum); + GenTree* dst = gtNewLclVarAddrNode(shadowLclNum); + + call->gtArgs.PushBack(this, NewCallArg::Primitive(dst)); + call->gtArgs.PushBack(this, NewCallArg::Primitive(src)); + + compCurBB = fgFirstBB; // Needed by some morphing + fgMorphTree(call); + compCurBB = nullptr; + range.InsertAtEnd(LIR::SeqTree(this, call)); + DISPTREERANGE(range, call); + return; + } +#endif - GenTree* src = gtNewLclVarNode(shadowVarNum); - src->gtFlags |= GTF_DONT_CSE; - GenTree* store = gtNewStoreLclVarNode(lclNum, src); + GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet()); + GenTree* store = gtNewStoreLclVarNode(shadowLclNum, src); - (void)fgNewStmtNearEnd(block, fgMorphTree(store)); - } - } - } + range.InsertAtBeginning(src, store); + JITDUMP("Created shadow param copy for V%02u to V%02u\n", lclNum, shadowLclNum); + DISPTREERANGE(range, store); } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 736e3163d27045..b7e23ab8a2c1ff 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -5671,6 +5671,7 @@ void Compiler::optRemoveRedundantZeroInits() // insert a call to CORINFO_HELP_INIT_PINVOKE_FRAME but that is not a gc-safe point. assert(s_helperCallProperties.IsNoGC(CORINFO_HELP_INIT_PINVOKE_FRAME)); + // Keep this in sync with gsParamsToShadows if (!lclDsc->HasGCPtr() || (!GetInterruptible() && !hasGCSafePoint)) { // The local hasn't been used and won't be reported to the gc between