dotnet / dotnet/runtime

JIT: (bug) UTF16 string comparison expansion reorders receiver null check ahead of earlier side effects

Open
#133,966 1 comment 0 reactions 0 assignees View on GitHub
area-CodeGen-coreclr
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

The importer expansion for short UTF16 string comparisons can append the unrolled comparison before earlier side-effecting expressions still on the evaluation stack, causing the receiver null check to throw first.

### Minimal Repro

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

public class Program
{
[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
public static int Test(string s, int[] a, int i)
{
return a[i] + (s.Equals("ab") ? 1 : 0);
}

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
public static int Test2(string s, int x, int y)
{
return (x / y) + (s.StartsWith("ab", StringComparison.Ordinal) ? 1 : 0);
}

private static void Run(string name, Func f)
{
try { Console.WriteLine(name + ": " + f()); }
catch (Exception e) { Console.WriteLine(name + ": " + e.GetType().Name); }
}

public static void Main()
{
int[] a = new int[1];
Run("Test", () => Test(null, a, 5));
Run("Test2", () => Test2(null, 1, 0));
}
}
```

### Expected

```
Test: IndexOutOfRangeException
Test2: DivideByZeroException
```

### Actual

```
Test: NullReferenceException
Test2: NullReferenceException
```

### Notes

`impUtf16StringComparison` stores the string temp and unrolled comparison with `CHECK_SPILL_NONE`, so prior stack entries are not spilled before the receiver dereference is appended.
The same pattern exists in `impUtf16SpanComparison`; a long literal that skips the unrolled expansion preserves the expected exception order.

Contributor guide

Open the contributing guide

Research direction

Start by tracing impUtf16StringComparison and the corresponding impUtf16SpanComparison path, focusing on how the string temp and unrolled comparison interact with CHECK_SPILL_NONE and earlier evaluation-stack entries. Run the minimal repro to confirm the current NullReferenceException results, then verify that both short UTF16 comparison paths preserve the expected IndexOutOfRangeException or DivideByZeroException ordering.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.