search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

Mixed integer size IL operations result in `AccessViolationException`s on 32-bit x86 machines #87298

Open Popax21 opened 1 year ago

Popax21 commented 1 year ago

Description

On 32-bit x86 machines (tested on win-x86), giving a two-operand integer operation IL opcode (namely add, sub, mul, div, div.un, rem, rem.un, and, or, xor, and all their derivatives) mixed int32/int64 operands causes the runtime to throw an AccessViolationException when trying to execute the function. According to ECMA 335 table III.5 "Integer Operations", this is an invalid combination of operands, however the runtime should still be throwing an InvalidProgramException instead of crashing in this manner.

Note: on 64-bit x86 machines (tested on win-x64 and linux-x64), the runtime does not crash, instead implicitly upcasting the 32 bit operand to 64 bits.

Reproduction Steps

The following snippet emits a function with such an operation using Reflection.Emit, and executes it afterwards, which crashes on 32-bit Windows machines:

using System.Reflection.Emit;

var method = new DynamicMethod("TestMethod", typeof(long), new[] { typeof(int), typeof(long) });
var gen = method.GetILGenerator();
gen.Emit(OpCodes.Ldarg_0);
gen.Emit(OpCodes.Ldarg_1);
gen.Emit(OpCodes.And);
gen.Emit(OpCodes.Ret);

var func = method.CreateDelegate<Func<int, long, long>>();
Console.WriteLine(func(0x1234, 0x5678).ToString("x16"));

Expected behavior

Either a InvalidProgramException should be thrown (the behavior according to the standard), or the 32 bit operand should be upcast to 64 bits (the current x64 behavior).

Actual behavior

The runtime crashes with an access violation, resulting in output like this:

Fatal error. 0xC0000005
    at DynamicClass.TestMethod(Int32, Int64)
    at Program.<Main>$(System.String[])

Regression?

No response

Known Workarounds

The 32 bit operand can be explicitly upcast to 64 bits, or vice versa. Note that this should be done regardless of this issue, as otherwise the IL is not standard-compliant.

Configuration

Tests have been performed on NET7.0, on linux-x64, win-x64 and win-x86. Only the latter crashes.

Other information

No response

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-reflection-emit See info in area-owners.md if you want to be subscribed.

