Re: [PATCH v2 00/23] All-stop on top of non-stop

Yao Qi
2015-04-10T08:41:07Z

Pedro Alves <palves@redhat.com> writes:

>> 3, on native,
>> -PASS: gdb.base/info-shared.exp: continue to breakpoint: library function #4
>> +FAIL: gdb.base/info-shared.exp: continue to breakpoint: library function #4
>>
>> continue^M
>> Continuing.^M
>> ^M
>> Program received signal SIGSEGV, Segmentation fault.^M
>> 0x40021564 in ?? () gdb/testsuite/gdb.base/info-shared-solib1.so^M
>> (gdb) FAIL: gdb.base/info-shared.exp: continue to breakpoint:
>> library function #4
>
> Most of the SIGSEGV/SIGILL/SIGBUS inferior crashes I saw were
> due to a displaced stepping bug. Force disabling displaced
> stepping with "set displaced off" usually "fixes" it. The next
> step I would usually take would be to run the test manually, with
> "set debug infrun 1" + "set debug displaced 1" + "set debug lin-lwp 1".

Yeah, it is a bug in displaced stepping... Here is the patch to fix it.

--
Yao (齐尧)

From: Yao Qi <yao.qi@linaro.org>
Subject: [PATCH] [arm] Fix displaced stepping for thumb alu reg instruction

Recent patch series "V2 All-stop on top of non-stop" causes a SIGSEGV
in the test case,

> -PASS: gdb.base/info-shared.exp: continue to breakpoint: library function #4
> +FAIL: gdb.base/info-shared.exp: continue to breakpoint: library function #4
>
> continue^M
> Continuing.^M
> ^M
> Program received signal SIGSEGV, Segmentation fault.^M
> 0x40021564 in ?? () gdb/testsuite/gdb.base/info-shared-solib1.so^M
> (gdb) FAIL: gdb.base/info-shared.exp: continue to breakpoint: library function #4

and an ARM displaced stepping bug is exposed. It can be reproduced by
the modified gdb.arch/arm-disp-step.exp as below,

continue^M
Continuing.^M
^M
Program received signal SIGSEGV, Segmentation fault.^M
0xa713cfcc in ?? ()^M
(gdb) FAIL: gdb.arch/arm-disp-step.exp: continue to breakpoint: continue to test_add_rn_pc_end

This patch is to fix it.

gdb:

2015-04-09 Yao Qi <yao.qi@linaro.org>

* arm-tdep.c (install_alu_reg): Update comment.
(thumb_copy_alu_reg): Remove local variable rn. Update
debugging message. Use r2 instead of r1 in the modified
instruction.

gdb/testsuite:

2015-04-09 Yao Qi <yao.qi@linaro.org>

