dotnet / dotnet/runtime

JIT: (bug) XOR-based rotation recognition produces a wrong result when the shift count is a multiple of the operand width

Open
#133,864 1 comment 1 reaction 2 assignees Claimed by @EgorBo View on GitHub
area-CodeGen-coreclr
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

`fgOperIsBitwiseRotationRoot` accepts `GT_XOR` as a rotation root, but `OR` and `XOR` are only interchangeable when
the two shifted values have disjoint bits. With a variable count that is a multiple of the operand width both
sub-shifts equal `x`, so the `XOR` must be `0` while `ROL(x, 0)` is `x`.

### Minimal Repro

```csharp
using System;
using System.Runtime.CompilerServices;

public class Program
{
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
static int Test(int x, int y) => (x << (y & 31)) ^ (int)((uint)x >>> ((32 - y) & 31));

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
static long TestLong(long x, int y) => (x << (y & 63)) ^ (long)((ulong)x >>> ((64 - y) & 63));

static void Main()
{
Console.WriteLine(Test(0x12345678, 0));
Console.WriteLine(Test(0x12345678, 32));
Console.WriteLine(TestLong(0x1234, 0));
}
}
```

Both shift counts are explicitly masked, so no unspecified out-of-range shift behavior is involved.

### Expected

```
0
0
0
```

(also what `DOTNET_JITMinOpts=1` produces)

### Actual

```
305419896
305419896
4660
```

The entire `XOR` tree collapses to a single `rol`, which is a no-op for count `0`:

```asm
; Program:Test(int,int):int (FullOpts)
mov eax, ecx
mov ecx, edx
rol eax, cl
ret
```

### Notes

`fgRecognizeAndMorphBitwiseRotation` (`morph.cpp`) has no nonzero-count requirement for the `GT_XOR` root.
The `GT_OR` root is fine — `(x << 0) | (x >>> N)` yields `x`, matching `ROL(x, 0)`.
Only the `GT_XOR` root with a *variable* rotate amount is affected; constant counts of `0` are already rejected
earlier by the overmask check.
Possible fix: only allow a `GT_XOR` root when the rotate amount is provably not a multiple of the operand width.
Also reproduces on .NET 10.0.12.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.