Issue Details
### Description On 32-bit x86 machines (tested on `win-x86`), giving a two-operand integer operation IL opcode (namely `add`, `sub`, `mul`, `div`, `div.un`, `rem`, `rem.un`, `and`, `or`, `xor`, and all their derivatives) mixed `int32`/`int64` operands causes the runtime to throw an `AccessViolationException` when trying to execute the function. According to ECMA 335 table III.5 "Integer Operations", this is an invalid combination of operands, however the runtime should still be throwing an `InvalidProgramException` instead of crashing in this manner. **Note:** on 64-bit x86 machines (tested on `win-x64` and `linux-x64`), the runtime does not crash, instead implicitly upcasting the 32 bit operand to 64 bits. ### Reproduction Steps The following snippet emits a function with such an operation using `Reflection.Emit`, and executes it afterwards, which crashes on 32-bit Windows machines: ```cs using System.Reflection.Emit; var method = new DynamicMethod("TestMethod", typeof(long), new[] { typeof(int), typeof(long) }); var gen = method.GetILGenerator(); gen.Emit(OpCodes.Ldarg_0); gen.Emit(OpCodes.Ldarg_1); gen.Emit(OpCodes.And); gen.Emit(OpCodes.Ret); var func = method.CreateDelegate>(); Console.WriteLine(func(0x1234, 0x5678).ToString("x16")); ``` ### Expected behavior Either a `InvalidProgramException` should be thrown (the behavior according to the standard), or the 32 bit operand should be upcast to 64 bits (the current x64 behavior). ### Actual behavior The runtime crashes with an access violation, resulting in output like this: ``` Fatal error. 0xC0000005 at DynamicClass.TestMethod(Int32, Int64) at Program.
$(System.String[]) ``` ### Regression? _No response_ ### Known Workarounds The 32 bit operand can be explicitly upcast to 64 bits, or vice versa. Note that this should be done regardless of this issue, as otherwise the IL is not standard-compliant. ### Configuration Tests have been performed on NET7.0, on `linux-x64`, `windows-x64` and `windows-x86`. Only the latter crashes. ### Other information _No response_
Author: Popax21
Assignees: -
Labels: `area-System.Reflection.Emit`, `untriaged`
Milestone: -
ghost commented 1 year ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

Issue Details
### Description On 32-bit x86 machines (tested on `win-x86`), giving a two-operand integer operation IL opcode (namely `add`, `sub`, `mul`, `div`, `div.un`, `rem`, `rem.un`, `and`, `or`, `xor`, and all their derivatives) mixed `int32`/`int64` operands causes the runtime to throw an `AccessViolationException` when trying to execute the function. According to ECMA 335 table III.5 "Integer Operations", this is an invalid combination of operands, however the runtime should still be throwing an `InvalidProgramException` instead of crashing in this manner. **Note:** on 64-bit x86 machines (tested on `win-x64` and `linux-x64`), the runtime does not crash, instead implicitly upcasting the 32 bit operand to 64 bits. ### Reproduction Steps The following snippet emits a function with such an operation using `Reflection.Emit`, and executes it afterwards, which crashes on 32-bit Windows machines: ```cs using System.Reflection.Emit; var method = new DynamicMethod("TestMethod", typeof(long), new[] { typeof(int), typeof(long) }); var gen = method.GetILGenerator(); gen.Emit(OpCodes.Ldarg_0); gen.Emit(OpCodes.Ldarg_1); gen.Emit(OpCodes.And); gen.Emit(OpCodes.Ret); var func = method.CreateDelegate>(); Console.WriteLine(func(0x1234, 0x5678).ToString("x16")); ``` ### Expected behavior Either a `InvalidProgramException` should be thrown (the behavior according to the standard), or the 32 bit operand should be upcast to 64 bits (the current x64 behavior). ### Actual behavior The runtime crashes with an access violation, resulting in output like this: ``` Fatal error. 0xC0000005 at DynamicClass.TestMethod(Int32, Int64) at Program.
$(System.String[]) ``` ### Regression? _No response_ ### Known Workarounds The 32 bit operand can be explicitly upcast to 64 bits, or vice versa. Note that this should be done regardless of this issue, as otherwise the IL is not standard-compliant. ### Configuration Tests have been performed on NET7.0, on `linux-x64`, `win-x64` and `win-x86`. Only the latter crashes. ### Other information _No response_
Author: Popax21
Assignees: -
Labels: `area-System.Reflection.Emit`, `area-CodeGen-coreclr`, `untriaged`
Milestone: -
jkotas commented 1 year ago

.NET Core is not robust against invalid IL. Trying to run invalid can produce crash or data corruption.

jkotas commented 1 year ago

For reference, the access violation happens at:

 # ChildEBP RetAddr      
00 02b7e464 517e1fa6     clrjit!Lowering::IsBinOpInRMWStoreInd+0x11 [D:\a\_work\1\s\src\coreclr\jit\lowerxarch.cpp @ 4327] 
01 02b7e48c 518313fd     clrjit!Lowering::ContainCheckBinary+0x52 [D:\a\_work\1\s\src\coreclr\jit\lowerxarch.cpp @ 5484] 
02 02b7e4a4 517e32fc     clrjit!Lowering::LowerBinaryArithmetic+0x3f [D:\a\_work\1\s\src\coreclr\jit\lowerxarch.cpp @ 212] 
03 02b7e584 517e2888     clrjit!Lowering::LowerNode+0x9cc [D:\a\_work\1\s\src\coreclr\jit\lower.cpp @ 437] 
04 (Inline) --------     clrjit!Lowering::LowerBlock+0x11 [D:\a\_work\1\s\src\coreclr\jit\lower.cpp @ 6627] 
05 02b7e5ac 517f5999     clrjit!Lowering::DoPhase+0x88 [D:\a\_work\1\s\src\coreclr\jit\lower.cpp @ 6398] 
06 02b7e5c4 517f5444     clrjit!Phase::Run+0xd9 [D:\a\_work\1\s\src\coreclr\jit\phase.cpp @ 62] 
...