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

EH table missmatch on ARM32 #6246

Closed
chunseoklee opened this issue Jun 30, 2016 · 8 comments
Closed

EH table missmatch on ARM32 #6246

chunseoklee opened this issue Jun 30, 2016 · 8 comments
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@chunseoklee
Copy link
Contributor

Env : arm32/linux

Some coreclr unit tests do not pass since it does not find Exception handler. This occurs only if the executable is compiled with release mode. See the following code.

//// b49809.cs unit test
namespace Test
{
    using System;

    struct AA
    {
        int[] m_an;

        static bool Test1(int[] param1) { return false; }

        static int[] Test(ref AA[] param1)
        {
            object P = null;
            while (Test1(null))
            {
                do
                {
                    if (Test1((int[])P))
                        Test1(param1[200].m_an);
                } while (Test1((int[])P));
            }
            return param1[0].m_an;
        }

        static int Main()
        {
            try
            {
                AA[] ax = null;
                Test(ref ax);
            }
            catch (NullReferenceException)
            {
                return 100;
            }
            return 1;
        }
    }
}

The jit code for above example looks like :

; Assembly listing for method Test.AA:Main():int
; Emitting BLENDED_CODE for generic ARM CPU
; optimized code
; r11 based frame
; fully interruptible
; Final local variable assignments
;
;  V00 loc0         [V00    ] (  2,   2  )     ref  ->  [sp+0x04]   do-not-enreg[X] must-init addr-exposed ld-addr-op
;* V01 loc1         [V01,T00] (  0,   0  )     int  ->  zero-ref    do-not-enreg[H]
;* V02 tmp0         [V02    ] (  0,   0  )     ref  ->  zero-ref   
;# V03 OutArgs      [V03    ] (  1,   1  )  lclBlk ( 0) [sp+0x00]  
;  V04 PSPSym       [V04    ] (  1,   1  )     int  ->  [sp+0x0C]   do-not-enreg[X] addr-exposed
;
; Lcl frame size = 16

G_M375_IG01:
000000  E92D 4800      push    {r11,lr}
000004  B084           sub     sp, 16
000006  F10D 0B10      add     r11, sp, 16
00000A  2000           movs    r0, 0
00000C  9001           str     r0, [sp+0x04]    // [V00 loc0]
00000E  A806           add     r0, sp, 24
000010  9003           str     r0, [sp+0x0c]    // [V04 PSPSym]

G_M375_IG02:
000012  2300           movs    r3, 0
000014  9301           str     r3, [sp+0x04]    // [V00 loc0]
000016  A801           add     r0, sp, 4    // [V00 loc0]
000018  F244 0345      movw    r3, 0x4045
00001C  F2CB 13E1      movt    r3, 0xb1e1
000020  4798           blx     r3       // Test.AA:Test(byref):ref

G_M375_IG03:
000022  2001           movs    r0, 1

In this example, AV happens on IL 000020(actually inside AA:test function). But when looking for its handler, its controlPc is set to IL 000022, not IL000020(is this bug ?)

When I set COMPlus_JITMinOpts=1, it pass the test. Note that there is dummy(silly) jump code(IL000026) in code below.

; Assembly listing for method Test.AA:Main():int
; Emitting BLENDED_CODE for generic ARM CPU
; compiler->opts.MinOpts() is true
; r11 based frame
; fully interruptible
; Final local variable assignments
;
;  V00 loc0         [V00    ] (  2,   2  )     ref  ->  [sp+0x0C]   do-not-enreg[XH] must-init addr-exposed ld-addr-op
;  V01 loc1         [V01,T00] (  2,   2  )     int  ->  [sp+0x08]   do-not-enreg[H] must-init
;  V02 tmp0         [V02,T01] (  1,   2  )     ref  ->  [sp+0x04]   do-not-enreg[H] must-init
;# V03 OutArgs      [V03,T02] (  1,   1  )  lclBlk ( 0) [sp+0x00]   do-not-enreg[H]
;  V04 PSPSym       [V04    ] (  1,   1  )     int  ->  [sp+0x14]   do-not-enreg[XH] addr-exposed
;
; Lcl frame size = 24

