Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NativeAOT/arm32] punning test failure in Checked build #98493

Closed
filipnavara opened this issue Feb 15, 2024 · 5 comments · Fixed by #98561
Closed

[NativeAOT/arm32] punning test failure in Checked build #98493

filipnavara opened this issue Feb 15, 2024 · 5 comments · Fixed by #98561
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Comments

@filipnavara
Copy link
Member

The following test fails in Checked builds:

public static void Via_Ldftn_Generics()
{
Console.WriteLine($"Running {nameof(Via_Ldftn_Generics)}...");
IntPtr fptr = B.Class.GetFunctionPointerGeneric();
Assert.NotEqual(IntPtr.Zero, fptr);
var b = new Caller.Struct<object>()
{
Field = 0x55
};
int fieldValue = Caller.Class.CallGetField(b, fptr, null);
Assert.Equal(b.Field, fieldValue);
}

with the following stack trace:

  * frame #0: 0x007513da punning
    frame #1: 0x0065fd46 punning`punning_punning__Via_Ldftn_Generics at punning.cs:67
    frame #2: 0x006600f6 punning`punning___GeneratedMainWrapper__Main at SimpleRunner.g.cs:10
    frame #3: 0x006b6f24 punning`__managed__Main at Utilities.cs:71
    frame #4: 0xf7deb3c0 libc.so.6`__libc_start_call_main(main=(libc.so.6`__libc_start_call_main + 140 at libc_start_call_main.h:74:3), argc=-135024640, argv=0x00556d19) at libc_start_call_main.h:58:16
    frame #5: 0xf7deb4c8 libc.so.6`__libc_start_main_impl(main=(punning`main + 1 at main.cpp:205), argc=1, argv=0xfffef4c4, init=0xf7fef048, fini=0x00000000, rtld_fini=0xf7fcb310, stack_end=0xfffef4c4) at libc-star

The address 0x007513da is a fat pointer (notice that the bit 2 is set on the address). The optimized code strips away the far pointer invocation (at address 0x65fd44):

punning`punning_punning__Via_Ldftn_Generics:
    0x65fd00 <+0>:   push.w {r4, r5, r11, lr}
    0x65fd04 <+3>:   add.w  r11, sp, #0x8
    0x65fd08 <+7>:   movw   r0, #0x5450
    0x65fd0c <+11>:  movt   r0, #0xd
    0x65fd10 <+15>:  add    r0, pc
    0x65fd12 <+17>:  bl     0x5a9d20                  ; System_Console_System_Console__WriteLine_12
    0x65fd16 <+21>:  movw   r4, #0x16b8
    0x65fd1a <+25>:  movt   r4, #0xf
    0x65fd1e <+29>:  add    r4, pc
    0x65fd20 <+31>:  movw   r0, #0x9868
    0x65fd24 <+35>:  movt   r0, #0x10
    0x65fd28 <+39>:  add    r0, pc
    0x65fd2a <+41>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd2e <+45>:  mov    r5, r0
    0x65fd30 <+47>:  mov    r0, r5
    0x65fd32 <+49>:  movs   r1, #0x0
    0x65fd34 <+51>:  bl     0x670990                  ; System.Collections.Generic.GenericEqualityComparer`1<IntPtr>__GetHashCode_0 + 15 at EqualityComparer.cs:140
    0x65fd38 <+55>:  mov    r2, r5
    0x65fd3a <+57>:  mov    r1, r4
    0x65fd3c <+59>:  movs   r0, #0x0
    0x65fd3e <+61>:  bl     0x6cfd50                  ; xunit_assert_Xunit_Assert__NotEqual_11<IntPtr>
    0x65fd42 <+65>:  movs   r0, #0x55
    0x65fd44 <+67>:  blx    r4
    0x65fd46 <+69>:  mov    r4, r0
    0x65fd48 <+71>:  movw   r0, #0x7c14
    0x65fd4c <+75>:  movt   r0, #0x10
    0x65fd50 <+79>:  add    r0, pc
    0x65fd52 <+81>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd56 <+85>:  mov    r5, r0
    0x65fd58 <+87>:  mov    r0, r5
    0x65fd5a <+89>:  movs   r1, #0x0
    0x65fd5c <+91>:  bl     0x669330                  ; System.Buffers.SharedArrayPool`1_<>c<Int32>___InitializeTlsBucketsAndTrimming_b__11_0 + 47 at SharedArrayPool.cs:291
    0x65fd60 <+95>:  mov    r2, r5
    0x65fd62 <+97>:  mov    r1, r4
    0x65fd64 <+99>:  movs   r0, #0x55
    0x65fd66 <+101>: bl     0x6cd930                  ; xunit_assert_Xunit_Assert__Equal_11<Int32>
    0x65fd6a <+105>: pop.w  {r4, r5, r11, pc}
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 15, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 15, 2024
@ghost
Copy link

ghost commented Feb 15, 2024

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

Issue Details

The following test fails in Checked builds:

public static void Via_Ldftn_Generics()
{
Console.WriteLine($"Running {nameof(Via_Ldftn_Generics)}...");
IntPtr fptr = B.Class.GetFunctionPointerGeneric();
Assert.NotEqual(IntPtr.Zero, fptr);
var b = new Caller.Struct<object>()
{
Field = 0x55
};
int fieldValue = Caller.Class.CallGetField(b, fptr, null);
Assert.Equal(b.Field, fieldValue);
}

with the following stack trace:

  * frame #0: 0x007513da punning
    frame #1: 0x0065fd46 punning`punning_punning__Via_Ldftn_Generics at punning.cs:67
    frame #2: 0x006600f6 punning`punning___GeneratedMainWrapper__Main at SimpleRunner.g.cs:10
    frame #3: 0x006b6f24 punning`__managed__Main at Utilities.cs:71
    frame #4: 0xf7deb3c0 libc.so.6`__libc_start_call_main(main=(libc.so.6`__libc_start_call_main + 140 at libc_start_call_main.h:74:3), argc=-135024640, argv=0x00556d19) at libc_start_call_main.h:58:16
    frame #5: 0xf7deb4c8 libc.so.6`__libc_start_main_impl(main=(punning`main + 1 at main.cpp:205), argc=1, argv=0xfffef4c4, init=0xf7fef048, fini=0x00000000, rtld_fini=0xf7fcb310, stack_end=0xfffef4c4) at libc-star

