[PATCH v2] gdb: riscv_scan_prologue: handle LD and LW instructions
Andrew Burgess
andrew.burgess@embecosm.com
Wed Aug 11 15:53:06 GMT 2021
* Lancelot SIX <lsix@lancelotsix.com> [2021-08-10 22:37:54 +0000]:
> Noticeable changes from V1:
>
> - Following review from Andrew Burgess, add support for LW.
> - Explicitly include C.LD and C.LW in the testcase.
>
> ---
>
> While working on the testsuite, I ended up noticing that GDB fails to
> produce a full backtrace from a thread waiting in pthread_join. When
> selecting the waiting thread and using the 'bt' command, the following
> result can be observed:
>
> (gdb) bt
> #0 0x0000003ff7fccd20 in __futex_abstimed_wait_common64 () from /lib/riscv64-linux-gnu/libpthread.so.0
> #1 0x0000003ff7fc43da in __pthread_clockjoin_ex () from /lib/riscv64-linux-gnu/libpthread.so.0
> Backtrace stopped: frame did not save the PC
>
> On my platform, I do not have debug symbols for glibc, so I need to rely
> on prologue analysis in order to unwind stack.
>
> Here is what the function prologue looks like:
>
> (gdb) disassemble __pthread_clockjoin_ex
> Dump of assembler code for function __pthread_clockjoin_ex:
> 0x0000003ff7fc42de <+0>: addi sp,sp,-144
> 0x0000003ff7fc42e0 <+2>: sd s5,88(sp)
> 0x0000003ff7fc42e2 <+4>: auipc s5,0xd
> 0x0000003ff7fc42e6 <+8>: ld s5,-2(s5) # 0x3ff7fd12e0
> 0x0000003ff7fc42ea <+12>: ld a5,0(s5)
> 0x0000003ff7fc42ee <+16>: sd ra,136(sp)
> 0x0000003ff7fc42f0 <+18>: sd s0,128(sp)
> 0x0000003ff7fc42f2 <+20>: sd s1,120(sp)
> 0x0000003ff7fc42f4 <+22>: sd s2,112(sp)
> 0x0000003ff7fc42f6 <+24>: sd s3,104(sp)
> 0x0000003ff7fc42f8 <+26>: sd s4,96(sp)
> 0x0000003ff7fc42fa <+28>: sd s6,80(sp)
> 0x0000003ff7fc42fc <+30>: sd s7,72(sp)
> 0x0000003ff7fc42fe <+32>: sd s8,64(sp)
> 0x0000003ff7fc4300 <+34>: sd s9,56(sp)
> 0x0000003ff7fc4302 <+36>: sd a5,40(sp)
>
> As far as prologue analysis is concerned, the most interesting part is
> done at address 0x0000003ff7fc42ee (<+16>): 'sd ra,136(sp)'. This stores
> the RA (return address) register on the stack, which is the information
> we are looking for in order to identify the caller.
>
> In the current implementation of the prologue scanner, GDB stops when
> hitting 0x0000003ff7fc42e6 (<+8>) because it does not know what to do
> with the 'ld' instruction. GDB thinks it reached the end of the
> prologue but have not yet reached the important part, which explain
> GDB's inability to unwind past this point.
>
> The section of the prologue starting at <+4> until <+12> is used to load
> the stack canary[1], which will then be placed on the stack at <+36> at
> the end of the prologue.
>
> In order to have the prologue properly handled, this commit proposes to
> add support for the ld instruction in the RISC-V prologue scanner.
> I guess riscv32 would use lw in such situation so this patch also adds
> support for this instruction.
>
> With this patch applied, gdb is now able to unwind past pthread_join:
>
> (gdb) bt
> #0 0x0000003ff7fccd20 in __futex_abstimed_wait_common64 () from /lib/riscv64-linux-gnu/libpthread.so.0
> #1 0x0000003ff7fc43da in __pthread_clockjoin_ex () from /lib/riscv64-linux-gnu/libpthread.so.0
> #2 0x0000002aaaaaa88e in bar() ()
> #3 0x0000002aaaaaa8c4 in foo() ()
> #4 0x0000002aaaaaa8da in main ()
>
> I have had a look to see if I could reproduce this easily, but in my
> simple testcases using '-fstack-protector-all', the canary is loaded
> after the RA register is saved. I do not have a reliable way of
> generating a prologue similar to the problematic one so I forged one
> instead.
>
> The testsuite have been run on riscv64 ubuntu 21.01 with no regression
> observed.
LGTM.
Thanks for doing this.
Andrew
>
> [1] https://en.wikipedia.org/wiki/Buffer_overflow_protection#Canaries
> ---
> gdb/riscv-tdep.c | 33 +++++++++
> .../riscv64-unwind-prologue-with-ld-lw-foo.s | 74 +++++++++++++++++++
> .../riscv64-unwind-prologue-with-ld-lw.exp | 45 +++++++++++
> .../riscv64-unwind-prologue-with-ld.c | 30 ++++++++
> 4 files changed, 182 insertions(+)
> create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw-foo.s
> create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw.exp
> create mode 100644 gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld.c
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index b5b0d2d79de..8b55bc33ded 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1409,6 +1409,8 @@ class riscv_insn
> LUI,
> SD,
> SW,
> + LD,
> + LW,
> /* These are needed for software breakpoint support. */
> JAL,
> JALR,
> @@ -1519,6 +1521,15 @@ class riscv_insn
> m_imm.s = EXTRACT_CITYPE_IMM (ival);
> }
>
> + /* Helper for DECODE, decode 16-bit compressed CL-type instruction. */
> + void decode_cl_type_insn (enum opcode opcode, ULONGEST ival)
> + {
> + m_opcode = opcode;
> + m_rd = decode_register_index_short (ival, OP_SH_CRS2S);
> + m_rs1 = decode_register_index_short (ival, OP_SH_CRS1S);
> + m_imm.s = EXTRACT_CLTYPE_IMM (ival);
> + }
> +
> /* Helper for DECODE, decode 32-bit S-type instruction. */
> void decode_s_type_insn (enum opcode opcode, ULONGEST ival)
> {
> @@ -1715,6 +1726,10 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> decode_r_type_insn (SC, ival);
> else if (is_ecall_insn (ival))
> decode_i_type_insn (ECALL, ival);
> + else if (is_ld_insn (ival))
> + decode_i_type_insn (LD, ival);
> + else if (is_lw_insn (ival))
> + decode_i_type_insn (LW, ival);
> else
> /* None of the other fields are valid in this case. */
> m_opcode = OTHER;
> @@ -1783,6 +1798,10 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> decode_cb_type_insn (BEQ, ival);
> else if (is_c_bnez_insn (ival))
> decode_cb_type_insn (BNE, ival);
> + else if (is_c_ld_insn (ival))
> + decode_cl_type_insn (LD, ival);
> + else if (is_c_lw_insn (ival))
> + decode_cl_type_insn (LW, ival);
> else
> /* None of the other fields of INSN are valid in this case. */
> m_opcode = OTHER;
> @@ -1931,6 +1950,20 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
> gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
> }
> + else if (insn.opcode () == riscv_insn::LD
> + || insn.opcode () == riscv_insn::LW)
> + {
> + /* Handle: ld reg, offset(rs1)
> + or: c.ld reg, offset(rs1)
> + or: lw reg, offset(rs1)
> + or: c.lw reg, offset(rs1) */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()]
> + = stack.fetch (pv_add_constant (regs[insn.rs1 ()],
> + insn.imm_signed ()),
> + (insn.opcode () == riscv_insn::LW ? 4 : 8));
> + }
> else
> {
> end_prologue_addr = cur_pc;
> diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw-foo.s b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw-foo.s
> new file mode 100644
> index 00000000000..ebc27ff1e8c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw-foo.s
> @@ -0,0 +1,74 @@
> +/* Copyright 2021 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* This testcase contains a function where the 'ld', 'c.ld', 'lw' or 'c.lw'
> + instruction is used in the prologue before the RA register have been saved
> + on the stack.
> +
> + This mimics a pattern observed in the __pthread_clockjoin_ex function
> + in libpthread.so.0 (from glibc-2.33-0ubuntu5) where a canary value is
> + loaded and placed on the stack in order to detect stack smashing.
> +
> + The skeleton for this file was generated using the following command:
> +
> + gcc -x c -S -c -o - - <<EOT
> + static long int __canary = 42;
> + extern int bar ();
> + int foo () { return bar(); }
> + EOT
> +
> + The result of this command is modified in the following way:
> + - The prologue is adapted to reserve 16 more bytes on the stack.
> + - A part that simulates the installation of a canary on the stack is
> + added. The canary is loaded multiple times to simulate the use of
> + various instructions that could do the work (ld or c.ld for a 64 bit
> + canary, lw or c.lw for a 32 bit canary).
> + - The epilogue is adjusted to be able to return properly. The epilogue
> + does not check the canary value since this testcase is only interested
> + in ensuring GDB can scan the prologue. */
> +
> + .option pic
> + .text
> + .data
> + .align 3
> + .type __canary, @object
> + .size __canary, 8
> +__canary:
> + .dword 42
> + .text
> + .align 1
> + .globl foo
> + .type foo, @function
> +foo:
> + addi sp,sp,-32
> + lla a5,__canary # Load the fake canary address.
> + lw t4,0(a5) # Load a 32 bit canary (use t4 to force the use of
> + # the non compressed instruction).
> + ld t4,0(a5) # Load a 64 bit canary (use t4 to force the use of
> + # the non compressed instruction).
> + c.lw a4,0(a5) # Load a 32 bit canary using the compressed insn.
> + c.ld a4,0(a5) # Load a 64 bit canary using the compressed insn.
> + sd a4,0(sp) # Place the fake canary on the stack.
> + sd ra,16(sp)
> + sd s0,8(sp)
> + addi s0,sp,32
> + call bar@plt
> + mv a5,a0
> + mv a0,a5
> + ld ra,16(sp)
> + ld s0,8(sp)
> + addi sp,sp,32
> + jr ra
> + .size foo, .-foo
> diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw.exp b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw.exp
> new file mode 100644
> index 00000000000..c329d2f72e0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld-lw.exp
> @@ -0,0 +1,45 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This tests GDB's ability to use the RISC-V prologue scanner in order to
> +# unwind through a function that uses the 'ld' instruction in its prologue.
> +
> +if {![istarget "riscv64-*-*"]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +standard_testfile riscv64-unwind-prologue-with-ld.c \
> + riscv64-unwind-prologue-with-ld-lw-foo.s
> +if {[prepare_for_testing "failed to prepare" $testfile \
> + "$srcfile $srcfile2" nodebug]} {
> + return -1
> +}
> +
> +if ![runto_main] then {
> + fail "can't run to main"
> + return 0
> +}
> +
> +gdb_breakpoint "bar"
> +gdb_continue_to_breakpoint "bar"
> +gdb_test "bt" \
> + [multi_line \
> + "#0\[ \t\]*$hex in bar \\\(\\\)" \
> + "#1\[ \t\]*$hex in foo \\\(\\\)" \
> + "#2\[ \t\]*$hex in main \\\(\\\)"] \
> + "Backtrace to the main frame"
> +gdb_test "finish" "foo \\\(\\\)" "finish bar"
> +gdb_test "finish" "main \\\(\\\)" "finish foo"
> diff --git a/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld.c b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld.c
> new file mode 100644
> index 00000000000..9ff950df273
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv64-unwind-prologue-with-ld.c
> @@ -0,0 +1,30 @@
> +/* Copyright 2021 Free Software Foundation, Inc.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +/* See riscv64-unwind-prologue-with-ld-foo.s for implementation. */
> +extern int foo (void);
> +
> +int
> +bar ()
> +{
> + return 0;
> +}
> +
> +int
> +main ()
> +{
> + return foo ();
> +}
> +
> --
> 2.30.2
>
More information about the Gdb-patches
mailing list