* gdb.arch/arm-disp-step.S (main): Call test_add_rn_pc.
(test_add_rn_pc): New function.
* gdb.arch/arm-disp-step.exp (test_add_rn_pc): New proc.
(top level): Invoke test_add_rn_pc.

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 630a207..8181f25 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -6407,7 +6407,7 @@ install_alu_reg (struct gdbarch *gdbarch, struct regcache *regs,

Preparation: tmp1, tmp2, tmp3 <- r0, r1, r2;
r0, r1, r2 <- rd, rn, rm
- Insn: <op><cond> r0, r1, r2 [, <shift>]
+ Insn: <op><cond> r0, [r1,] r2 [, <shift>]
Cleanup: rd <- r0; r0, r1, r2 <- tmp1, tmp2, tmp3
*/

@@ -6454,22 +6454,21 @@ thumb_copy_alu_reg (struct gdbarch *gdbarch, uint16_t insn,
struct regcache *regs,
struct displaced_step_closure *dsc)
{
- unsigned rn, rm, rd;
+ unsigned rm, rd;

- rd = bits (insn, 3, 6);
- rn = (bit (insn, 7) << 3) | bits (insn, 0, 2);
- rm = 2;
+ rm = bits (insn, 3, 6);
+ rd = (bit (insn, 7) << 3) | bits (insn, 0, 2);

- if (rd != ARM_PC_REGNUM && rn != ARM_PC_REGNUM)
+ if (rd != ARM_PC_REGNUM && rm != ARM_PC_REGNUM)
return thumb_copy_unmodified_16bit (gdbarch, insn, "ALU reg", dsc);

if (debug_displaced)
- fprintf_unfiltered (gdb_stdlog, "displaced: copying reg %s insn %.4x\n",
- "ALU", (unsigned short) insn);
+ fprintf_unfiltered (gdb_stdlog, "displaced: copying ALU reg insn %.4x\n",
+ (unsigned short) insn);

- dsc->modinsn[0] = ((insn & 0xff00) | 0x08);
+ dsc->modinsn[0] = ((insn & 0xff00) | 0x10);

- install_alu_reg (gdbarch, regs, dsc, rd, rn, rm);
+ install_alu_reg (gdbarch, regs, dsc, rd, rd, rm);

return 0;
}
diff --git a/gdb/testsuite/gdb.arch/arm-disp-step.S b/gdb/testsuite/gdb.arch/arm-disp-step.S
index c9c9447..c3b1088 100644
--- a/gdb/testsuite/gdb.arch/arm-disp-step.S
+++ b/gdb/testsuite/gdb.arch/arm-disp-step.S
@@ -78,6 +78,11 @@ test_ret_end:
#if !defined(__thumb__)
bl test_str_pc
#endif
+
+ /* Test add with pc in Thumb and Thumb-2 */
+#if defined(__thumb__)
+ bl test_add_rn_pc
+#endif
/* Return */
mov sp, r7
sub sp, sp, #4
@@ -372,3 +377,17 @@ pc_offset_wrong:
test_str_pc_end:
bx lr
#endif
+
+#if defined(__thumb__)
+ .global test_add_rn_pc
+ .code 16
+ .thumb_func
+test_add_rn_pc:
+ mov r3, 4
+test_add_rn_pc_start:
+ add r3, pc
+ .global test_add_rn_pc_end
+test_add_rn_pc_end:
+ bx lr
+ .size test_add_rn_pc, .-test_add_rn_pc
+#endif
diff --git a/gdb/testsuite/gdb.arch/arm-disp-step.exp b/gdb/testsuite/gdb.arch/arm-disp-step.exp
index 7eac4d1..e191f73 100644
--- a/gdb/testsuite/gdb.arch/arm-disp-step.exp
+++ b/gdb/testsuite/gdb.arch/arm-disp-step.exp
@@ -371,6 +371,45 @@ proc test_str_pc {} {
".*bx lr.*"
}

+# Test 16 bit thumb instruction 'add rd, pc'.
+
+proc test_add_rn_pc {} {
+ global srcfile gdb_prompt
+
+ set test "break test_add_rn_pc"
+ gdb_test_multiple "break *test_add_rn_pc" $test {
+ -re "Breakpoint.*at.* file .*$srcfile, line.*\r\n$gdb_prompt $" {
+ pass $test
+ }
+ -re "No symbol.*\r\n$gdb_prompt $" {
+ return
+ }
+ }
+
+ gdb_continue_to_breakpoint "continue to test_add_rn_pc" \
+ ".*mov.*r3, 4.*"
+
+ gdb_test "break *test_add_rn_pc_start" \
+ "Breakpoint.*at.* file .*$srcfile, line.*" \
+ "break test_add_rn_pc_start"
+
+ gdb_continue_to_breakpoint "continue to test_add_rn_pc_start" \
+ ".*add.*r3,.*pc.*"
+
+ set pc_val [get_integer_valueof "\$pc" 0]
+
+ gdb_test "break *test_add_rn_pc_end" \
+ "Breakpoint.*at.* file .*$srcfile, line.*" \
+ "break test_add_rn_pc_end"
+
+ gdb_continue_to_breakpoint "continue to test_add_rn_pc_end" \
+ ".*bx lr.*"
+
+ set r3_val [get_integer_valueof "\$r3" 0]
+ # Test the value in r3 is correct.
+ gdb_assert { [expr {$pc_val + 4 + 4} == $r3_val] }
+}
+
# Get things started.

clean_restart ${testfile}
@@ -410,6 +449,8 @@ test_pop_pc

test_str_pc

+test_add_rn_pc
+
##########################################

# Done, run program to exit.