The address 0x007513da is a fat pointer (notice that the bit 2 is set on the address). The optimized code strips away the far pointer invocation (at address 0x65fd44):

punning`punning_punning__Via_Ldftn_Generics:
    0x65fd00 <+0>:   push.w {r4, r5, r11, lr}
    0x65fd04 <+3>:   add.w  r11, sp, #0x8
    0x65fd08 <+7>:   movw   r0, #0x5450
    0x65fd0c <+11>:  movt   r0, #0xd
    0x65fd10 <+15>:  add    r0, pc
    0x65fd12 <+17>:  bl     0x5a9d20                  ; System_Console_System_Console__WriteLine_12
    0x65fd16 <+21>:  movw   r4, #0x16b8
    0x65fd1a <+25>:  movt   r4, #0xf
    0x65fd1e <+29>:  add    r4, pc
    0x65fd20 <+31>:  movw   r0, #0x9868
    0x65fd24 <+35>:  movt   r0, #0x10
    0x65fd28 <+39>:  add    r0, pc
    0x65fd2a <+41>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd2e <+45>:  mov    r5, r0
    0x65fd30 <+47>:  mov    r0, r5
    0x65fd32 <+49>:  movs   r1, #0x0
    0x65fd34 <+51>:  bl     0x670990                  ; System.Collections.Generic.GenericEqualityComparer`1<IntPtr>__GetHashCode_0 + 15 at EqualityComparer.cs:140
    0x65fd38 <+55>:  mov    r2, r5
    0x65fd3a <+57>:  mov    r1, r4
    0x65fd3c <+59>:  movs   r0, #0x0
    0x65fd3e <+61>:  bl     0x6cfd50                  ; xunit_assert_Xunit_Assert__NotEqual_11<IntPtr>
    0x65fd42 <+65>:  movs   r0, #0x55
    0x65fd44 <+67>:  blx    r4
    0x65fd46 <+69>:  mov    r4, r0
    0x65fd48 <+71>:  movw   r0, #0x7c14
    0x65fd4c <+75>:  movt   r0, #0x10
    0x65fd50 <+79>:  add    r0, pc
    0x65fd52 <+81>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd56 <+85>:  mov    r5, r0
    0x65fd58 <+87>:  mov    r0, r5
    0x65fd5a <+89>:  movs   r1, #0x0
    0x65fd5c <+91>:  bl     0x669330                  ; System.Buffers.SharedArrayPool`1_<>c<Int32>___InitializeTlsBucketsAndTrimming_b__11_0 + 47 at SharedArrayPool.cs:291
    0x65fd60 <+95>:  mov    r2, r5
    0x65fd62 <+97>:  mov    r1, r4
    0x65fd64 <+99>:  movs   r0, #0x55
    0x65fd66 <+101>: bl     0x6cd930                  ; xunit_assert_Xunit_Assert__Equal_11<Int32>
    0x65fd6a <+105>: pop.w  {r4, r5, r11, pc}
Author: filipnavara
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@filipnavara filipnavara added arch-arm32 area-NativeAOT-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels Feb 15, 2024
@ghost
Copy link

ghost commented Feb 15, 2024

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

Issue Details

The following test fails in Checked builds:

public static void Via_Ldftn_Generics()
{
Console.WriteLine($"Running {nameof(Via_Ldftn_Generics)}...");
IntPtr fptr = B.Class.GetFunctionPointerGeneric();
Assert.NotEqual(IntPtr.Zero, fptr);
var b = new Caller.Struct<object>()
{
Field = 0x55
};
int fieldValue = Caller.Class.CallGetField(b, fptr, null);
Assert.Equal(b.Field, fieldValue);
}

with the following stack trace:

  * frame #0: 0x007513da punning
    frame #1: 0x0065fd46 punning`punning_punning__Via_Ldftn_Generics at punning.cs:67
    frame #2: 0x006600f6 punning`punning___GeneratedMainWrapper__Main at SimpleRunner.g.cs:10
    frame #3: 0x006b6f24 punning`__managed__Main at Utilities.cs:71
    frame #4: 0xf7deb3c0 libc.so.6`__libc_start_call_main(main=(libc.so.6`__libc_start_call_main + 140 at libc_start_call_main.h:74:3), argc=-135024640, argv=0x00556d19) at libc_start_call_main.h:58:16
    frame #5: 0xf7deb4c8 libc.so.6`__libc_start_main_impl(main=(punning`main + 1 at main.cpp:205), argc=1, argv=0xfffef4c4, init=0xf7fef048, fini=0x00000000, rtld_fini=0xf7fcb310, stack_end=0xfffef4c4) at libc-star

