JIT: (bug) XOR-based rotation recognition produces a wrong result when the shift count is a multiple of the operand width
- 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
Assessment
This issue has not been assessed yet.