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

[RFC] Introduce JitCompBreakpoint #606

Closed
xiangzhai opened this issue Dec 6, 2019 · 16 comments
Closed

[RFC] Introduce JitCompBreakpoint #606

xiangzhai opened this issue Dec 6, 2019 · 16 comments
Labels
arch-mips64 Related to MIPS64 architecture (unsupported) area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@xiangzhai
Copy link
Contributor

Hi,

When I was an apprentice in the OpenJDK world, I have no idea about How to use gdb to debug C1 compiler's internal error. I always lost in the Assembly forest :(

Dean, an experienced OpenJDK HotSpot developer, introduced C1Breakpoint and then
single-stepping si through the generated code.

So I just implement JitCompBreakpoint for CoreCLR about Debugging JIT-ed Code With GDB or lldb:

diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp
index 7eb808f..fda3e89 100644
--- a/src/coreclr/src/jit/codegencommon.cpp
+++ b/src/coreclr/src/jit/codegencommon.cpp
@@ -4865,6 +4865,21 @@ void          CodeGen::genPushCalleeSavedRegisters()
     }
 #endif // DEBUG
 
+#ifdef DEBUG
+    const char* JitCompBreakpoint = getenv("JitCompBreakpoint");
+    if (JitCompBreakpoint)
+    {
+        if (strstr(JitCompBreakpoint, "*") || strstr(compiler->info.compFullName, JitCompBreakpoint))
+        {
+#if defined(_TARGET_ARMARCH_)
+            instGen(INS_brk);
+#elif defined(_TARGET_XARCH_)
+            instGen(INS_int3);
+#endif
+        }
+    }
+#endif
+
 #if defined(_TARGET_ARM_)
     regMaskTP maskPushRegsFloat = rsPushRegs & RBM_ALLFLOAT;
     regMaskTP maskPushRegsInt   = rsPushRegs & ~maskPushRegsFloat;
diff --git a/src/coreclr/src/jit/emitarm64.cpp b/src/coreclr/src/jit/emitarm64.cpp
index 427a1c0..f7add7a 100644
--- a/src/coreclr/src/jit/emitarm64.cpp
+++ b/src/coreclr/src/jit/emitarm64.cpp
@@ -3368,7 +3368,10 @@ void emitter::emitIns(instruction ins)
     instrDesc* id  = emitNewInstrSmall(EA_8BYTE);
     insFormat  fmt = emitInsFormat(ins);
 
-    assert(fmt == IF_SN_0A);
+    if (ins != INS_brk)
+    {
+        assert(fmt == IF_SN_0A);
+    }
 
     id->idIns(ins);
     id->idInsFmt(fmt);

Then it is able to single-stepping si through the generated code by setting JitCompBreakpoint enviroment value:

export JitCompBreakpoint="Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this"

For X86:

