Fix ifConversion hoisting of invalid managed references

[Copilot]

Description

ifConversion can speculatively hoist potentially invalid byrefs into branchless code (cmov/csel), which may crash the GC. The root cause is that byref constructions don't light up any side-effect flags, so the existing varTypeIsIntegralOrI() check on the root node is insufficient — intermediate nodes in the operand tree may produce invalid GC-tracked references that would be unconditionally evaluated.

byte Problem(ref byte x, int len)
{
    ref byte t = ref (len == 0 ?
        ref Unsafe.NullRef<byte>() :
        ref Unsafe.Add(ref x, 100));
    return t;
}

Before fix — both sides are unconditionally evaluated:

xor      rax, rax
add      rdx, 100       ; computes potentially invalid byref
cmovne   rax, rdx       ; selects between them

After fix — branches preserved, invalid byref never materialized:

test     esi, esi
jne      SHORT G_M000_IG04
xor      rax, rax
jmp      SHORT G_M000_IG05
lea      rax, [rdi+0x64]

Changes

• src/coreclr/jit/ifconversion.cpp: Added a ContainsGCType static helper that recursively scans the entire operand tree for any TYP_BYREF or TYP_REF typed node. Applied it to the value trees in both GT_STORE_LCL_VAR (on op1) and GT_RETURN (on retVal) paths in IfConvertCheckStmts, preventing if-conversion when GC types are found anywhere in the value tree.
• src/tests/JIT/opt/IfConversion/ByrefConversion.cs: Regression test with X64-NOT: cmov and ARM64-NOT: csel FileCheck assertions.

The optimizebools phase was also checked and is not affected — it only combines boolean branch conditions with AND/OR operators, it does not produce GT_SELECT or conditional moves.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>ifConversion can hoist invalid managed references.</issue_title>
<issue_description>We shouldn't speculatively hoist potentially invalid byrefs as those may lead to GC crashes.

byte Problem(ref byte x, int len)
{
    ref byte t = ref (len == 0 ? 
        ref Unsafe.NullRef<byte>() : 
        ref Unsafe.Add(ref x, 100));
    return t;
}

Codegen:

; Method Benchmarks:Problem(byref,int):byte:this (FullOpts)
G_M23671_IG01:        ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0000 {}, byref, nogc
G_M23671_IG02:        ; bbWeight=1, gcrefRegs=0000 {}, byrefRegs=0004 {rdx}, byref
                            ; byrRegs +[rdx]
       xor      rax, rax
                            ; byrRegs +[rax]
       add      rdx, 100
       test     r8d, r8d
       cmovne   rax, rdx
       movzx    rax, byte  ptr [rax]
                            ; byrRegs -[rax]
G_M23671_IG03:        ; bbWeight=1, epilog, nogc, extend
       ret      
; Total bytes of code: 17
```</issue_description>

<agent_instructions>Please fix the possible bug, add a jit regression test with FILECHECK that makes sure no brnachless instruction is emitted (both x64 and arm64) like cmov*</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

• Fixes ifConversion can hoist invalid managed references. #124807

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[EgorBo]

@copilot that is likely not enough. Since byref constructions currently don't light up any side-effects, we have to scan the whole tree for any TYP_BYREF typed node (and, presumably, GT_INT_CNS with TYP_REF type). Also, please check if optimizebools phase can possibly produce these.

[Copilot]

@copilot that is likely not enough. Since byref constructions currently don't light up any side-effects, we have to scan the whole tree for any TYP_BYREF typed node (and, presumably, G...

Updated in 02e3143. The fix now adds a ContainsGCType helper that recursively scans the entire operand tree (op1 for GT_STORE_LCL_VAR, retVal for GT_RETURN) for any TYP_BYREF or TYP_REF typed node, rather than only checking the root node type. This catches cases where intermediate byref computations (like Unsafe.Add) are buried in the tree but don't have side-effect flags.

Regarding optimizebools: I checked and it does not produce GT_SELECT or conditional moves — it only combines boolean branch conditions (two BBJ_COND blocks) into a single block with AND/OR operators on the comparands. The GC handling there (lines 200, 1325) is already scoped to its own transformations (type widening for fold ops and GC-null comparand folding), so it shouldn't be affected.