runtime: RyuJIT: Incorrect 4-byte immediate emitted for shift causes access violation
In debug, RyuJIT seems to try to emit a 4-byte immediate on a shl instruction for the following case:
public class Program
{
static ushort s_2;
static short[] s_5 = new short[]{1};
static ulong s_8;
public static void Main()
{
var vr2 = s_5[0];
M9();
}
static void M9()
{
s_8 <<= (0 & s_2) + 186;
}
}
According to the JIT, this is the disassembly:
G_M23195_IG03:
90 nop
48C1255C29EEFFBA000000 shl qword ptr [reloc classVar[0x391252c0]], 186
90 nop
Of course shifts do not have a 4-byte immediate form, and VS knows better:
00007FFE51C915DE 90 nop
00007FFE51C915DF 48 C1 25 06 36 EE FF BA shl qword ptr [7FFE51B74BEDh],0BAh
00007FFE51C915E7 00 00 add byte ptr [rax],al
00007FFE51C915E9 00 90 48 8D 65 30 add byte ptr [rax+30658D48h],dl
This access violates depending on the value of rax.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Comments: 17 (17 by maintainers)
Commits related to this issue
- Mask shift value for shift [mem], const The shift value needs to be masked as otherwise the emitter may end up believing it needs 4 bytes for constants >= 128. Since an encoding with 4-byte immediate... — committed to jakobbotsch/coreclr by jakobbotsch 6 years ago
- Mask shift value for shift [mem], const The shift value needs to be masked as otherwise the emitter may end up believing it needs 4 bytes for constants >= 128. Since an encoding with 4-byte immediate... — committed to jakobbotsch/coreclr by jakobbotsch 6 years ago
This bug is dedicated to everyone who has ever said that C# doesn’t support inline assembly…
The problem is that lowering marks the immediate as contained because it is lower than 256: https://github.com/dotnet/coreclr/blob/fea6dcacb820057eb24a25b97c337b5dcf881301/src/jit/lowerxarch.cpp#L1694-L1712 However, since x86 instructions sign-extend immediates, the emitter believes it needs 4 bytes to represent values >= 128: https://github.com/dotnet/coreclr/blob/fea6dcacb820057eb24a25b97c337b5dcf881301/src/jit/emitxarch.cpp#L2143
Two potential solutions. Either lowering should not mark constants >= 128 as contained, or we can sign extend the value manually so the emitter only uses 1 byte. Probably the former is best since such shifts should be super rare anyway and it avoids introducing new code/changing trees in code-gen. Thoughts?