...
{ Start Jitting Method  880 Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this (MethodHash=a94d4759) MinOpts
} Jitted Method  880 at 00007fff`7c6a8dd0 method Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this size 0000002f
Process 6804 stopped
* thread #7, name = '.NET Finalizer', stop reason = signal SIGTRAP
    frame #0: 0x00007fff7c6a8dd2
->  0x7fff7c6a8dd2: subq   $0x10, %rsp
    0x7fff7c6a8dd6: leaq   0x10(%rsp), %rbp
    0x7fff7c6a8ddb: movq   %rdi, -0x8(%rbp)
    0x7fff7c6a8ddf: movq   %rsi, -0x10(%rbp)
(lldb) si
Process 6804 stopped
* thread #7, name = '.NET Finalizer', stop reason = instruction step into
    frame #0: 0x00007fff7c6a8dd6
->  0x7fff7c6a8dd6: leaq   0x10(%rsp), %rbp
    0x7fff7c6a8ddb: movq   %rdi, -0x8(%rbp)
    0x7fff7c6a8ddf: movq   %rsi, -0x10(%rbp)
    0x7fff7c6a8de3: movabsq $0x7fff7bfa45e0, %rax     ; imm = 0x7FFF7BFA45E0 
(lldb) si
Process 6804 stopped
* thread #7, name = '.NET Finalizer', stop reason = instruction step into
    frame #0: 0x00007fff7c6a8ddb
->  0x7fff7c6a8ddb: movq   %rdi, -0x8(%rbp)
    0x7fff7c6a8ddf: movq   %rsi, -0x10(%rbp)
    0x7fff7c6a8de3: movabsq $0x7fff7bfa45e0, %rax     ; imm = 0x7FFF7BFA45E0 
    0x7fff7c6a8ded: cmpl   $0x0, (%rax)
(lldb) si
Process 6804 stopped
* thread #7, name = '.NET Finalizer', stop reason = instruction step into
    frame #0: 0x00007fff7c6a8ddf
->  0x7fff7c6a8ddf: movq   %rsi, -0x10(%rbp)
    0x7fff7c6a8de3: movabsq $0x7fff7bfa45e0, %rax     ; imm = 0x7FFF7BFA45E0 
    0x7fff7c6a8ded: cmpl   $0x0, (%rax)
    0x7fff7c6a8df0: je     0x7fff7c6a8df7
(lldb) si
Process 6804 stopped
* thread #7, name = '.NET Finalizer', stop reason = instruction step into
    frame #0: 0x00007fff7c6a8de3
->  0x7fff7c6a8de3: movabsq $0x7fff7bfa45e0, %rax     ; imm = 0x7FFF7BFA45E0 
    0x7fff7c6a8ded: cmpl   $0x0, (%rax)
    0x7fff7c6a8df0: je     0x7fff7c6a8df7
    0x7fff7c6a8df2: callq  0x7ffff5f276c0            ; JIT_DbgIsJustMyCode at jithelpers.cpp:5149
(lldb) 

For ARM64, it is easy to get registers information:

...
{ Start Jitting Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this (MethodHash=f16af6a0)
} Jitted Entry 22a at 0000007f`3d731c00 method Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this size 0000003c

