镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

JIT: stale ZF after rotate causes wrong-direction branch in if (BitOperations.RotateRight(x, n) == 0) pattern #129288

Closed

Description

@AndyAyersMS

Summary

The x64 JIT emits ror reg, 1 followed directly by jne to test whether
BitOperations.RotateRight(value, 1) is non-zero, but ROR does not
modify ZF on x64. The jne therefore reads stale ZF from a prior
unrelated cmp, producing wrong-direction branches and corrupting control
flow. Discovered by an experimental in-development fuzzer.

Note

This issue was authored by GitHub Copilot CLI on @AndyAyersMS's machine,
based on a bug surfaced by an experimental in-development fuzzer.
The C# repro is verified.

Root cause

GenTree::SupportsSettingZeroFlag() in
gentree.cpp:21685
treats rotates the same as shifts:

#if defined(TARGET_XARCH)
    if (OperIs(GT_LSH, GT_RSH, GT_RSZ, GT_ROL, GT_ROR))
    {
        // Shift/Rotate instructions do not update the flags in case of count being zero.
        return gtGetOp2()->IsNeverZero();
    }

Lowering uses this signal to drop a TEST/CMP before a zero-comparison
branch. That is correct for shifts (which set ZF when count != 0) but
wrong for rotates: per the Intel SDM, ROL/ROR/RCL/RCR only
affect CF (and OF for 1-bit form). SF, ZF, AF, PF are not
affected, regardless of count.
So even with a non-zero count, a rotate
does not produce a meaningful ZF.

GT_ROL and GT_ROR were added to the supports-zero-flag set in
#113037 (Apr 2025) and
the partial repair in
#117960 (Jul 2025)
moved the IsNeverZero check but kept rotates in the same block.

The proposed fix is to split them:

if (OperIs(GT_LSH, GT_RSH, GT_RSZ))
{
    // Shift instructions do not update the flags in case of count being zero.
    return gtGetOp2()->IsNeverZero();
}
// GT_ROL / GT_ROR omitted: rotates never modify ZF on xarch.

Repro

C# source (verified to throw on FullOpts, succeed on MinOpts and tiered):

using System;
using System.Numerics;
using System.Runtime.CompilerServices;

public static class Program
{
    private static ulong Input_p0 = 0UL;
    private static ulong Input_p1 = 0UL;
    private static int[] __trace = new int[64];
    private static int __traceLen = 0;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public static ulong Fn(ulong p0, ulong p1)
    {
        unchecked
        {
            ulong v4 = 0UL, v11 = 0UL, v30 = 0UL, v33 = 0UL, v34 = 0UL;
            int v12 = 0;
        b0:
            __trace[__traceLen++] = 0;
            v4 = BitOperations.RotateLeft(p0, (int)(ulong)0x7FFFFFFFFFFFFFFFUL);
        b1:
            __trace[__traceLen++] = 1;
        b2:
            __trace[__traceLen++] = 2;
            v11 = BitOperations.RotateRight(v4, 1);
            v12 = (v11 < 1UL) ? 1 : 0;
            if (v12 != 0) goto b3;
            goto b4;
        b3:
            __trace[__traceLen++] = 3;
            goto b6;
        b4:
            __trace[__traceLen++] = 4;
            goto b7;
        b6:
            __trace[__traceLen++] = 6;
            goto b0;
        b7:
            __trace[__traceLen++] = 7;
            v34 = v33 % v30;   // dead in source semantics
            goto b0;
        }
    }

    public static int Main()
    {
        ulong result = Fn(Input_p0, Input_p1);
        Console.WriteLine($"{result:X16}");
        return 0;
    }
}

With DOTNET_TieredCompilation=0 (FullOpts), this throws
System.DivideByZeroException on the very first iteration of the loop.

Under the source semantics, control should go
b0 → b1 → b2 → b3 → b6 → b0 → ... repeatedly: v4 is
RotateLeft(p0=0, ...) = 0, so v11 = RotateRight(0, 1) = 0, so
v12 = (v11 < 1) ? 1 : 0 = 1, so the branch v12 != 0 is taken to
b3. Block b7 (with the v33 % v30 where both are 0) is
unreachable. With __trace sized at 64, the correct semantics eventually
overflow the trace array on iteration ~13 with
IndexOutOfRangeException; with default tiered or JitMinOpts=1 that
is what is observed.

Generated code (offending fragment, FullOpts x64)

G_M44088_IG04:        ; BB07 (main loop body)
    ...
    cmp   ebx, r11d            ; bounds check for __trace store (sets ZF)
    jae   SHORT G_M44088_IG06
    mov   dword ptr [rdx+4*rbx+0x10], 2
    ror   rax, 1               ; ROR does NOT modify ZF
    jne   G_M44088_IG03        ; reads STALE ZF from the cmp above

G_M44088_IG03:        ; BB04 -> b4/b7
    ...
    xor   eax, eax
    xor   r11d, r11d
    xor   edx, edx
    div   rdx:rax, r11         ; <-- DivideByZeroException

The cmp ebx, r11d immediately preceding the rotate-then-branch
compares the trace cursor with __trace.Length = 64. Since
ebx != 64, ZF = 0, and the jne is taken — into the unreachable
b4/b7 path that does % 0.

Environment

  • HEAD of main as of 2026-06-10 (Checked CoreCLR, x64 Windows).
  • Repros on corerun with DOTNET_TieredCompilation=0.
  • Discovered using ReifyCs (in-development C# fuzzer); seed 2000017594.

Suggested test

A regression test could synthesize any
if (BitOperations.RotateRight(x, n) == 0) pattern preceded by a
flag-setting cmp whose ZF differs from "is rotate result zero",
asserting the branch direction matches the interpreter.

Activity

  1. added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    on Jun 11, 2026
  2. changed the title [-]JIT: stale ZF after or/ ol causes wrong-direction branch in if (BitOperations.RotateRight(x, n) == 0) pattern[/-] [+]JIT: stale ZF after rotate causes wrong-direction branch in if (BitOperations.RotateRight(x, n) == 0) pattern[/+] on Jun 11, 2026
  3. dotnet-policy-service commented on Jun 11, 2026

    @dotnet-policy-service
    Contributor

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

  4. tannergooding commented on Jun 11, 2026

    @tannergooding
    Member

    We notably have this info in the instruction tables. For example, seehttps://github.com/dotnet/runtime/blob/main/src/coreclr/jit/instrsxarch.h#L1258-L1260, where ror is tracked as Undefined_OF | Writes_CF which means other flags (like ZF) are left "as is".

    It might be goodness to have most of the trivial arithmetic ops like this rather query the instruction codegen would emit and lookup the actual flags, rather than hardcode them all. Particularly because that may get out of sync, be missed when we fixup the general tables, or in the future if/when we change the emit strategy for a given node kind.

    Codegen itself typically has such a mapping of Oper -> ins already and while there is some nuance, the places we typically care about such queries (lowering) should know statically what codegen will emit (as this is required for containment and other LSRA required flags to be correct)

  5. removed
    untriagedNew issue has not been triaged by the area owner
    on Jun 12, 2026
  6. added this to the 11.0.0 milestone on Jun 12, 2026
  7. added a commit that references this issue on Jul 15, 2026
    ec31001
  8. locked and limited conversation to collaborators on Jul 15, 2026
  9. added a commit that references this issue on Jul 22, 2026
    4db0fde
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions