Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 48 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17160,6 +17160,54 @@ 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<ByrefSideEffectVisitor>
{
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))
{
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.
*/
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/jit/ifconversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Comment on lines +693 to +698
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The invalid-byref guard only inspects the original then/else statement trees. In the no-else case, if-conversion synthesizes a new GT_LCL_VAR read of the destination local (see later selectTrueInput = gtNewLclVarNode(...)), which can introduce an unconditionally-evaluated byref that wasn’t present before. Consider bailing out when !m_doElseConversion and the destination local is TYP_BYREF, or run the invalid-byref check on the final select inputs (including the synthesized one) before proceeding.

Copilot uses AI. Check for mistakes.

// Get the select node inputs.
var_types selectType;
GenTree* selectTrueInput;
Expand Down
Loading