G_M375_IG01:
000000  E92D 4C10      push    {r4,r10,r11,lr}
000004  B086           sub     sp, 24
000006  F10D 0B20      add     r11, sp, 32
00000A  2000           movs    r0, 0
00000C  9003           str     r0, [sp+0x0c]    // [V00 loc0]
00000E  9002           str     r0, [sp+0x08]    // [V01 loc1]
000010  9001           str     r0, [sp+0x04]    // [V02 tmp0]
000012  A80A           add     r0, sp, 40
000014  9005           str     r0, [sp+0x14]    // [V04 PSPSym]

G_M375_IG02:
000016  2300           movs    r3, 0
000018  9303           str     r3, [sp+0x0c]    // [V00 loc0]
00001A  A803           add     r0, sp, 12   // [V00 loc0]
00001C  F244 0345      movw    r3, 0x4045
000020  F2CB 13A8      movt    r3, 0xb1a8
000024  4798           blx     r3       // Test.AA:Test(byref):ref
000026  E7FF           b       SHORT G_M375_IG03

G_M375_IG03:
000028  2001           movs    r0, 1

I think that this kind of patch [https://github.com/dotnet/coreclr/pull/4503/commits/40b23c0b8fc8da2e8f0306966c15a63a33a4bccc] should be applied. But one concern is that such dummy instruction could be removed on jit optimization step.

@chunseoklee
Copy link
Contributor Author

cc @jkotas @janvorli

@jkotas
Copy link
Member

jkotas commented Jun 30, 2016

The JIT inserts extra no-op after the call on amd64. Looks like that this piece of logic is missing for arm. I would look into how it gets inserted on amd64 and replicate the same on arm.

cc @BruceForstall

@myungjoo
Copy link
Contributor

I do not remember exactly; thus I cannot sure if this is the case that i've found with @leemgs. However, there had been similar issues caused by LOCALE information. Yes, that looks extremely weird, but the behavior of coreclr RELEASE (not debug) changed erratic based on LOCALE. (e.g., where $ export shows no LOCALE (LC) related info vs where $ export shows something like "ko.kr_UTF-8")

Would you show me what $ export gives in your ARM system?

@chunseoklee
Copy link
Contributor Author

@myungjoo Here is my target setting.

sh-3.2# export 
export DISPLAY=":0"
export HOME="/root"
export OLDPWD
export PATH="/sbin:/bin:/usr/sbin:/usr/bin"
export PWD="/"
export SHLVL="1"
export TERM="linux"
sh-3.2# cat /etc/sysconfig/i18n 
LANG="en_US.UTF-8"

@BruceForstall
Copy link
Member

@chunseoklee The patch you refer to was not applied: it was proposed, but a better solution was implemented. See the discussion in dotnet/coreclr#4503. I don't believe the JIT needs to implement the additional NOP insertion on ARM32 due to the reasons stated in that PR conversation.

@chunseoklee
Copy link
Contributor Author

chunseoklee commented Jul 1, 2016

@janvorli please, check this approach. [https://github.com/chunseoklee/coreclr/commit/ed9aa2af1fca44074e1574cb05bbe0c44eab03e8] This patch is based on previous discussion( dotnet/coreclr#4503) ), which is not merged. I am not certain this patch is valid. I already have tested on arm target and works fine.

@hseok-oh
Copy link
Contributor

hseok-oh commented Jul 1, 2016

@chunseoklee ControlPcIsUnwound came from !(frameContext->ContextFlags & CONTEXT_EXCEPTION_ACTIVE), and ContextRecord->ContextFlags came from frameContext->ContextFlags. So
pDispatcherContext->ControlPcIsUnwound || (pDispatcherContext->ContextRecord->ContextFlags & CONTEXT_UNWOUND_TO_CALL
always become true.

This problem could be solved by setting correct value to ControlPcISUnwound. This value is always 0 on my test.

@chunseoklee
Copy link
Contributor Author

patch updated. IMHO, the patch looks OK.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2020
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 bug
Projects
None yet
Development

No branches or pull requests

6 participants