diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 89e5f28d1bd447..c379281b8967de 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3668,6 +3668,8 @@ class Compiler // Returns "true" iff "tree" or its (transitive) children have any of the side effects in "flags". bool gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags, bool ignoreCctors = false); + static bool gtTreeMayHaveInvalidByrefs(Compiler* compiler, GenTree* tree); + void gtExtractSideEffList(GenTree* expr, GenTree** pList, GenTreeFlags GenTreeFlags = GTF_SIDE_EFFECT, diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d44e0bf76a1a91..b026681f38cb95 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17160,6 +17160,55 @@ bool Compiler::gtNodeHasSideEffects(GenTree* tree, GenTreeFlags flags, bool igno return false; } +//----------------------------------------------------------- +// gtTreeMayHaveInvalidByrefs: Returns true if the given tree may contain potentially invalid byrefs. +// Technically, we should place a special side-effect flag on all TYP_BYREF-typed nodes until +// they are proven to be unconditionally valid or not pointing to the heap. We don't +// do that today, as it would likely cause a significant size regression. +// Consequently, any code that speculatively hoists nodes with byrefs must call this +// method to verify the tree and avoid hoisting if potentially invalid byrefs are present. +// We also assume that all TYP_REF nodes are always valid. All attempts to have some kinds +// of tagged unions of unmanaged + managed fields are UB. +// +// Arguments: +// compiler - The compiler instance to use for the walk. +// tree - The tree to check. +// +// Return Value: +// True if the tree may contain potentially invalid byrefs; false otherwise. +// +bool Compiler::gtTreeMayHaveInvalidByrefs(Compiler* compiler, GenTree* tree) +{ + class ByrefSideEffectVisitor final : public GenTreeVisitor + { + public: + ByrefSideEffectVisitor(Compiler* compiler) + : GenTreeVisitor(compiler) + { + } + + enum + { + DoPreOrder = true, + }; + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* tree = *use; + // It is possible to be more precise, but it is not required by the contract. + if (tree->TypeIs(TYP_BYREF) && !tree->OperIs(GT_LCL_VAR, GT_STORE_LCL_VAR, GT_RETURN) && + !tree->IsIntegralConst(0)) + { + return WALK_ABORT; + } + return WALK_CONTINUE; + } + }; + + ByrefSideEffectVisitor visitor(compiler); + return visitor.WalkTree(&tree, nullptr) == WALK_ABORT; +} + /***************************************************************************** * Returns true if the expr tree has any side effects. */ diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp index 0acda646e0d9e6..3e271a18c630da 100644 --- a/src/coreclr/jit/ifconversion.cpp +++ b/src/coreclr/jit/ifconversion.cpp @@ -690,6 +690,13 @@ bool OptIfConversionDsc::optIfConvert(int* pReachabilityBudget) } } + if (Compiler::gtTreeMayHaveInvalidByrefs(m_compiler, m_thenOperation.node) || + (m_doElseConversion && Compiler::gtTreeMayHaveInvalidByrefs(m_compiler, m_elseOperation.node))) + { + JITDUMP("Skipping if-conversion that may have invalid byrefs in the then or else block\n"); + return false; + } + // Get the select node inputs. var_types selectType; GenTree* selectTrueInput;