The address 0x007513da is a fat pointer (notice that the bit 2 is set on the address). The optimized code strips away the far pointer invocation (at address 0x65fd44):

punning`punning_punning__Via_Ldftn_Generics:
    0x65fd00 <+0>:   push.w {r4, r5, r11, lr}
    0x65fd04 <+3>:   add.w  r11, sp, #0x8
    0x65fd08 <+7>:   movw   r0, #0x5450
    0x65fd0c <+11>:  movt   r0, #0xd
    0x65fd10 <+15>:  add    r0, pc
    0x65fd12 <+17>:  bl     0x5a9d20                  ; System_Console_System_Console__WriteLine_12
    0x65fd16 <+21>:  movw   r4, #0x16b8
    0x65fd1a <+25>:  movt   r4, #0xf
    0x65fd1e <+29>:  add    r4, pc
    0x65fd20 <+31>:  movw   r0, #0x9868
    0x65fd24 <+35>:  movt   r0, #0x10
    0x65fd28 <+39>:  add    r0, pc
    0x65fd2a <+41>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd2e <+45>:  mov    r5, r0
    0x65fd30 <+47>:  mov    r0, r5
    0x65fd32 <+49>:  movs   r1, #0x0
    0x65fd34 <+51>:  bl     0x670990                  ; System.Collections.Generic.GenericEqualityComparer`1<IntPtr>__GetHashCode_0 + 15 at EqualityComparer.cs:140
    0x65fd38 <+55>:  mov    r2, r5
    0x65fd3a <+57>:  mov    r1, r4
    0x65fd3c <+59>:  movs   r0, #0x0
    0x65fd3e <+61>:  bl     0x6cfd50                  ; xunit_assert_Xunit_Assert__NotEqual_11<IntPtr>
    0x65fd42 <+65>:  movs   r0, #0x55
    0x65fd44 <+67>:  blx    r4
    0x65fd46 <+69>:  mov    r4, r0
    0x65fd48 <+71>:  movw   r0, #0x7c14
    0x65fd4c <+75>:  movt   r0, #0x10
    0x65fd50 <+79>:  add    r0, pc
    0x65fd52 <+81>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd56 <+85>:  mov    r5, r0
    0x65fd58 <+87>:  mov    r0, r5
    0x65fd5a <+89>:  movs   r1, #0x0
    0x65fd5c <+91>:  bl     0x669330                  ; System.Buffers.SharedArrayPool`1_<>c<Int32>___InitializeTlsBucketsAndTrimming_b__11_0 + 47 at SharedArrayPool.cs:291
    0x65fd60 <+95>:  mov    r2, r5
    0x65fd62 <+97>:  mov    r1, r4
    0x65fd64 <+99>:  movs   r0, #0x55
    0x65fd66 <+101>: bl     0x6cd930                  ; xunit_assert_Xunit_Assert__Equal_11<Int32>
    0x65fd6a <+105>: pop.w  {r4, r5, r11, pc}
Author: filipnavara
Assignees: -
Labels:

arch-arm32, untriaged, area-NativeAOT-coreclr

Milestone: -

@MichalStrehovsky
Copy link
Member

Moving this to the JIT are path. For some reason we're ending up optimizing out the fat call check.

Here's the JitDump for ARM32:
fatcall.txt

And here's another one for x64:
fatcall-ok.txt

Notice what we did with STMT00018 in the ARM32 case:

At some point we turn it from

STMT00018 ( 0x007[E-] ... ??? ) <- INLRT @ 0x02C[E-]
               [000065] ------------                        *  JTRUE     void  
               [000064] ------------                        \--*  NE        int   
               [000062] ------------                           +--*  AND       int   
               [000061] ------------                           |  +--*  LCL_VAR   int    V00 loc0         
               [000060] ------------                           |  \--*  CNS_INT   int    2
               [000063] ------------                           \--*  CNS_INT   int    0

Into

removing useless STMT00018 ( INL04 @ 0x007[E-] ... ??? ) <- INLRT @ 0x02C[E-]
N006 (  7,  9) [000065] ------------                        *  JTRUE     void   $VN.Void
N005 (  5,  7) [000064] ------------                        \--*  CNS_INT   int    0
 from BB03

And then it gets optimized away completely for obvious reasons.

I think value numbering is to blame. In the x64 case we have:

N001 [000063]   LCL_VAR   V00 loc0         u:2 => $200 {$180, int <- long}
N002 [000062]   CNS_INT   2 => $46 {IntCns 2}
N003 [000064]   AND       => $201 {AND($46, $200)}
N004 [000065]   CNS_INT   0 => $40 {IntCns 0}
N005 [000066]   NE        => $202 {NE($201, $40)}
N006 [000067]   JTRUE     => $VN.Void

Whereas in the Arm32 case we have:

N001 [000061]   LCL_VAR   V00 loc0         u:2 => $140 {Hnd const: 0x4000000000423110 GTF_ICON_FTN_ADDR}
N002 [000060]   CNS_INT   2 => $45 {IntCns 2}
N003 [000062]   AND       => $141 {Hnd const: 0x0000000000000000 GTF_ICON_CONST_PTR}
N004 [000063]   CNS_INT   0 => $40 {IntCns 0}
N005 [000064]   NE        => $40 {IntCns 0}
N006 [000065]   JTRUE     => $VN.Void

I wonder if this is one of those "treating a handle as an integer constant" things.

@MichalStrehovsky MichalStrehovsky added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed area-NativeAOT-coreclr labels Feb 16, 2024
@ghost
Copy link

ghost commented Feb 16, 2024

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

Issue Details

The following test fails in Checked builds:

public static void Via_Ldftn_Generics()
{
Console.WriteLine($"Running {nameof(Via_Ldftn_Generics)}...");
IntPtr fptr = B.Class.GetFunctionPointerGeneric();
Assert.NotEqual(IntPtr.Zero, fptr);
var b = new Caller.Struct<object>()
{
Field = 0x55
};
int fieldValue = Caller.Class.CallGetField(b, fptr, null);
Assert.Equal(b.Field, fieldValue);
}

with the following stack trace:

  * frame #0: 0x007513da punning
    frame #1: 0x0065fd46 punning`punning_punning__Via_Ldftn_Generics at punning.cs:67
    frame #2: 0x006600f6 punning`punning___GeneratedMainWrapper__Main at SimpleRunner.g.cs:10
    frame #3: 0x006b6f24 punning`__managed__Main at Utilities.cs:71
    frame #4: 0xf7deb3c0 libc.so.6`__libc_start_call_main(main=(libc.so.6`__libc_start_call_main + 140 at libc_start_call_main.h:74:3), argc=-135024640, argv=0x00556d19) at libc_start_call_main.h:58:16
    frame #5: 0xf7deb4c8 libc.so.6`__libc_start_main_impl(main=(punning`main + 1 at main.cpp:205), argc=1, argv=0xfffef4c4, init=0xf7fef048, fini=0x00000000, rtld_fini=0xf7fcb310, stack_end=0xfffef4c4) at libc-star