Thread 7 "corerun" received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 0x7fb3eb51e0 (LWP 32000)]
0x0000007f3d731c00 in ?? ()
(gdb) x/22i $pc
=> 0x7f3d731c00:	brk	#0x0
   0x7f3d731c04:	stp	x29, x30, [sp,#-32]!
   0x7f3d731c08:	mov	x29, sp
   0x7f3d731c0c:	str	x0, [x29,#24]
   0x7f3d731c10:	str	x1, [x29,#16]
   0x7f3d731c14:	mov	x0, #0x4658                	// #18008
   0x7f3d731c18:	movk	x0, #0x3d19, lsl #16
   0x7f3d731c1c:	movk	x0, #0x7f, lsl #32
   0x7f3d731c20:	ldr	w0, [x0]
   0x7f3d731c24:	cbz	w0, 0x7f3d731c2c
   0x7f3d731c28:	bl	0x7f3d307bd0
   0x7f3d731c2c:	nop
   0x7f3d731c30:	nop
   0x7f3d731c34:	ldp	x29, x30, [sp],#32
   0x7f3d731c38:	ret
   0x7f3d731c3c:	ldxrb	w15, [x0]
   0x7f3d731c40:	.inst	0x0040000d ; undefined
   0x7f3d731c44:	.inst	0xe4e483e1 ; undefined
   0x7f3d731c48:	.inst	0x00000040 ; undefined
   0x7f3d731c4c:	.inst	0x00000000 ; undefined
   0x7f3d731c50:	.inst	0x00000000 ; undefined
   0x7f3d731c54:	.inst	0x00000000 ; undefined
(gdb) set $pc+=4
(gdb) si
0x0000007f3d731c08 in ?? ()
(gdb) i r x29 x30
x29            0x7fb3eb4000	548479385600
x30            0x7f3d7305ac	546491794860

It is also helpful for MIPS64 RyuJIT porting :)

Request for Comments.

Thanks,
Leslie Zhai

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Dec 6, 2019
@xiangzhai
Copy link
Contributor Author

\cc @BruceForstall

@xiangzhai xiangzhai changed the title Introduce JitCompBreakpoint [RFC] Introduce JitCompBreakpoint Dec 6, 2019
@BruceForstall
Copy link
Member

This looks like the same thing as the pre-existing COMPlus_JitHalt (there is also COMPlus_JitHashHalt).

@BruceForstall
Copy link
Member

There are lots of similar (and not so similar) options in jitconfigvalues.h.

@BruceForstall BruceForstall added arch-mips64 Related to MIPS64 architecture (unsupported) and removed untriaged New issue has not been triaged by the area owner labels Dec 6, 2019
@BruceForstall BruceForstall added this to the Future milestone Dec 6, 2019
@xiangzhai
Copy link
Contributor Author

This looks like the same thing as the pre-existing COMPlus_JitHalt (there is also COMPlus_JitHashHalt).

Thanks for your teaching!

It also works for MIPS64:

...
{ Start Jitting Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this (MethodHash=f16af6a0)
DEBUG: MIPS64, frameType:1

} Jitted Entry 221 at 000000ff`7cc56d40 method Enumerator[Int64,__Canon][System.Int64,System.__Canon]:Dispose():this size 00000094
warning: GDB can't find the start of the function at 0xff7cc56d44.

Program received signal SIGTRAP, Trace/breakpoint trap.
[Switching to Thread 0xfff364b1d0 (LWP 20477)]
0x000000ff7cc56d44 in ?? ()

(gdb) x/22i $pc
=> 0xff7cc56d44:	break
   0xff7cc56d48:	daddiu	sp,sp,-32
   0xff7cc56d4c:	sd	s8,0(sp)
   0xff7cc56d50:	sd	ra,8(sp)
   0xff7cc56d54:	ori	s8,sp,0x0
   0xff7cc56d58:	sd	a0,24(s8)
   0xff7cc56d5c:	sd	a1,16(s8)
   0xff7cc56d60:	lui	a0,0xff
   0xff7cc56d64:	ori	a0,a0,0x7c6f
   0xff7cc56d68:	dsll	a0,a0,0x10
   0xff7cc56d6c:	ori	a0,a0,0x658
   0xff7cc56d70:	lw	a0,0(a0)
   0xff7cc56d74:	sltiu	at,a0,1
   0xff7cc56d78:	beqz	at,0xff7cc56da0
   0xff7cc56d7c:	nop
   0xff7cc56d80:	b	0xff7cc56db8
   0xff7cc56d84:	nop
   0xff7cc56d88:	nop
   0xff7cc56d8c:	nop
   0xff7cc56d90:	nop
   0xff7cc56d94:	nop
   0xff7cc56d98:	nop
(gdb) set $pc+=4
warning: GDB can't find the start of the function at 0xff7cc56d48.
(gdb) si
warning: GDB can't find the start of the function at 0xff7cc56d4c.
0x000000ff7cc56d4c in ?? ()
(gdb) i r a0 a1 a2
a0: 0xfff364a010
a1: 0xff7ccf2488
a2: 0x0

I made the wheel repeatedly :P

Thanks,
Leslie Zhai

@xiangzhai
Copy link
Contributor Author

xiangzhai commented Dec 13, 2019

Hi @BruceForstall

export COMPlus_JitHalt="*" failed to work for ARM64, gdb just hang without response:

(gdb) r
Starting program: /home/zhaixiang/coreclr-mips64-dev/bin/Product/Linux.arm64.Debug/corerun ./bin/tests/Linux.arm64.Debug/JIT/Methodical/divrem/div/u4div_cs_do/u4div_cs_do.exe
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/aarch64-linux-gnu/libthread_db.so.1".
[New Thread 0x7fb6c051e0 (LWP 20701)]
[New Thread 0x7fb64051e0 (LWP 20702)]
[New Thread 0x7fb5c051e0 (LWP 20703)]
[New Thread 0x7fb53e51e0 (LWP 20704)]
[New Thread 0x7fb4be51e0 (LWP 20705)]
[New Thread 0x7fb3f101e0 (LWP 20706)]
[New Thread 0x7fb336f1e0 (LWP 20707)]
{ Start Jitting System.AppContext:Setup(long,long,int) (MethodHash=d8570543)
DEBUG: ARM64, frameType:1

} Jitted Entry 001 at 0000007f`3d377a80 method System.AppContext:Setup(long,long,int) size 00000120



* ARM64 might not generate `brk` instruction * 





But my JitCompBreakPoint wheel is able to work for ARM64, MIPS64 and X86.

Thanks,
Leslie Zhai

@xiangzhai xiangzhai reopened this Dec 13, 2019
@BruceForstall
Copy link
Member

Doesn't your JitCompBreakPoint and JitHalt generate the same instruction (some kind of break instruction) into the code stream?

@xiangzhai
Copy link
Contributor Author

xiangzhai commented Dec 14, 2019

bool Compiler::compJitHaltMethod()                                              
{                                                                                    
    /* This method returns true when we use an * INS_BREAKPOINT * to allow us to step into the generated native code */
    /* Note that this these two "Jit" environment variables also work for ngen images */
                                                                                     
    if (JitConfig.JitHalt().contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
    {                                                                           
        return true;                                                            
    }                                                                           
    ...                                                      
}

JitHalt generate INS_bkpt for ARM64.

But JitCompBreakPoint generate INS_brk for ARM64, INS_int3 for X86 and INS_break for MIPS64.

Thanks,
Leslie Zhai

@BruceForstall
Copy link
Member

From the ARMv8 manual, it looks like perhaps we should never be generating INS_bkpt for ARM64, and should only generate INS_brk. It looks like INS_bkpt is an arm32-only instruction. So maybe you can just change the definition of INS_BREAKPOINT for ARM64 to be INS_brk.

@briansull @TamarChristinaArm comments?

@xiangzhai
Copy link
Contributor Author

So maybe you can just change the definition of INS_BREAKPOINT for ARM64 to be INS_brk.

diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h
index 869267d..fc0d76d 100644
--- a/src/coreclr/src/jit/compiler.h
+++ b/src/coreclr/src/jit/compiler.h
@@ -10607,7 +10607,7 @@ const instruction INS_SQRT = INS_vsqrt;
 #ifdef _TARGET_ARM64_
 
 const instruction INS_MULADD     = INS_madd;
-const instruction INS_BREAKPOINT = INS_bkpt;
+const instruction INS_BREAKPOINT = INS_brk;
 
 const instruction INS_ABS  = INS_fabs;
 const instruction INS_SQRT = INS_fsqrt;

Thanks,
Leslie Zhai

@TamarChristinaArm
Copy link
Contributor

From the ARMv8 manual, it looks like perhaps we should never be generating INS_bkpt for ARM64, and should only generate INS_brk. It looks like INS_bkpt is an arm32-only instruction. So maybe you can just change the definition of INS_BREAKPOINT for ARM64 to be INS_brk.

Yeah that's correct, the two execution states have different breakpoint instructions.

@xiangzhai
Copy link
Contributor Author

Hi @TamarChristinaArm

Thanks for your kind response!

@BruceForstall

May I create PR to fix the issue?

Cheers,
Leslie Zhai

@BruceForstall
Copy link
Member

@xiangzhai

May I create PR to fix the issue?

Yes, that would be great.

@xiangzhai
Copy link
Contributor Author

Hi @BruceForstall

Please review it #892

Cheers,
Leslie Zhai

@BruceForstall
Copy link
Member

Unfortunately, it looks like the situation is a little more complicated that it should be.

I reproduced the failure and your fix on Linux, and it looks good.

I tried the existing code and your fix on Windows, and it doesn't work. With the existing code (using bkpt), setting COMPlus_JitHalt to some function and running a test, the Windows windbg debugger works as desired, and stops at that instruction. With your change, windbg shows an illegal instruction exception, and kills the process.

So, I think we need to leave the existing code, but #ifdef your fix (for defining INS_BREAKPOINT especially) under _TARGET_UNIX_.

@BruceForstall
Copy link
Member

Oops. I should have put this comment in the PR #892; I'll do that.

@xiangzhai
Copy link
Contributor Author

Oops. I should have put this comment in the PR #892; I'll do that.

You are welcome :)

BruceForstall pushed a commit that referenced this issue Jan 6, 2020
* [ARM64] Change INS_bkpt to INS_brk for INS_BREAKPOINT

#606

* [ARM64] Change INS_bkpt to INS_brk for INS_BREAKPOINT

#606

* [ARM64] Change INS_bkpt to INS_brk for INS_BREAKPOINT

#606

* [ARM64] Change INS_bkpt to INS_brk for TARGET_UNIX

#606

* [ARM64] Change INS_bkpt to INS_brk for TARGET_UNIX

#606

* Fix formatting errors for INS_MULADD
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-mips64 Related to MIPS64 architecture (unsupported) area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

4 participants