When the .NET WebAssembly interpreter rotates by the wrong number of bits

Note: I reported this issue in dotnet/runtime#137279.

A conditional argument to BitOperations.RotateLeft can produce an erroneous result in the .NET WebAssembly interpreter:

ulong Rotate(ulong value, bool flag) => BitOperations.RotateLeft(value, flag ? 49 : 24);

With flag set to false, it should rotate by 24 bits. However, calling Rotate(0x123456789ABCDEF0UL, false) gave these results:

I also reproduced the issue with RotateRight(ulong) and RotateLeft(uint): when a conditional expression supplied two constant counts directly to the call, the interpreter used the true branch's constant for both branches. Woops!

I built 20 test cases. The results were surprising:

• 14/20 passed in the browser
• 20/20 passed on desktop CoreCLR

Disabling the jiterpreter, Mono's partial JIT for WebAssembly, did not help. Neither did --optimize=-all. The next step was to find where the choice between 24 and 49 disappeared.

01The Compiler Preserved the Choice#

The Release IL contained exactly the branches I expected:

IL_0000: ldarg.0            // Push value
IL_0001: ldarg.1            // Push flag
IL_0002: brtrue.s IL_0008   // Pop flag; branch if true
IL_0004: ldc.i4.s 24        // False path: push count 24
IL_0006: br.s IL_000a       // Skip the load of 49; stack is [value, 24]
IL_0008: ldc.i4.s 49        // True path: push count 49; stack is [value, 49]
IL_000a: call uint64 System.Numerics.BitOperations::RotateLeft(uint64, int32)
IL_000f: ret                // Return the rotation result

The false path loads 24 and jumps to the call. The true path loads 49 and reaches the same call. Looks correct.

To see what Mono did with this IL, I enabled MONO_VERBOSE_METHOD=Repro:Conditional and ran the WebAssembly. The method trace showed the mistake in Mono's intermediate representation:

BB4:
IL_000a: rol.i8.imm [6 <- 2], 49

rol.i8.imm rotates a 64-bit value by a count embedded in the instruction. Here, that count is 49, regardless of which branch ran. The choice was present in the IL, but Mono had replaced it with a constant while translating.

02An Optimization Gone Wrong#

Mono's rotation intrinsic handling in transform.c looks at td->last_ins, the last instruction it emitted. If that instruction loads an integer constant, Mono can use an immediate rotation instead of a normal method call. It does not check whether that instruction belonged to the current basic block.

When Mono processed the join, its global last-instruction pointer still referred to the load of 49 in the preceding block. The intrinsic handling treated that constant as the count for the call, even though another path could arrive with 24.

The defect dates back to 6ddb03f8e8, Add intrinsics for BitOperations, merged on 14 March 2023 through PR #82827. That commit introduced the immediate rotation instructions and the unchecked use of td->last_ins.

03The Missing Check#

The fix is simple. Add one condition before Mono accepts a constant rotation count:

if (td->last_ins == td->cbb->last_ins) {
    // Code here
}

It checks if the last emitted instruction is also the last instruction in the current basic block. Nearby conversion optimizations use the same guard.

The guard covers both rotation directions (Left/Right) and both integer widths (uint/ulong). I patched mono with the fix, and my 20 tests all passed.

A simple workaround until the bug is fixed, is to simply pull out the branching before the call:

static ulong Rotate(ulong value, bool flag)
{
    int count = flag ? 49 : 24;
    return BitOperations.RotateLeft(value, count);
}