The address 0x007513da is a fat pointer (notice that the bit 2 is set on the address). The optimized code strips away the far pointer invocation (at address 0x65fd44):

punning`punning_punning__Via_Ldftn_Generics:
    0x65fd00 <+0>:   push.w {r4, r5, r11, lr}
    0x65fd04 <+3>:   add.w  r11, sp, #0x8
    0x65fd08 <+7>:   movw   r0, #0x5450
    0x65fd0c <+11>:  movt   r0, #0xd
    0x65fd10 <+15>:  add    r0, pc
    0x65fd12 <+17>:  bl     0x5a9d20                  ; System_Console_System_Console__WriteLine_12
    0x65fd16 <+21>:  movw   r4, #0x16b8
    0x65fd1a <+25>:  movt   r4, #0xf
    0x65fd1e <+29>:  add    r4, pc
    0x65fd20 <+31>:  movw   r0, #0x9868
    0x65fd24 <+35>:  movt   r0, #0x10
    0x65fd28 <+39>:  add    r0, pc
    0x65fd2a <+41>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd2e <+45>:  mov    r5, r0
    0x65fd30 <+47>:  mov    r0, r5
    0x65fd32 <+49>:  movs   r1, #0x0
    0x65fd34 <+51>:  bl     0x670990                  ; System.Collections.Generic.GenericEqualityComparer`1<IntPtr>__GetHashCode_0 + 15 at EqualityComparer.cs:140
    0x65fd38 <+55>:  mov    r2, r5
    0x65fd3a <+57>:  mov    r1, r4
    0x65fd3c <+59>:  movs   r0, #0x0
    0x65fd3e <+61>:  bl     0x6cfd50                  ; xunit_assert_Xunit_Assert__NotEqual_11<IntPtr>
    0x65fd42 <+65>:  movs   r0, #0x55
    0x65fd44 <+67>:  blx    r4
    0x65fd46 <+69>:  mov    r4, r0
    0x65fd48 <+71>:  movw   r0, #0x7c14
    0x65fd4c <+75>:  movt   r0, #0x10
    0x65fd50 <+79>:  add    r0, pc
    0x65fd52 <+81>:  bl     0x59c1a4                  ; RhpNewFast
    0x65fd56 <+85>:  mov    r5, r0
    0x65fd58 <+87>:  mov    r0, r5
    0x65fd5a <+89>:  movs   r1, #0x0
    0x65fd5c <+91>:  bl     0x669330                  ; System.Buffers.SharedArrayPool`1_<>c<Int32>___InitializeTlsBucketsAndTrimming_b__11_0 + 47 at SharedArrayPool.cs:291
    0x65fd60 <+95>:  mov    r2, r5
    0x65fd62 <+97>:  mov    r1, r4
    0x65fd64 <+99>:  movs   r0, #0x55
    0x65fd66 <+101>: bl     0x6cd930                  ; xunit_assert_Xunit_Assert__Equal_11<Int32>
    0x65fd6a <+105>: pop.w  {r4, r5, r11, pc}
Author: filipnavara
Assignees: -
Labels:

arch-arm32, area-CodeGen-coreclr, untriaged

Milestone: -

@filipnavara
Copy link
Member Author

filipnavara commented Feb 16, 2024

May not be exactly the right fix, but preventing constant folding of the AND with handle seems to do the trick:

--- a/src/coreclr/jit/valuenum.cpp
+++ b/src/coreclr/jit/valuenum.cpp
@@ -4421,6 +4421,11 @@ bool ValueNumStore::VNEvalCanFoldBinaryFunc(var_types type, VNFunc func, ValueNu
             case GT_RSZ:
             case GT_ROL:
             case GT_ROR:
+                if (IsVNHandle(arg0VN) || IsVNHandle(arg1VN))
+                {
+                    return false;
+                }
+                break;
 
             case GT_EQ:
             case GT_NE:

(or rather IsVNHandle(arg0VN) != IsVNHandle(arg1VN) ?)

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 16, 2024
@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Feb 19, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants