dotnet / dotnet/runtime

JIT: (bug) Rotation recognition drops a checked `add.ovf` in the shift count (missing OverflowException + Checked assert)

Closed
#133,863 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

`fgRecognizeAndMorphBitwiseRotation` matches the shift-count pattern `ADD(NEG(y), 32)` and deletes the addition
when rewriting `OR(LSH, RSZ)` into a `ROL`, without checking `gtOverflow()`. A required `OverflowException`
disappears, and the new `ROL` keeps a stale `GTF_EXCEPT` flag.

### 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 >>> (checked(unchecked(-y) + 32) & 31));

static void Main()
{
try { Console.WriteLine(Test(0x12345678, -2147483647)); }
catch (OverflowException) { Console.WriteLine("OverflowException"); }
}
}
```

With `y == -2147483647`, `unchecked(-y)` is `2147483647` and `checked(2147483647 + 32)` must throw before the
rotate is evaluated.

### Expected

```
OverflowException
```

### Actual

Checked JIT (`main`, x64) asserts instead of compiling the method (exit code `0xC0000409`):

```
Extra flags on tree [000017]: --X-------
[000017] ---X-+----- * ROL int
[000000] -----+----- +--* LCL_VAR int V00 arg0
[000022] -----+----- \--* AND int

Assert failure: Assertion failed '!"Extra flags on tree"' in 'Program:Test(int,int):int' during 'Post-Morph'
File: src\coreclr\jit\fgdiagnostic.cpp:3680
```

Release .NET 10.0.12 prints `610839792` (= `RotateLeft(0x12345678, 1)`) — no `OverflowException` at all.

### Notes

The `ADD(NEG(y), N)` / `SUB(N, y)` shift-count matching in `morph.cpp` needs to bail out (or preserve the side
effect) when the matched `ADD`/`SUB`/`NEG` has `gtOverflow()` set. The recognizer allows `GTF_EXCEPT` on matched
operands assuming the exception-raising node survives into the new tree; here it does not, which is the same root
cause surfacing as the Checked-only assert.
Requires `AggressiveOptimization` (or `DOTNET_TieredCompilation=0`) so the rotation recognizer runs.

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.