This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH/v2] Fix PR backtrace/16558: GDB Aarch64 signal frame unwinder issue
- From: Hui Zhu <hui_zhu at mentor dot com>
- To: Marcus Shawcroft <marcus dot shawcroft at gmail dot com>
- Cc: Yao Qi <Yao_Qi at mentor dot com>, gdb-patches ml <gdb-patches at sourceware dot org>
- Date: Wed, 9 Apr 2014 16:42:10 +0800
- Subject: Re: [PATCH/v2] Fix PR backtrace/16558: GDB Aarch64 signal frame unwinder issue
- Authentication-results: sourceware.org; auth=none
- References: <533295A3 dot 90907 at mentor dot com>
Ping.
Thanks,
Hui
-------- Original Message --------
Subject: Re: [PATCH/v2] Fix PR backtrace/16558: GDB Aarch64 signal frame unwinder issue
Date: Wed, 26 Mar 2014 16:53:55 +0800
From: Hui Zhu <hui_zhu@mentor.com>
To: Marcus Shawcroft <marcus.shawcroft@gmail.com>
CC: gdb-patches ml <gdb-patches@sourceware.org>, Yao Qi <Yao_Qi@mentor.com>
On 03/20/14 00:01, Marcus Shawcroft wrote:
On 5 March 2014 06:33, Hui Zhu <hui_zhu@mentor.com> wrote:
/* Signal frame handling.
- +----------+ ^
- | saved lr | |
- +->| saved fp |--+
- | | |
- | | |
- | +----------+
- | | saved lr |
- +--| saved fp |
- ^ | |
- | | |
- | +----------+
- ^ | |
- | | signal |
- | | |
- | | saved lr |-->interrupted_function_pc
- +--| saved fp |
- | +----------+
- | | saved lr |--> default_restorer (movz x8, NR_sys_rt_sigreturn; svc
0)
- +--| saved fp |<- FP
- | |
- | |<- SP
- +----------+
-
Better no diagram than a broken diagram, but wouldn't it be better to
fix the diagram rather than just remove the whole comment?
Add the pic back according to Yao's pic:
+------------+ ^
| saved lr | |
+->| saved fp |--+
| | |
| | |
| +------------+
| | saved lr |
+--| saved fp |
^ | |
| | |
| +------------+
^ | |
| | signal |
| | | SIGTRAMP_FRAME (struct rt_sigframe)
| | saved regs |
+--| saved sp |--> interrupted_sp
| | saved pc |--> interrupted_pc
| | |
| +------------+
| | saved lr |--> default_restorer (movz x8, NR_sys_rt_sigreturn; svc 0)
+--| saved fp |<- FP
| | NORMAL_FRAME
| |<- SP
+------------+
I removed "saved lr |-->interrupted_function_pc". Because the lr didn't save the
address of of interrupted_function. It saved the caller address of interrupted_function.
This is not a special behavior of ABI. So I think It does not need to be added here.
On signal delivery, the kernel will create a signal handler stack
- frame and setup the return address in LR to point at restorer stub.
+ frame in arch/arm64/kernel/signal.c:setup_rt_frame.
I don;t think documenting the name of a function in a different source
tree here in a comment is a good idea, should the kernel guys decide
to refactor that code in the future the comment will be left bit
rotten. It would be better to say what we are expecting to find on
the stack and in the registers rather than who we are expecting to
setup the stack and registers.
The signal stack frame is defined by:
struct rt_sigframe
@@ -123,8 +100,8 @@
d28015a8 movz x8, #0xad
d4000001 svc #0x0
- We detect signal frames by snooping the return code for the restorer
- instruction sequence.
+ This is a system call sys_rt_sigreturn. The kernel will detect signal
+ frame from sp and call arch/arm64/kernel/signal.c:restore_sigframe.
Likewise.
I added them back.
The handler then needs to recover the saved register set from
ucontext.uc_mcontext. */
@@ -146,7 +123,6 @@ aarch64_linux_sigframe_init (const struc
{
struct gdbarch *gdbarch = get_frame_arch (this_frame);
CORE_ADDR sp = get_frame_register_unsigned (this_frame,
AARCH64_SP_REGNUM);
- CORE_ADDR fp = get_frame_register_unsigned (this_frame,
AARCH64_FP_REGNUM);
CORE_ADDR sigcontext_addr =
sp
+ AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
@@ -160,12 +136,14 @@ aarch64_linux_sigframe_init (const struc
cd sigcontext_addr +
AARCH64_SIGCONTEXT_XO_OFFSET
+ i * AARCH64_SIGCONTEXT_REG_SIZE);
}
+ trad_frame_set_reg_addr (this_cache, AARCH64_SP_REGNUM,
+ sigcontext_addr + AARCH64_SIGCONTEXT_XO_OFFSET
+ + 31 * AARCH64_SIGCONTEXT_REG_SIZE);
+ trad_frame_set_reg_addr (this_cache, AARCH64_PC_REGNUM,
+ sigcontext_addr + AARCH64_SIGCONTEXT_XO_OFFSET
+ + 32 * AARCH64_SIGCONTEXT_REG_SIZE);
- trad_frame_set_reg_addr (this_cache, AARCH64_FP_REGNUM, fp);
- trad_frame_set_reg_addr (this_cache, AARCH64_LR_REGNUM, fp + 8);
- trad_frame_set_reg_addr (this_cache, AARCH64_PC_REGNUM, fp + 8);
-
- trad_frame_set_id (this_cache, frame_id_build (fp, func));
+ trad_frame_set_id (this_cache, frame_id_build (sp, func));
The comments above aside, I think the actual functional change in this
patch looks reasonable. However I cannot approve patches in GDB.
Thanks for your help.
Best,
Hui
2014-03-26 Hui Zhu <hui@codesourcery.com>
Yao Qi <yao@codesourcery.com>
PR backtrace/16558
* aarch64-linux-tdep.c (aarch64_linux_sigframe_init): Update comments
and change address of sp and pc.
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -53,28 +53,30 @@
/* Signal frame handling.
- +----------+ ^
- | saved lr | |
- +->| saved fp |--+
- | | |
- | | |
- | +----------+
- | | saved lr |
- +--| saved fp |
- ^ | |
- | | |
- | +----------+
- ^ | |
- | | signal |
- | | |
- | | saved lr |-->interrupted_function_pc
- +--| saved fp |
- | +----------+
- | | saved lr |--> default_restorer (movz x8, NR_sys_rt_sigreturn; svc 0)
- +--| saved fp |<- FP
- | |
- | |<- SP
- +----------+
+ +------------+ ^
+ | saved lr | |
+ +->| saved fp |--+
+ | | |
+ | | |
+ | +------------+
+ | | saved lr |
+ +--| saved fp |
+ ^ | |
+ | | |
+ | +------------+
+ ^ | |
+ | | signal |
+ | | | SIGTRAMP_FRAME (struct rt_sigframe)
+ | | saved regs |
+ +--| saved sp |--> interrupted_sp
+ | | saved pc |--> interrupted_pc
+ | | |
+ | +------------+
+ | | saved lr |--> default_restorer (movz x8, NR_sys_rt_sigreturn; svc 0)
+ +--| saved fp |<- FP
+ | | NORMAL_FRAME
+ | |<- SP
+ +------------+
On signal delivery, the kernel will create a signal handler stack
frame and setup the return address in LR to point at restorer stub.
@@ -123,6 +125,8 @@
d28015a8 movz x8, #0xad
d4000001 svc #0x0
+ This is a system call sys_rt_sigreturn.
+
We detect signal frames by snooping the return code for the restorer
instruction sequence.
@@ -146,7 +150,6 @@ aarch64_linux_sigframe_init (const struc
{
struct gdbarch *gdbarch = get_frame_arch (this_frame);
CORE_ADDR sp = get_frame_register_unsigned (this_frame, AARCH64_SP_REGNUM);
- CORE_ADDR fp = get_frame_register_unsigned (this_frame, AARCH64_FP_REGNUM);
CORE_ADDR sigcontext_addr =
sp
+ AARCH64_RT_SIGFRAME_UCONTEXT_OFFSET
@@ -160,12 +163,14 @@ aarch64_linux_sigframe_init (const struc
sigcontext_addr + AARCH64_SIGCONTEXT_XO_OFFSET
+ i * AARCH64_SIGCONTEXT_REG_SIZE);
}
+ trad_frame_set_reg_addr (this_cache, AARCH64_SP_REGNUM,
+ sigcontext_addr + AARCH64_SIGCONTEXT_XO_OFFSET
+ + 31 * AARCH64_SIGCONTEXT_REG_SIZE);
+ trad_frame_set_reg_addr (this_cache, AARCH64_PC_REGNUM,
+ sigcontext_addr + AARCH64_SIGCONTEXT_XO_OFFSET
+ + 32 * AARCH64_SIGCONTEXT_REG_SIZE);
- trad_frame_set_reg_addr (this_cache, AARCH64_FP_REGNUM, fp);
- trad_frame_set_reg_addr (this_cache, AARCH64_LR_REGNUM, fp + 8);
- trad_frame_set_reg_addr (this_cache, AARCH64_PC_REGNUM, fp + 8);
-
- trad_frame_set_id (this_cache, frame_id_build (fp, func));
+ trad_frame_set_id (this_cache, frame_id_build (sp, func));
}
static const struct tramp_frame aarch64_linux_rt_sigframe =