-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Description
On x86 such IL is not rejected by our Jit:
ldc.i4 1 // it goes to int64 arg
ldc.i8 2 // it goes to int32 arg
call int32 GitHub_18295::Test(int64 a, int32 b)
but we actually compile it and end up mixing them and pass 1 in a register, 2 on the stack (because we don't track precise types nowadays).
so if inside Test we check the values we will see 1 in b and 2 in a.
This is not the correct behavior.
Full test
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
.assembly extern mscorlib { auto }
.assembly extern System.Console {auto}
.class private auto ansi beforefieldinit GitHub_18295
extends [mscorlib]System.Object
{
.method private hidebysig static int32 Test(int64 l, int32 i) cil managed
{
// Code size 36 (0x24)
.maxstack 2
.locals init (int32 V_0)
IL_0000: nop
IL_0001: ldarg.0
IL_0002: ldc.i8 1
IL_000b: ceq
IL_000d: call void [System.Diagnostics.Debug]System.Diagnostics.Debug::Assert(bool)
IL_0012: nop
IL_0013: ldarg.1
IL_0014: ldc.i4.2
IL_0015: ceq
IL_0017: call void [System.Diagnostics.Debug]System.Diagnostics.Debug::Assert(bool)
IL_001c: nop
IL_001d: ldc.i4.s 100
IL_001f: stloc.0
IL_0020: br.s IL_0022
IL_0022: ldloc.0
IL_0023: ret
}
.method private hidebysig static int32 Main() cil managed
{
.entrypoint
.vtentry 11 : 1
// Code size 131 (0x83)
.maxstack 4
ldc.i4 1
ldc.i8 2
call int32 GitHub_18295::Test(int64, int32)
ldc.i4 100
ret
}
}
We have an example of such IL in
runtime/src/tests/JIT/Regression/JitBlue/GitHub_18295/GitHub_18295.il
Lines 49 to 52 in 1d9b5e0
| // if (Test(1,1) != 1) goto F1 | |
| ldc.i4 1 | |
| ldc.i8 1 | |
| call int32 GitHub_18295::Test(int64, int32) |
that failed during my arm64 OSX work #43130.
In this test, we probably should just swap int8 and int4, but Jit should be changed to reject such IL as BADCODE?
Ecma III.1.6 Implicit argument coercion (page 305).
@dotnet/jit-contrib do you think it should be BADCODE or should we support it?
category:correctness
theme:importer
skill-level:intermediate
cost:medium
Metadata
Metadata
Assignees
Labels
Type
Projects
Status