* [PATCH 3/4] Stop prologue analysis when past the epilogue
2014-07-03 6:11 [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
@ 2014-07-03 6:11 ` Yao Qi
2014-07-03 8:39 ` Will Newton
2014-07-11 13:39 ` Joel Brobecker
2014-07-03 6:11 ` [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode Yao Qi
` (3 subsequent siblings)
4 siblings, 2 replies; 17+ messages in thread
From: Yao Qi @ 2014-07-03 6:11 UTC (permalink / raw)
To: gdb-patches
We see a fail in gdb.trace/entry-values.exp on armv4t thumb,
bt^M
#0 0x000086fc in foo (i=0, i@entry=<optimized out>, j=2, j@entry=<optimized out>)^M
#1 0x00000002 in ?? ()^M
Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
(gdb) FAIL: gdb.trace/entry-values.exp: bt (1) (pattern 1)
The fail is caused by incorrect prologue analysis, which can be illustrated by
setting a breakpoint on function foo,
(gdb) disassemble foo
Dump of assembler code for function foo:
0x000086e8 <+0>: push {r7, lr}
0x000086ea <+2>: sub sp, #8
0x000086ec <+4>: add r7, sp, #0
0x000086ee <+6>: str r0, [r7, #4]
0x000086f0 <+8>: str r1, [r7, #0]
0x000086f2 <+10>: movs r3, #0
0x000086f4 <+12>: adds r0, r3, #0
0x000086f6 <+14>: mov sp, r7
0x000086f8 <+16>: add sp, #8
0x000086fa <+18>: pop {r7}
0x000086fc <+20>: pop {r1}
0x000086fe <+22>: bx r1
End of assembler dump.
(gdb) b foo
Breakpoint 1 at 0x86fc
As we can see, GDB analyzes the prologue and skip the prologue to the last
instruction but one. The breakpoint is set within the epilogue, and GDB
skips too many instruction for prologue. This patch teaches GDB to stop
prologue analysis when goes into the epilogue. With this patch applied,
GDB is able to unwind correctly,
(gdb) bt
#0 0x000086f6 in foo (i=0, i@entry=2, j=2, j@entry=3)
#1 0x00008718 in bar (i=<optimized out>)
#2 0x00008758 in main ()
gdb:
2014-07-02 Yao Qi <yao@codesourcery.com>
* arm-tdep.c (thumb_analyze_prologue): Break the loop if
thumb_instruction_restores_sp return true.
---
gdb/arm-tdep.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 153ef42..72beeb1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -754,6 +754,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
-offset);
}
+ else if (thumb_instruction_restores_sp (insn))
+ {
+ /* Don't scan past the epilogue. */
+ break;
+ }
else if ((insn & 0xf800) == 0xa800) /* add Rd, sp, #imm */
regs[bits (insn, 8, 10)] = pv_add_constant (regs[ARM_SP_REGNUM],
(insn & 0xff) << 2);
--
1.9.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode
@ 2014-07-03 6:11 Yao Qi
2014-07-03 6:11 ` [PATCH 3/4] Stop prologue analysis when past the epilogue Yao Qi
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Yao Qi @ 2014-07-03 6:11 UTC (permalink / raw)
To: gdb-patches
We see some fails in gdb.trace/entry-values.exp in thumb mode
(-mthumb -march={armv4t,armv7-a}), which are caused by two problems in
gdb,
- prologue analysis in gdb for thumb code is broken. Patch #3 is to
fix it by stopping prologue analysis when it goes to the epilogue.
- dwarf assembler in gdb.trace/entry-values.exp uses functions whose
address's bit 0 is set, which is incorrect. Patch #4 is to fix it.
Patch 1 is preparatory patch and patch 2 is a refactor one. Each of them
is regression tested on arm-none-linux-gnueabi.
*** BLURB HERE ***
Yao Qi (4):
Restrict matching add/sub sp, #imm
Match instruction adjusts SP in thumb
Stop prologue analysis when past the epilogue
Fix gdb.trace/entry-values.exp for thumb mode
gdb/arm-tdep.c | 41 +++++++++++++++++---------------
gdb/testsuite/gdb.trace/entry-values.c | 6 +++++
gdb/testsuite/gdb.trace/entry-values.exp | 10 ++++----
3 files changed, 33 insertions(+), 24 deletions(-)
--
1.9.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode
2014-07-03 6:11 [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
2014-07-03 6:11 ` [PATCH 3/4] Stop prologue analysis when past the epilogue Yao Qi
@ 2014-07-03 6:11 ` Yao Qi
2014-07-07 15:15 ` Joel Brobecker
2014-07-03 6:11 ` [PATCH 1/4] Restrict matching add/sub sp, #imm Yao Qi
` (2 subsequent siblings)
4 siblings, 1 reply; 17+ messages in thread
From: Yao Qi @ 2014-07-03 6:11 UTC (permalink / raw)
To: gdb-patches
We see some fails in gdb.trace/entry-values.exp in thumb mode
(-mthumb -march={armv4t,armv7-a}).
In thumb mode, the lsb of references to 'foo' and 'bar' in the assembly
(produced by dwarf assember) is set, so the generated debug
information is incorrect.
This patch copies the approach used by
[PATCH 4/4] Fix dw2-ifort-parameter.exp on PPC64
https://sourceware.org/ml/gdb-patches/2014-03/msg00202.html
to introduce new labels 'foo_start' and 'bar_start' which are about
the correct function address (without lsb set). This patch fixes
these fails we've seen. This should also fix the fails on PPC64, but
I didn't test.
gdb/testsuite:
2014-07-02 Yao Qi <yao@codesourcery.com>
* gdb.trace/entry-values.c: Define labels 'foo_start' and
'bar_start' at the beginning of functions 'foo' and 'bar'
respectively.
* gdb.trace/entry-values.exp: Use 'foo_start' and 'bar_start'
instead of 'foo' and 'bar'.
---
gdb/testsuite/gdb.trace/entry-values.c | 6 ++++++
gdb/testsuite/gdb.trace/entry-values.exp | 10 +++++-----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/gdb/testsuite/gdb.trace/entry-values.c b/gdb/testsuite/gdb.trace/entry-values.c
index 4f80eb0..11bb739 100644
--- a/gdb/testsuite/gdb.trace/entry-values.c
+++ b/gdb/testsuite/gdb.trace/entry-values.c
@@ -15,12 +15,18 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
+asm (".section \".text\"");
+asm (".balign 8");
+asm ("foo_start: .globl foo_start");
+
int
foo (int i, int j)
{
return 0;
}
+asm ("bar_start: .globl bar_start");
+
int
bar (int i)
{
diff --git a/gdb/testsuite/gdb.trace/entry-values.exp b/gdb/testsuite/gdb.trace/entry-values.exp
index f10ffa6..3ccb4d2 100644
--- a/gdb/testsuite/gdb.trace/entry-values.exp
+++ b/gdb/testsuite/gdb.trace/entry-values.exp
@@ -145,8 +145,8 @@ Dwarf::assemble $asm_file {
foo_label: subprogram {
{name foo}
{decl_file 1}
- {low_pc foo addr}
- {high_pc "foo + $foo_length" addr}
+ {low_pc foo_start addr}
+ {high_pc "foo_start + $foo_length" addr}
} {
formal_parameter {
{type :$int_label}
@@ -163,8 +163,8 @@ Dwarf::assemble $asm_file {
subprogram {
{name bar}
{decl_file 1}
- {low_pc bar addr}
- {high_pc "bar + $bar_length" addr}
+ {low_pc bar_start addr}
+ {high_pc "bar_start + $bar_length" addr}
{GNU_all_call_sites 1}
} {
formal_parameter {
@@ -173,7 +173,7 @@ Dwarf::assemble $asm_file {
}
GNU_call_site {
- {low_pc "bar + $bar_call_foo" addr}
+ {low_pc "bar_start + $bar_call_foo" addr}
{abstract_origin :$foo_label}
} {
# Faked entry values are reference to variables 'global1'
--
1.9.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] Match instruction adjusts SP in thumb
2014-07-03 6:11 [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
` (2 preceding siblings ...)
2014-07-03 6:11 ` [PATCH 1/4] Restrict matching add/sub sp, #imm Yao Qi
@ 2014-07-03 6:11 ` Yao Qi
2014-07-03 8:35 ` Will Newton
` (2 more replies)
2014-07-11 7:34 ` [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
4 siblings, 3 replies; 17+ messages in thread
From: Yao Qi @ 2014-07-03 6:11 UTC (permalink / raw)
To: gdb-patches
This is a refactor patch, that moves matching instructions adjusting
SP into a new function, thumb_instruction_restores_sp. The second
call to thumb_instruction_restores_sp in thumb_in_function_epilogue_p
is a little different from the original. The original code matches
'POP <registers> without PC', but thumb_in_function_epilogue_p matches
'POP <registers> (with and without PC)'. However, GDB found one
instruction about return and is scanning the previous instruction,
which should be an instruction about return too, so the code change
doesn't affect the functionality.
gdb:
2014-07-02 Yao Qi <yao@codesourcery.com>
* arm-tdep.c (thumb_instruction_restores_sp): New function.
(thumb_in_function_epilogue_p): Call
thumb_instruction_restores_sp.
---
gdb/arm-tdep.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0fc7fc1..153ef42 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -685,6 +685,17 @@ thumb2_instruction_changes_pc (unsigned short inst1, unsigned short inst2)
return 0;
}
+/* Return 1 if the 16-bit Thumb instruction INSN restores SP in
+ epilogue, 0 otherwise. */
+
+static int
+thumb_instruction_restores_sp (unsigned short insn)
+{
+ return (insn == 0x46bd /* mov sp, r7 */
+ || (insn & 0xff80) == 0xb000 /* add sp, imm */
+ || (insn & 0xfe00) == 0xbc00); /* pop <registers> */
+}
+
/* Analyze a Thumb prologue, looking for a recognizable stack frame
and frame pointer. Scan until we encounter a store that could
clobber the stack frame unexpectedly, or an unknown instruction.
@@ -3257,14 +3268,10 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
found_return = 1;
else if (insn == 0x46f7) /* mov pc, lr */
found_return = 1;
- else if (insn == 0x46bd) /* mov sp, r7 */
- found_stack_adjust = 1;
- else if ((insn & 0xff80) == 0xb000) /* add sp, imm */
- found_stack_adjust = 1;
- else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
+ else if (thumb_instruction_restores_sp (insn))
{
found_stack_adjust = 1;
- if (insn & 0x0100) /* <registers> include PC. */
+ if ((insn & 0xfe00) == 0xbd00) /* pop <registers, PC> */
found_return = 1;
}
else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instruction */
@@ -3317,11 +3324,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
insn = extract_unsigned_integer (buf, 2, byte_order_for_code);
insn2 = extract_unsigned_integer (buf + 2, 2, byte_order_for_code);
- if (insn2 == 0x46bd) /* mov sp, r7 */
- found_stack_adjust = 1;
- else if ((insn2 & 0xff80) == 0xb000) /* add sp, imm */
- found_stack_adjust = 1;
- else if ((insn2 & 0xff00) == 0xbc00) /* pop <registers> without PC */
+ if (thumb_instruction_restores_sp (insn2))
found_stack_adjust = 1;
else if (insn == 0xe8bd) /* ldm.w sp!, <registers> */
found_stack_adjust = 1;
--
1.9.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/4] Restrict matching add/sub sp, #imm
2014-07-03 6:11 [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
2014-07-03 6:11 ` [PATCH 3/4] Stop prologue analysis when past the epilogue Yao Qi
2014-07-03 6:11 ` [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode Yao Qi
@ 2014-07-03 6:11 ` Yao Qi
2014-07-03 8:31 ` Will Newton
2014-07-03 6:11 ` [PATCH 2/4] Match instruction adjusts SP in thumb Yao Qi
2014-07-11 7:34 ` [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
4 siblings, 1 reply; 17+ messages in thread
From: Yao Qi @ 2014-07-03 6:11 UTC (permalink / raw)
To: gdb-patches
Currently, GDB matches both add/sub sp, #imm in prologue and epilogue,
which is not very precise. On the instruction level, the immediate
number in both instruction can't be negative, so 'sub sp, #imm' only
appears in prologue while 'add sp, #imm' only appears in epilogue.
Note that on assembly level, we can write 'add sp, -8', but gas will
translate to 'sub sp, 8' instruction.
This patch is to only match 'sub sp, #imm' in prologue and match
'add sp, #immm' in epilogue. It paves the way for the following
patch.
gdb:
2014-07-02 Yao Qi <yao@codesourcery.com>
* arm-tdep.c (thumb_analyze_prologue): Don't match instruction
'add sp, #immm'.
(thumb_in_function_epilogue_p): Don't match 'sub sp, #imm'.
---
gdb/arm-tdep.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 8cc60a4..0fc7fc1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -737,16 +737,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
pv_area_store (stack, regs[ARM_SP_REGNUM], 4, regs[regno]);
}
}
- else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR
- sub sp, #simm */
+ else if ((insn & 0xff80) == 0xb080) /* sub sp, #simm */
{
offset = (insn & 0x7f) << 2; /* get scaled offset */
- if (insn & 0x80) /* Check for SUB. */
- regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
- -offset);
- else
- regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
- offset);
+ regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
+ -offset);
}
else if ((insn & 0xf800) == 0xa800) /* add Rd, sp, #imm */
regs[bits (insn, 8, 10)] = pv_add_constant (regs[ARM_SP_REGNUM],
@@ -3264,7 +3259,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
found_return = 1;
else if (insn == 0x46bd) /* mov sp, r7 */
found_stack_adjust = 1;
- else if ((insn & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */
+ else if ((insn & 0xff80) == 0xb000) /* add sp, imm */
found_stack_adjust = 1;
else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
{
@@ -3324,7 +3319,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
if (insn2 == 0x46bd) /* mov sp, r7 */
found_stack_adjust = 1;
- else if ((insn2 & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */
+ else if ((insn2 & 0xff80) == 0xb000) /* add sp, imm */
found_stack_adjust = 1;
else if ((insn2 & 0xff00) == 0xbc00) /* pop <registers> without PC */
found_stack_adjust = 1;
--
1.9.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] Restrict matching add/sub sp, #imm
2014-07-03 6:11 ` [PATCH 1/4] Restrict matching add/sub sp, #imm Yao Qi
@ 2014-07-03 8:31 ` Will Newton
2014-07-07 1:38 ` Yao Qi
0 siblings, 1 reply; 17+ messages in thread
From: Will Newton @ 2014-07-03 8:31 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 3 July 2014 07:09, Yao Qi <yao@codesourcery.com> wrote:
> Currently, GDB matches both add/sub sp, #imm in prologue and epilogue,
> which is not very precise. On the instruction level, the immediate
> number in both instruction can't be negative, so 'sub sp, #imm' only
> appears in prologue while 'add sp, #imm' only appears in epilogue.
> Note that on assembly level, we can write 'add sp, -8', but gas will
> translate to 'sub sp, 8' instruction.
>
> This patch is to only match 'sub sp, #imm' in prologue and match
> 'add sp, #immm' in epilogue. It paves the way for the following
> patch.
>
> gdb:
>
> 2014-07-02 Yao Qi <yao@codesourcery.com>
>
> * arm-tdep.c (thumb_analyze_prologue): Don't match instruction
> 'add sp, #immm'.
One too many ms?
> (thumb_in_function_epilogue_p): Don't match 'sub sp, #imm'.
> ---
> gdb/arm-tdep.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 8cc60a4..0fc7fc1 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -737,16 +737,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
> pv_area_store (stack, regs[ARM_SP_REGNUM], 4, regs[regno]);
> }
> }
> - else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR
> - sub sp, #simm */
> + else if ((insn & 0xff80) == 0xb080) /* sub sp, #simm */
I wonder if we should adjust the comment to just #imm, as #simm
implies it is a signed quantity.
> {
> offset = (insn & 0x7f) << 2; /* get scaled offset */
> - if (insn & 0x80) /* Check for SUB. */
> - regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
> - -offset);
> - else
> - regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
> - offset);
> + regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
> + -offset);
> }
> else if ((insn & 0xf800) == 0xa800) /* add Rd, sp, #imm */
> regs[bits (insn, 8, 10)] = pv_add_constant (regs[ARM_SP_REGNUM],
> @@ -3264,7 +3259,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> found_return = 1;
> else if (insn == 0x46bd) /* mov sp, r7 */
> found_stack_adjust = 1;
> - else if ((insn & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */
> + else if ((insn & 0xff80) == 0xb000) /* add sp, imm */
> found_stack_adjust = 1;
> else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
> {
> @@ -3324,7 +3319,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
>
> if (insn2 == 0x46bd) /* mov sp, r7 */
> found_stack_adjust = 1;
> - else if ((insn2 & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */
> + else if ((insn2 & 0xff80) == 0xb000) /* add sp, imm */
> found_stack_adjust = 1;
> else if ((insn2 & 0xff00) == 0xbc00) /* pop <registers> without PC */
> found_stack_adjust = 1;
Otherwise the patch looks good to me.
--
Will Newton
Toolchain Working Group, Linaro
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Match instruction adjusts SP in thumb
2014-07-03 6:11 ` [PATCH 2/4] Match instruction adjusts SP in thumb Yao Qi
@ 2014-07-03 8:35 ` Will Newton
2014-07-11 13:25 ` Joel Brobecker
2014-09-24 12:56 ` Yao Qi
2 siblings, 0 replies; 17+ messages in thread
From: Will Newton @ 2014-07-03 8:35 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 3 July 2014 07:09, Yao Qi <yao@codesourcery.com> wrote:
> This is a refactor patch, that moves matching instructions adjusting
> SP into a new function, thumb_instruction_restores_sp. The second
> call to thumb_instruction_restores_sp in thumb_in_function_epilogue_p
> is a little different from the original. The original code matches
> 'POP <registers> without PC', but thumb_in_function_epilogue_p matches
> 'POP <registers> (with and without PC)'. However, GDB found one
> instruction about return and is scanning the previous instruction,
> which should be an instruction about return too, so the code change
> doesn't affect the functionality.
>
> gdb:
>
> 2014-07-02 Yao Qi <yao@codesourcery.com>
>
> * arm-tdep.c (thumb_instruction_restores_sp): New function.
> (thumb_in_function_epilogue_p): Call
> thumb_instruction_restores_sp.
> ---
> gdb/arm-tdep.c | 25 ++++++++++++++-----------
> 1 file changed, 14 insertions(+), 11 deletions(-)
This patch looks good to me.
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 0fc7fc1..153ef42 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -685,6 +685,17 @@ thumb2_instruction_changes_pc (unsigned short inst1, unsigned short inst2)
> return 0;
> }
>
> +/* Return 1 if the 16-bit Thumb instruction INSN restores SP in
> + epilogue, 0 otherwise. */
> +
> +static int
> +thumb_instruction_restores_sp (unsigned short insn)
> +{
> + return (insn == 0x46bd /* mov sp, r7 */
> + || (insn & 0xff80) == 0xb000 /* add sp, imm */
> + || (insn & 0xfe00) == 0xbc00); /* pop <registers> */
> +}
> +
> /* Analyze a Thumb prologue, looking for a recognizable stack frame
> and frame pointer. Scan until we encounter a store that could
> clobber the stack frame unexpectedly, or an unknown instruction.
> @@ -3257,14 +3268,10 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> found_return = 1;
> else if (insn == 0x46f7) /* mov pc, lr */
> found_return = 1;
> - else if (insn == 0x46bd) /* mov sp, r7 */
> - found_stack_adjust = 1;
> - else if ((insn & 0xff80) == 0xb000) /* add sp, imm */
> - found_stack_adjust = 1;
> - else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
> + else if (thumb_instruction_restores_sp (insn))
> {
> found_stack_adjust = 1;
> - if (insn & 0x0100) /* <registers> include PC. */
> + if ((insn & 0xfe00) == 0xbd00) /* pop <registers, PC> */
> found_return = 1;
> }
> else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instruction */
> @@ -3317,11 +3324,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> insn = extract_unsigned_integer (buf, 2, byte_order_for_code);
> insn2 = extract_unsigned_integer (buf + 2, 2, byte_order_for_code);
>
> - if (insn2 == 0x46bd) /* mov sp, r7 */
> - found_stack_adjust = 1;
> - else if ((insn2 & 0xff80) == 0xb000) /* add sp, imm */
> - found_stack_adjust = 1;
> - else if ((insn2 & 0xff00) == 0xbc00) /* pop <registers> without PC */
> + if (thumb_instruction_restores_sp (insn2))
> found_stack_adjust = 1;
> else if (insn == 0xe8bd) /* ldm.w sp!, <registers> */
> found_stack_adjust = 1;
> --
> 1.9.0
>
--
Will Newton
Toolchain Working Group, Linaro
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] Stop prologue analysis when past the epilogue
2014-07-03 6:11 ` [PATCH 3/4] Stop prologue analysis when past the epilogue Yao Qi
@ 2014-07-03 8:39 ` Will Newton
2014-07-11 13:39 ` Joel Brobecker
1 sibling, 0 replies; 17+ messages in thread
From: Will Newton @ 2014-07-03 8:39 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 3 July 2014 07:09, Yao Qi <yao@codesourcery.com> wrote:
> We see a fail in gdb.trace/entry-values.exp on armv4t thumb,
>
> bt^M
> #0 0x000086fc in foo (i=0, i@entry=<optimized out>, j=2, j@entry=<optimized out>)^M
> #1 0x00000002 in ?? ()^M
> Backtrace stopped: previous frame identical to this frame (corrupt stack?)^M
> (gdb) FAIL: gdb.trace/entry-values.exp: bt (1) (pattern 1)
>
> The fail is caused by incorrect prologue analysis, which can be illustrated by
> setting a breakpoint on function foo,
>
> (gdb) disassemble foo
> Dump of assembler code for function foo:
> 0x000086e8 <+0>: push {r7, lr}
> 0x000086ea <+2>: sub sp, #8
> 0x000086ec <+4>: add r7, sp, #0
> 0x000086ee <+6>: str r0, [r7, #4]
> 0x000086f0 <+8>: str r1, [r7, #0]
> 0x000086f2 <+10>: movs r3, #0
> 0x000086f4 <+12>: adds r0, r3, #0
> 0x000086f6 <+14>: mov sp, r7
> 0x000086f8 <+16>: add sp, #8
> 0x000086fa <+18>: pop {r7}
> 0x000086fc <+20>: pop {r1}
> 0x000086fe <+22>: bx r1
> End of assembler dump.
> (gdb) b foo
> Breakpoint 1 at 0x86fc
>
> As we can see, GDB analyzes the prologue and skip the prologue to the last
> instruction but one. The breakpoint is set within the epilogue, and GDB
> skips too many instruction for prologue. This patch teaches GDB to stop
> prologue analysis when goes into the epilogue. With this patch applied,
> GDB is able to unwind correctly,
>
> (gdb) bt
> #0 0x000086f6 in foo (i=0, i@entry=2, j=2, j@entry=3)
> #1 0x00008718 in bar (i=<optimized out>)
> #2 0x00008758 in main ()
>
> gdb:
>
> 2014-07-02 Yao Qi <yao@codesourcery.com>
>
> * arm-tdep.c (thumb_analyze_prologue): Break the loop if
> thumb_instruction_restores_sp return true.
> ---
> gdb/arm-tdep.c | 5 +++++
> 1 file changed, 5 insertions(+)
This patch looks good to me too.
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 153ef42..72beeb1 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -754,6 +754,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
> regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
> -offset);
> }
> + else if (thumb_instruction_restores_sp (insn))
> + {
> + /* Don't scan past the epilogue. */
> + break;
> + }
> else if ((insn & 0xf800) == 0xa800) /* add Rd, sp, #imm */
> regs[bits (insn, 8, 10)] = pv_add_constant (regs[ARM_SP_REGNUM],
> (insn & 0xff) << 2);
> --
> 1.9.0
>
--
Will Newton
Toolchain Working Group, Linaro
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] Restrict matching add/sub sp, #imm
2014-07-03 8:31 ` Will Newton
@ 2014-07-07 1:38 ` Yao Qi
2014-07-11 13:25 ` Joel Brobecker
0 siblings, 1 reply; 17+ messages in thread
From: Yao Qi @ 2014-07-07 1:38 UTC (permalink / raw)
To: Will Newton; +Cc: gdb-patches
On 07/03/2014 04:31 PM, Will Newton wrote:
> One too many ms?
>
Oh, sorry.
>> > (thumb_in_function_epilogue_p): Don't match 'sub sp, #imm'.
>> > ---
>> > gdb/arm-tdep.c | 15 +++++----------
>> > 1 file changed, 5 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> > index 8cc60a4..0fc7fc1 100644
>> > --- a/gdb/arm-tdep.c
>> > +++ b/gdb/arm-tdep.c
>> > @@ -737,16 +737,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
>> > pv_area_store (stack, regs[ARM_SP_REGNUM], 4, regs[regno]);
>> > }
>> > }
>> > - else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR
>> > - sub sp, #simm */
>> > + else if ((insn & 0xff80) == 0xb080) /* sub sp, #simm */
> I wonder if we should adjust the comment to just #imm, as #simm
> implies it is a signed quantity.
>
Fixed.
--
Yao (é½å°§)
gdb:
2014-07-07 Yao Qi <yao@codesourcery.com>
* arm-tdep.c (thumb_analyze_prologue): Don't match instruction
'add sp, #imm'.
(thumb_in_function_epilogue_p): Don't match 'sub sp, #imm'.
---
gdb/arm-tdep.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 8cc60a4..6b1cf3c 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -737,16 +737,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
pv_area_store (stack, regs[ARM_SP_REGNUM], 4, regs[regno]);
}
}
- else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR
- sub sp, #simm */
+ else if ((insn & 0xff80) == 0xb080) /* sub sp, #imm */
{
offset = (insn & 0x7f) << 2; /* get scaled offset */
- if (insn & 0x80) /* Check for SUB. */
- regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
- -offset);
- else
- regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
- offset);
+ regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM],
+ -offset);
}
else if ((insn & 0xf800) == 0xa800) /* add Rd, sp, #imm */
regs[bits (insn, 8, 10)] = pv_add_constant (regs[ARM_SP_REGNUM],
@@ -3264,7 +3259,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
found_return = 1;
else if (insn == 0x46bd) /* mov sp, r7 */
found_stack_adjust = 1;
- else if ((insn & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */
+ else if ((insn & 0xff80) == 0xb000) /* add sp, imm */
found_stack_adjust = 1;
else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
{
@@ -3324,7 +3319,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
if (insn2 == 0x46bd) /* mov sp, r7 */
found_stack_adjust = 1;
- else if ((insn2 & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */
+ else if ((insn2 & 0xff80) == 0xb000) /* add sp, imm */
found_stack_adjust = 1;
else if ((insn2 & 0xff00) == 0xbc00) /* pop <registers> without PC */
found_stack_adjust = 1;
--
1.9.0
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode
2014-07-03 6:11 ` [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode Yao Qi
@ 2014-07-07 15:15 ` Joel Brobecker
2014-07-08 8:53 ` Yao Qi
0 siblings, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2014-07-07 15:15 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> 2014-07-02 Yao Qi <yao@codesourcery.com>
>
> * gdb.trace/entry-values.c: Define labels 'foo_start' and
> 'bar_start' at the beginning of functions 'foo' and 'bar'
> respectively.
> * gdb.trace/entry-values.exp: Use 'foo_start' and 'bar_start'
> instead of 'foo' and 'bar'.
This is OK.
Thank you,
--
Joel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode
2014-07-07 15:15 ` Joel Brobecker
@ 2014-07-08 8:53 ` Yao Qi
0 siblings, 0 replies; 17+ messages in thread
From: Yao Qi @ 2014-07-08 8:53 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On 07/07/2014 11:15 PM, Joel Brobecker wrote:
>> 2014-07-02 Yao Qi <yao@codesourcery.com>
>>
>> * gdb.trace/entry-values.c: Define labels 'foo_start' and
>> 'bar_start' at the beginning of functions 'foo' and 'bar'
>> respectively.
>> * gdb.trace/entry-values.exp: Use 'foo_start' and 'bar_start'
>> instead of 'foo' and 'bar'.
>
> This is OK.
>
Patch is pushed in.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode
2014-07-03 6:11 [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
` (3 preceding siblings ...)
2014-07-03 6:11 ` [PATCH 2/4] Match instruction adjusts SP in thumb Yao Qi
@ 2014-07-11 7:34 ` Yao Qi
4 siblings, 0 replies; 17+ messages in thread
From: Yao Qi @ 2014-07-11 7:34 UTC (permalink / raw)
To: gdb-patches
On 07/03/2014 02:09 PM, Yao Qi wrote:
> We see some fails in gdb.trace/entry-values.exp in thumb mode
> (-mthumb -march={armv4t,armv7-a}), which are caused by two problems in
> gdb,
>
> - prologue analysis in gdb for thumb code is broken. Patch #3 is to
> fix it by stopping prologue analysis when it goes to the epilogue.
> - dwarf assembler in gdb.trace/entry-values.exp uses functions whose
> address's bit 0 is set, which is incorrect. Patch #4 is to fix it.
>
> Patch 1 is preparatory patch and patch 2 is a refactor one. Each of them
> is regression tested on arm-none-linux-gnueabi.
Ping. Patch 4 was approved by Joel and was pushed in, but patches 1~3
still need approval.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] Restrict matching add/sub sp, #imm
2014-07-07 1:38 ` Yao Qi
@ 2014-07-11 13:25 ` Joel Brobecker
2014-07-11 13:49 ` Yao Qi
0 siblings, 1 reply; 17+ messages in thread
From: Joel Brobecker @ 2014-07-11 13:25 UTC (permalink / raw)
To: Yao Qi; +Cc: Will Newton, gdb-patches
> 2014-07-07 Yao Qi <yao@codesourcery.com>
>
> * arm-tdep.c (thumb_analyze_prologue): Don't match instruction
> 'add sp, #imm'.
> (thumb_in_function_epilogue_p): Don't match 'sub sp, #imm'.
Sorry for the delay, Yao. The updated version is OK to push.
--
Joel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Match instruction adjusts SP in thumb
2014-07-03 6:11 ` [PATCH 2/4] Match instruction adjusts SP in thumb Yao Qi
2014-07-03 8:35 ` Will Newton
@ 2014-07-11 13:25 ` Joel Brobecker
2014-09-24 12:56 ` Yao Qi
2 siblings, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-07-11 13:25 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> 2014-07-02 Yao Qi <yao@codesourcery.com>
>
> * arm-tdep.c (thumb_instruction_restores_sp): New function.
> (thumb_in_function_epilogue_p): Call
> thumb_instruction_restores_sp.
OK.
--
Joel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] Stop prologue analysis when past the epilogue
2014-07-03 6:11 ` [PATCH 3/4] Stop prologue analysis when past the epilogue Yao Qi
2014-07-03 8:39 ` Will Newton
@ 2014-07-11 13:39 ` Joel Brobecker
1 sibling, 0 replies; 17+ messages in thread
From: Joel Brobecker @ 2014-07-11 13:39 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
> 2014-07-02 Yao Qi <yao@codesourcery.com>
>
> * arm-tdep.c (thumb_analyze_prologue): Break the loop if
> thumb_instruction_restores_sp return true.
OK to push.
--
Joel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] Restrict matching add/sub sp, #imm
2014-07-11 13:25 ` Joel Brobecker
@ 2014-07-11 13:49 ` Yao Qi
0 siblings, 0 replies; 17+ messages in thread
From: Yao Qi @ 2014-07-11 13:49 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Will Newton, gdb-patches
On 07/11/2014 09:25 PM, Joel Brobecker wrote:
> Sorry for the delay, Yao. The updated version is OK to push.
Thanks for the review, Joel. All of them are pushed in.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] Match instruction adjusts SP in thumb
2014-07-03 6:11 ` [PATCH 2/4] Match instruction adjusts SP in thumb Yao Qi
2014-07-03 8:35 ` Will Newton
2014-07-11 13:25 ` Joel Brobecker
@ 2014-09-24 12:56 ` Yao Qi
2 siblings, 0 replies; 17+ messages in thread
From: Yao Qi @ 2014-09-24 12:56 UTC (permalink / raw)
To: gdb-patches
On 07/03/2014 02:09 PM, Yao Qi wrote:
> - else if ((insn & 0xfe00) == 0xbc00) /* pop <registers> */
> + else if (thumb_instruction_restores_sp (insn))
> {
> found_stack_adjust = 1;
> - if (insn & 0x0100) /* <registers> include PC. */
> + if ((insn & 0xfe00) == 0xbd00) /* pop <registers, PC> */
> found_return = 1;
> }
This patch fixes a typo in the bit mask I've made in my previous code
refactor. If PC is in the register list, the bit 8 is one, so bit
mask 0xff00 should be used. Current condition is a constant false.
Regression tested on arm-linux-gnueabi. Patch is pushed in.
gdb:
2014-09-24 Yao Qi <yao@codesourcery.com>
* arm-tdep.c (thumb_in_function_epilogue_p): Fix typo in the
bitmask.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 742053f..5dccf0a 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3273,7 +3273,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
found_return = 1;
else if (thumb_instruction_restores_sp (insn))
{
- if ((insn & 0xfe00) == 0xbd00) /* pop <registers, PC> */
+ if ((insn & 0xff00) == 0xbd00) /* pop <registers, PC> */
found_return = 1;
}
else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instruction */
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-09-24 12:56 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-03 6:11 [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
2014-07-03 6:11 ` [PATCH 3/4] Stop prologue analysis when past the epilogue Yao Qi
2014-07-03 8:39 ` Will Newton
2014-07-11 13:39 ` Joel Brobecker
2014-07-03 6:11 ` [PATCH 4/4] Fix gdb.trace/entry-values.exp for thumb mode Yao Qi
2014-07-07 15:15 ` Joel Brobecker
2014-07-08 8:53 ` Yao Qi
2014-07-03 6:11 ` [PATCH 1/4] Restrict matching add/sub sp, #imm Yao Qi
2014-07-03 8:31 ` Will Newton
2014-07-07 1:38 ` Yao Qi
2014-07-11 13:25 ` Joel Brobecker
2014-07-11 13:49 ` Yao Qi
2014-07-03 6:11 ` [PATCH 2/4] Match instruction adjusts SP in thumb Yao Qi
2014-07-03 8:35 ` Will Newton
2014-07-11 13:25 ` Joel Brobecker
2014-09-24 12:56 ` Yao Qi
2014-07-11 7:34 ` [PATCH 0/4] Fix gdb.trace/entry-values.exp fails in thumb mode Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).