* [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
@ 2018-09-19 16:41 Andrew Burgess
2018-09-21 16:20 ` Palmer Dabbelt
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Andrew Burgess @ 2018-09-19 16:41 UTC (permalink / raw)
To: gdb-patches; +Cc: jimw, palmer, Andrew Burgess
This commit improves the prologue scanning stack unwinder, to better
support AUIPC, LUI, and more variants of ADD and ADDI.
This allows unwinding over frames containing large local variables,
where the frame size does not fit into a single instruction immediate,
and is first loaded into a temporary register, before being added to
the stack pointer.
A new test is added that tests this behaviour. As there's nothing
truely RiscV specific about this test I've added it into gdb.base, but
as this depends on target specific code to perform the unwind it is
possible that some targets might fail this new test.
gdb/ChangeLog:
* riscv-tdep.c (riscv_insn::decode): Decode c.lui.
(riscv_scan_prologue): Split handling of AUIPC, LUI, ADD, ADDI,
and NOP.
gdb/testsuite/ChangeLog:
* gdb.base/large-frame-1.c: New file.
* gdb.base/large-frame-2.c: New file.
* gdb.base/large-frame.exp: New file.
* gdb.base/large-frame.h: New file.
---
gdb/ChangeLog | 6 ++++
gdb/riscv-tdep.c | 59 ++++++++++++++++++++++------------
gdb/testsuite/ChangeLog | 7 ++++
gdb/testsuite/gdb.base/large-frame-1.c | 32 ++++++++++++++++++
gdb/testsuite/gdb.base/large-frame-2.c | 25 ++++++++++++++
gdb/testsuite/gdb.base/large-frame.exp | 57 ++++++++++++++++++++++++++++++++
gdb/testsuite/gdb.base/large-frame.h | 24 ++++++++++++++
7 files changed, 189 insertions(+), 21 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/large-frame-1.c
create mode 100644 gdb/testsuite/gdb.base/large-frame-2.c
create mode 100644 gdb/testsuite/gdb.base/large-frame.exp
create mode 100644 gdb/testsuite/gdb.base/large-frame.h
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 254914c9c7e..163c8ec231d 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1227,7 +1227,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);
}
else if (is_c_lui_insn (ival))
- m_opcode = OTHER;
+ {
+ m_opcode = LUI;
+ m_rd = decode_register_index (ival, OP_SH_CRS1S);
+ m_imm.s = EXTRACT_RVC_LUI_IMM (ival);
+ }
/* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
and C_FSW is RV32 only. */
else if (xlen != 4 && is_c_sd_insn (ival))
@@ -1359,28 +1363,41 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
regs[insn.rd ()] = pv_add_constant (regs[insn.rs1 ()], 0);
}
- else if ((insn.rd () == RISCV_GP_REGNUM
- && (insn.opcode () == riscv_insn::AUIPC
- || insn.opcode () == riscv_insn::LUI
- || (insn.opcode () == riscv_insn::ADDI
- && insn.rs1 () == RISCV_GP_REGNUM)
- || (insn.opcode () == riscv_insn::ADD
- && (insn.rs1 () == RISCV_GP_REGNUM
- || insn.rs2 () == RISCV_GP_REGNUM))))
- || (insn.opcode () == riscv_insn::ADDI
- && insn.rd () == RISCV_ZERO_REGNUM
- && insn.rs1 () == RISCV_ZERO_REGNUM
- && insn.imm_signed () == 0))
+ else if ((insn.opcode () == riscv_insn::ADDI
+ && insn.rd () == RISCV_ZERO_REGNUM
+ && insn.rs1 () == RISCV_ZERO_REGNUM
+ && insn.imm_signed () == 0))
{
- /* Handle: auipc gp, n
- or: addi gp, gp, n
- or: add gp, gp, reg
- or: add gp, reg, gp
- or: lui gp, n
- or: add x0, x0, 0 (NOP) */
- /* These instructions are part of the prologue, but we don't need
- to do anything special to handle them. */
+ /* Handle: add x0, x0, 0 (NOP) */
}
+ else if (insn.opcode () == riscv_insn::AUIPC)
+ {
+ gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
+ regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
+ }
+ else if (insn.opcode () == riscv_insn::LUI)
+ {
+ /* Handle: lui REG, n
+ Where REG is not gp register. */
+ gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
+ regs[insn.rd ()] = pv_constant (insn.imm_signed ());
+ }
+ else if (insn.opcode () == riscv_insn::ADDI)
+ {
+ /* Handle: addi REG1, REG2, IMM */
+ gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
+ gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
+ regs[insn.rd ()]
+ = pv_add_constant (regs[insn.rs1 ()], insn.imm_signed ());
+ }
+ else if (insn.opcode () == riscv_insn::ADD)
+ {
+ /* Handle: addi REG1, REG2, IMM */
+ gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
+ gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
+ gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
+ regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
+ }
else
{
end_prologue_addr = cur_pc;
diff --git a/gdb/testsuite/gdb.base/large-frame-1.c b/gdb/testsuite/gdb.base/large-frame-1.c
new file mode 100644
index 00000000000..cd9c5ccdd0a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/large-frame-1.c
@@ -0,0 +1,32 @@
+/* This file is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+#include "large-frame.h"
+
+__attribute__ ((noinline)) void
+blah (int *a)
+{
+ asm ("" :: "m" (a) : "memory");
+}
+
+int
+main ()
+{
+ func ();
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/large-frame-2.c b/gdb/testsuite/gdb.base/large-frame-2.c
new file mode 100644
index 00000000000..2491c347292
--- /dev/null
+++ b/gdb/testsuite/gdb.base/large-frame-2.c
@@ -0,0 +1,25 @@
+/* This file is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+#include "large-frame.h"
+
+__attribute__ ((noinline)) int
+func (void)
+{
+ int a[4096];
+ blah (a);
+}
diff --git a/gdb/testsuite/gdb.base/large-frame.exp b/gdb/testsuite/gdb.base/large-frame.exp
new file mode 100644
index 00000000000..00090da795e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/large-frame.exp
@@ -0,0 +1,57 @@
+# Copyright 2018 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 file is part of the gdb testsuite.
+
+# This test was added to test GDB's ability to backtrace over a large
+# stack frame for which there is no debug information. This should
+# test the non-DWARF stack unwinder.
+#
+# The test was originally added for Risc-V where this case caused a
+# problem at one point, however, there's nothing Risc-V specific in
+# the test.
+
+proc run_test { opt_level } {
+ global srcfile srcfile2 binfile hex
+
+ standard_testfile large-frame-1.c large-frame-2.c
+
+ if {[prepare_for_testing_full "failed to prepare" \
+ [list ${binfile}-${opt_level} debug \
+ $srcfile [list debug] \
+ $srcfile2 [list nodebug optimize=-$opt_level]]]} {
+ return
+ }
+
+ if { ![runto_main] } then {
+ unsupported "runto main"
+ return
+ }
+
+ gdb_breakpoint "blah"
+ gdb_continue_to_breakpoint "blah"
+
+ gdb_test "backtrace" [multi_line \
+ "#0 blah \[^\n\r\]+" \
+ "#1 $hex in func \[^\n\r\]+" \
+ "#2 $hex in main \[^\n\r\]+"]
+}
+
+foreach opt { O0 O1 O2 } {
+ with_test_prefix "optimize=-$opt" {
+ run_test $opt
+ }
+}
+
diff --git a/gdb/testsuite/gdb.base/large-frame.h b/gdb/testsuite/gdb.base/large-frame.h
new file mode 100644
index 00000000000..17058a5efd2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/large-frame.h
@@ -0,0 +1,24 @@
+/* This file is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+#ifndef LARGE_FRAME_H
+#define LARGE_FRAME_H
+
+extern void blah (int *);
+extern int func (void);
+
+#endif /* LARGE_FRAME_H */
--
2.14.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-19 16:41 [PATCH] gdb/riscv: Improve non-dwarf stack unwinding Andrew Burgess
@ 2018-09-21 16:20 ` Palmer Dabbelt
2018-09-26 13:13 ` PING: " Andrew Burgess
2018-09-26 13:49 ` Joel Brobecker
2 siblings, 0 replies; 8+ messages in thread
From: Palmer Dabbelt @ 2018-09-21 16:20 UTC (permalink / raw)
To: andrew.burgess; +Cc: gdb-patches, Jim Wilson, andrew.burgess
On Wed, 19 Sep 2018 09:41:38 PDT (-0700), andrew.burgess@embecosm.com wrote:
> This commit improves the prologue scanning stack unwinder, to better
> support AUIPC, LUI, and more variants of ADD and ADDI.
>
> This allows unwinding over frames containing large local variables,
> where the frame size does not fit into a single instruction immediate,
> and is first loaded into a temporary register, before being added to
> the stack pointer.
>
> A new test is added that tests this behaviour. As there's nothing
> truely RiscV specific about this test I've added it into gdb.base, but
> as this depends on target specific code to perform the unwind it is
> possible that some targets might fail this new test.
>
> gdb/ChangeLog:
>
> * riscv-tdep.c (riscv_insn::decode): Decode c.lui.
> (riscv_scan_prologue): Split handling of AUIPC, LUI, ADD, ADDI,
> and NOP.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/large-frame-1.c: New file.
> * gdb.base/large-frame-2.c: New file.
> * gdb.base/large-frame.exp: New file.
> * gdb.base/large-frame.h: New file.
Looks good to me, thanks!
> ---
> gdb/ChangeLog | 6 ++++
> gdb/riscv-tdep.c | 59 ++++++++++++++++++++++------------
> gdb/testsuite/ChangeLog | 7 ++++
> gdb/testsuite/gdb.base/large-frame-1.c | 32 ++++++++++++++++++
> gdb/testsuite/gdb.base/large-frame-2.c | 25 ++++++++++++++
> gdb/testsuite/gdb.base/large-frame.exp | 57 ++++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/large-frame.h | 24 ++++++++++++++
> 7 files changed, 189 insertions(+), 21 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/large-frame-1.c
> create mode 100644 gdb/testsuite/gdb.base/large-frame-2.c
> create mode 100644 gdb/testsuite/gdb.base/large-frame.exp
> create mode 100644 gdb/testsuite/gdb.base/large-frame.h
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 254914c9c7e..163c8ec231d 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1227,7 +1227,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);
> }
> else if (is_c_lui_insn (ival))
> - m_opcode = OTHER;
> + {
> + m_opcode = LUI;
> + m_rd = decode_register_index (ival, OP_SH_CRS1S);
> + m_imm.s = EXTRACT_RVC_LUI_IMM (ival);
> + }
> /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
> and C_FSW is RV32 only. */
> else if (xlen != 4 && is_c_sd_insn (ival))
> @@ -1359,28 +1363,41 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
> gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> regs[insn.rd ()] = pv_add_constant (regs[insn.rs1 ()], 0);
> }
> - else if ((insn.rd () == RISCV_GP_REGNUM
> - && (insn.opcode () == riscv_insn::AUIPC
> - || insn.opcode () == riscv_insn::LUI
> - || (insn.opcode () == riscv_insn::ADDI
> - && insn.rs1 () == RISCV_GP_REGNUM)
> - || (insn.opcode () == riscv_insn::ADD
> - && (insn.rs1 () == RISCV_GP_REGNUM
> - || insn.rs2 () == RISCV_GP_REGNUM))))
> - || (insn.opcode () == riscv_insn::ADDI
> - && insn.rd () == RISCV_ZERO_REGNUM
> - && insn.rs1 () == RISCV_ZERO_REGNUM
> - && insn.imm_signed () == 0))
> + else if ((insn.opcode () == riscv_insn::ADDI
> + && insn.rd () == RISCV_ZERO_REGNUM
> + && insn.rs1 () == RISCV_ZERO_REGNUM
> + && insn.imm_signed () == 0))
> {
> - /* Handle: auipc gp, n
> - or: addi gp, gp, n
> - or: add gp, gp, reg
> - or: add gp, reg, gp
> - or: lui gp, n
> - or: add x0, x0, 0 (NOP) */
> - /* These instructions are part of the prologue, but we don't need
> - to do anything special to handle them. */
> + /* Handle: add x0, x0, 0 (NOP) */
> }
> + else if (insn.opcode () == riscv_insn::AUIPC)
> + {
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::LUI)
> + {
> + /* Handle: lui REG, n
> + Where REG is not gp register. */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_constant (insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::ADDI)
> + {
> + /* Handle: addi REG1, REG2, IMM */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()]
> + = pv_add_constant (regs[insn.rs1 ()], insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::ADD)
> + {
> + /* Handle: addi REG1, REG2, IMM */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
> + }
> else
> {
> end_prologue_addr = cur_pc;
> diff --git a/gdb/testsuite/gdb.base/large-frame-1.c b/gdb/testsuite/gdb.base/large-frame-1.c
> new file mode 100644
> index 00000000000..cd9c5ccdd0a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame-1.c
> @@ -0,0 +1,32 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#include "large-frame.h"
> +
> +__attribute__ ((noinline)) void
> +blah (int *a)
> +{
> + asm ("" :: "m" (a) : "memory");
> +}
> +
> +int
> +main ()
> +{
> + func ();
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/large-frame-2.c b/gdb/testsuite/gdb.base/large-frame-2.c
> new file mode 100644
> index 00000000000..2491c347292
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame-2.c
> @@ -0,0 +1,25 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#include "large-frame.h"
> +
> +__attribute__ ((noinline)) int
> +func (void)
> +{
> + int a[4096];
> + blah (a);
> +}
> diff --git a/gdb/testsuite/gdb.base/large-frame.exp b/gdb/testsuite/gdb.base/large-frame.exp
> new file mode 100644
> index 00000000000..00090da795e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame.exp
> @@ -0,0 +1,57 @@
> +# Copyright 2018 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 file is part of the gdb testsuite.
> +
> +# This test was added to test GDB's ability to backtrace over a large
> +# stack frame for which there is no debug information. This should
> +# test the non-DWARF stack unwinder.
> +#
> +# The test was originally added for Risc-V where this case caused a
> +# problem at one point, however, there's nothing Risc-V specific in
> +# the test.
> +
> +proc run_test { opt_level } {
> + global srcfile srcfile2 binfile hex
> +
> + standard_testfile large-frame-1.c large-frame-2.c
> +
> + if {[prepare_for_testing_full "failed to prepare" \
> + [list ${binfile}-${opt_level} debug \
> + $srcfile [list debug] \
> + $srcfile2 [list nodebug optimize=-$opt_level]]]} {
> + return
> + }
> +
> + if { ![runto_main] } then {
> + unsupported "runto main"
> + return
> + }
> +
> + gdb_breakpoint "blah"
> + gdb_continue_to_breakpoint "blah"
> +
> + gdb_test "backtrace" [multi_line \
> + "#0 blah \[^\n\r\]+" \
> + "#1 $hex in func \[^\n\r\]+" \
> + "#2 $hex in main \[^\n\r\]+"]
> +}
> +
> +foreach opt { O0 O1 O2 } {
> + with_test_prefix "optimize=-$opt" {
> + run_test $opt
> + }
> +}
> +
> diff --git a/gdb/testsuite/gdb.base/large-frame.h b/gdb/testsuite/gdb.base/large-frame.h
> new file mode 100644
> index 00000000000..17058a5efd2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame.h
> @@ -0,0 +1,24 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#ifndef LARGE_FRAME_H
> +#define LARGE_FRAME_H
> +
> +extern void blah (int *);
> +extern int func (void);
> +
> +#endif /* LARGE_FRAME_H */
^ permalink raw reply [flat|nested] 8+ messages in thread
* PING: Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-19 16:41 [PATCH] gdb/riscv: Improve non-dwarf stack unwinding Andrew Burgess
2018-09-21 16:20 ` Palmer Dabbelt
@ 2018-09-26 13:13 ` Andrew Burgess
2018-09-26 13:49 ` Joel Brobecker
2 siblings, 0 replies; 8+ messages in thread
From: Andrew Burgess @ 2018-09-26 13:13 UTC (permalink / raw)
To: gdb-patches; +Cc: jimw, palmer
Ping!
Please could a global maintainer OK the testsuite changes.
Thanks,
Andrew
* Andrew Burgess <andrew.burgess@embecosm.com> [2018-09-19 17:41:38 +0100]:
> This commit improves the prologue scanning stack unwinder, to better
> support AUIPC, LUI, and more variants of ADD and ADDI.
>
> This allows unwinding over frames containing large local variables,
> where the frame size does not fit into a single instruction immediate,
> and is first loaded into a temporary register, before being added to
> the stack pointer.
>
> A new test is added that tests this behaviour. As there's nothing
> truely RiscV specific about this test I've added it into gdb.base, but
> as this depends on target specific code to perform the unwind it is
> possible that some targets might fail this new test.
>
> gdb/ChangeLog:
>
> * riscv-tdep.c (riscv_insn::decode): Decode c.lui.
> (riscv_scan_prologue): Split handling of AUIPC, LUI, ADD, ADDI,
> and NOP.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/large-frame-1.c: New file.
> * gdb.base/large-frame-2.c: New file.
> * gdb.base/large-frame.exp: New file.
> * gdb.base/large-frame.h: New file.
> ---
> gdb/ChangeLog | 6 ++++
> gdb/riscv-tdep.c | 59 ++++++++++++++++++++++------------
> gdb/testsuite/ChangeLog | 7 ++++
> gdb/testsuite/gdb.base/large-frame-1.c | 32 ++++++++++++++++++
> gdb/testsuite/gdb.base/large-frame-2.c | 25 ++++++++++++++
> gdb/testsuite/gdb.base/large-frame.exp | 57 ++++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/large-frame.h | 24 ++++++++++++++
> 7 files changed, 189 insertions(+), 21 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/large-frame-1.c
> create mode 100644 gdb/testsuite/gdb.base/large-frame-2.c
> create mode 100644 gdb/testsuite/gdb.base/large-frame.exp
> create mode 100644 gdb/testsuite/gdb.base/large-frame.h
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 254914c9c7e..163c8ec231d 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1227,7 +1227,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);
> }
> else if (is_c_lui_insn (ival))
> - m_opcode = OTHER;
> + {
> + m_opcode = LUI;
> + m_rd = decode_register_index (ival, OP_SH_CRS1S);
> + m_imm.s = EXTRACT_RVC_LUI_IMM (ival);
> + }
> /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
> and C_FSW is RV32 only. */
> else if (xlen != 4 && is_c_sd_insn (ival))
> @@ -1359,28 +1363,41 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
> gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> regs[insn.rd ()] = pv_add_constant (regs[insn.rs1 ()], 0);
> }
> - else if ((insn.rd () == RISCV_GP_REGNUM
> - && (insn.opcode () == riscv_insn::AUIPC
> - || insn.opcode () == riscv_insn::LUI
> - || (insn.opcode () == riscv_insn::ADDI
> - && insn.rs1 () == RISCV_GP_REGNUM)
> - || (insn.opcode () == riscv_insn::ADD
> - && (insn.rs1 () == RISCV_GP_REGNUM
> - || insn.rs2 () == RISCV_GP_REGNUM))))
> - || (insn.opcode () == riscv_insn::ADDI
> - && insn.rd () == RISCV_ZERO_REGNUM
> - && insn.rs1 () == RISCV_ZERO_REGNUM
> - && insn.imm_signed () == 0))
> + else if ((insn.opcode () == riscv_insn::ADDI
> + && insn.rd () == RISCV_ZERO_REGNUM
> + && insn.rs1 () == RISCV_ZERO_REGNUM
> + && insn.imm_signed () == 0))
> {
> - /* Handle: auipc gp, n
> - or: addi gp, gp, n
> - or: add gp, gp, reg
> - or: add gp, reg, gp
> - or: lui gp, n
> - or: add x0, x0, 0 (NOP) */
> - /* These instructions are part of the prologue, but we don't need
> - to do anything special to handle them. */
> + /* Handle: add x0, x0, 0 (NOP) */
> }
> + else if (insn.opcode () == riscv_insn::AUIPC)
> + {
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::LUI)
> + {
> + /* Handle: lui REG, n
> + Where REG is not gp register. */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_constant (insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::ADDI)
> + {
> + /* Handle: addi REG1, REG2, IMM */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()]
> + = pv_add_constant (regs[insn.rs1 ()], insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::ADD)
> + {
> + /* Handle: addi REG1, REG2, IMM */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
> + }
> else
> {
> end_prologue_addr = cur_pc;
> diff --git a/gdb/testsuite/gdb.base/large-frame-1.c b/gdb/testsuite/gdb.base/large-frame-1.c
> new file mode 100644
> index 00000000000..cd9c5ccdd0a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame-1.c
> @@ -0,0 +1,32 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#include "large-frame.h"
> +
> +__attribute__ ((noinline)) void
> +blah (int *a)
> +{
> + asm ("" :: "m" (a) : "memory");
> +}
> +
> +int
> +main ()
> +{
> + func ();
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/large-frame-2.c b/gdb/testsuite/gdb.base/large-frame-2.c
> new file mode 100644
> index 00000000000..2491c347292
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame-2.c
> @@ -0,0 +1,25 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#include "large-frame.h"
> +
> +__attribute__ ((noinline)) int
> +func (void)
> +{
> + int a[4096];
> + blah (a);
> +}
> diff --git a/gdb/testsuite/gdb.base/large-frame.exp b/gdb/testsuite/gdb.base/large-frame.exp
> new file mode 100644
> index 00000000000..00090da795e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame.exp
> @@ -0,0 +1,57 @@
> +# Copyright 2018 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 file is part of the gdb testsuite.
> +
> +# This test was added to test GDB's ability to backtrace over a large
> +# stack frame for which there is no debug information. This should
> +# test the non-DWARF stack unwinder.
> +#
> +# The test was originally added for Risc-V where this case caused a
> +# problem at one point, however, there's nothing Risc-V specific in
> +# the test.
> +
> +proc run_test { opt_level } {
> + global srcfile srcfile2 binfile hex
> +
> + standard_testfile large-frame-1.c large-frame-2.c
> +
> + if {[prepare_for_testing_full "failed to prepare" \
> + [list ${binfile}-${opt_level} debug \
> + $srcfile [list debug] \
> + $srcfile2 [list nodebug optimize=-$opt_level]]]} {
> + return
> + }
> +
> + if { ![runto_main] } then {
> + unsupported "runto main"
> + return
> + }
> +
> + gdb_breakpoint "blah"
> + gdb_continue_to_breakpoint "blah"
> +
> + gdb_test "backtrace" [multi_line \
> + "#0 blah \[^\n\r\]+" \
> + "#1 $hex in func \[^\n\r\]+" \
> + "#2 $hex in main \[^\n\r\]+"]
> +}
> +
> +foreach opt { O0 O1 O2 } {
> + with_test_prefix "optimize=-$opt" {
> + run_test $opt
> + }
> +}
> +
> diff --git a/gdb/testsuite/gdb.base/large-frame.h b/gdb/testsuite/gdb.base/large-frame.h
> new file mode 100644
> index 00000000000..17058a5efd2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame.h
> @@ -0,0 +1,24 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#ifndef LARGE_FRAME_H
> +#define LARGE_FRAME_H
> +
> +extern void blah (int *);
> +extern int func (void);
> +
> +#endif /* LARGE_FRAME_H */
> --
> 2.14.4
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-19 16:41 [PATCH] gdb/riscv: Improve non-dwarf stack unwinding Andrew Burgess
2018-09-21 16:20 ` Palmer Dabbelt
2018-09-26 13:13 ` PING: " Andrew Burgess
@ 2018-09-26 13:49 ` Joel Brobecker
2018-09-26 15:11 ` Andrew Burgess
2 siblings, 1 reply; 8+ messages in thread
From: Joel Brobecker @ 2018-09-26 13:49 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches, jimw, palmer
Hi Andrew,
> This commit improves the prologue scanning stack unwinder, to better
> support AUIPC, LUI, and more variants of ADD and ADDI.
>
> This allows unwinding over frames containing large local variables,
> where the frame size does not fit into a single instruction immediate,
> and is first loaded into a temporary register, before being added to
> the stack pointer.
>
> A new test is added that tests this behaviour. As there's nothing
> truely RiscV specific about this test I've added it into gdb.base, but
> as this depends on target specific code to perform the unwind it is
> possible that some targets might fail this new test.
>
> gdb/ChangeLog:
>
> * riscv-tdep.c (riscv_insn::decode): Decode c.lui.
> (riscv_scan_prologue): Split handling of AUIPC, LUI, ADD, ADDI,
> and NOP.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/large-frame-1.c: New file.
> * gdb.base/large-frame-2.c: New file.
> * gdb.base/large-frame.exp: New file.
> * gdb.base/large-frame.h: New file.
I am OK with the patch.
Are you sure you want to assert on the register number? What if
the instruction was malformed and the register number ended up
being larger than RISCV_NUM_INTEGER_REGS; is that possible?
> ---
> gdb/ChangeLog | 6 ++++
> gdb/riscv-tdep.c | 59 ++++++++++++++++++++++------------
> gdb/testsuite/ChangeLog | 7 ++++
> gdb/testsuite/gdb.base/large-frame-1.c | 32 ++++++++++++++++++
> gdb/testsuite/gdb.base/large-frame-2.c | 25 ++++++++++++++
> gdb/testsuite/gdb.base/large-frame.exp | 57 ++++++++++++++++++++++++++++++++
> gdb/testsuite/gdb.base/large-frame.h | 24 ++++++++++++++
> 7 files changed, 189 insertions(+), 21 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/large-frame-1.c
> create mode 100644 gdb/testsuite/gdb.base/large-frame-2.c
> create mode 100644 gdb/testsuite/gdb.base/large-frame.exp
> create mode 100644 gdb/testsuite/gdb.base/large-frame.h
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 254914c9c7e..163c8ec231d 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1227,7 +1227,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);
> }
> else if (is_c_lui_insn (ival))
> - m_opcode = OTHER;
> + {
> + m_opcode = LUI;
> + m_rd = decode_register_index (ival, OP_SH_CRS1S);
> + m_imm.s = EXTRACT_RVC_LUI_IMM (ival);
> + }
> /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
> and C_FSW is RV32 only. */
> else if (xlen != 4 && is_c_sd_insn (ival))
> @@ -1359,28 +1363,41 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
> gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> regs[insn.rd ()] = pv_add_constant (regs[insn.rs1 ()], 0);
> }
> - else if ((insn.rd () == RISCV_GP_REGNUM
> - && (insn.opcode () == riscv_insn::AUIPC
> - || insn.opcode () == riscv_insn::LUI
> - || (insn.opcode () == riscv_insn::ADDI
> - && insn.rs1 () == RISCV_GP_REGNUM)
> - || (insn.opcode () == riscv_insn::ADD
> - && (insn.rs1 () == RISCV_GP_REGNUM
> - || insn.rs2 () == RISCV_GP_REGNUM))))
> - || (insn.opcode () == riscv_insn::ADDI
> - && insn.rd () == RISCV_ZERO_REGNUM
> - && insn.rs1 () == RISCV_ZERO_REGNUM
> - && insn.imm_signed () == 0))
> + else if ((insn.opcode () == riscv_insn::ADDI
> + && insn.rd () == RISCV_ZERO_REGNUM
> + && insn.rs1 () == RISCV_ZERO_REGNUM
> + && insn.imm_signed () == 0))
> {
> - /* Handle: auipc gp, n
> - or: addi gp, gp, n
> - or: add gp, gp, reg
> - or: add gp, reg, gp
> - or: lui gp, n
> - or: add x0, x0, 0 (NOP) */
> - /* These instructions are part of the prologue, but we don't need
> - to do anything special to handle them. */
> + /* Handle: add x0, x0, 0 (NOP) */
> }
> + else if (insn.opcode () == riscv_insn::AUIPC)
> + {
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::LUI)
> + {
> + /* Handle: lui REG, n
> + Where REG is not gp register. */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_constant (insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::ADDI)
> + {
> + /* Handle: addi REG1, REG2, IMM */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()]
> + = pv_add_constant (regs[insn.rs1 ()], insn.imm_signed ());
> + }
> + else if (insn.opcode () == riscv_insn::ADD)
> + {
> + /* Handle: addi REG1, REG2, IMM */
> + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> + gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> + regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
> + }
> else
> {
> end_prologue_addr = cur_pc;
> diff --git a/gdb/testsuite/gdb.base/large-frame-1.c b/gdb/testsuite/gdb.base/large-frame-1.c
> new file mode 100644
> index 00000000000..cd9c5ccdd0a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame-1.c
> @@ -0,0 +1,32 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#include "large-frame.h"
> +
> +__attribute__ ((noinline)) void
> +blah (int *a)
> +{
> + asm ("" :: "m" (a) : "memory");
> +}
> +
> +int
> +main ()
> +{
> + func ();
> +
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/large-frame-2.c b/gdb/testsuite/gdb.base/large-frame-2.c
> new file mode 100644
> index 00000000000..2491c347292
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame-2.c
> @@ -0,0 +1,25 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#include "large-frame.h"
> +
> +__attribute__ ((noinline)) int
> +func (void)
> +{
> + int a[4096];
> + blah (a);
> +}
> diff --git a/gdb/testsuite/gdb.base/large-frame.exp b/gdb/testsuite/gdb.base/large-frame.exp
> new file mode 100644
> index 00000000000..00090da795e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame.exp
> @@ -0,0 +1,57 @@
> +# Copyright 2018 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 file is part of the gdb testsuite.
> +
> +# This test was added to test GDB's ability to backtrace over a large
> +# stack frame for which there is no debug information. This should
> +# test the non-DWARF stack unwinder.
> +#
> +# The test was originally added for Risc-V where this case caused a
> +# problem at one point, however, there's nothing Risc-V specific in
> +# the test.
> +
> +proc run_test { opt_level } {
> + global srcfile srcfile2 binfile hex
> +
> + standard_testfile large-frame-1.c large-frame-2.c
> +
> + if {[prepare_for_testing_full "failed to prepare" \
> + [list ${binfile}-${opt_level} debug \
> + $srcfile [list debug] \
> + $srcfile2 [list nodebug optimize=-$opt_level]]]} {
> + return
> + }
> +
> + if { ![runto_main] } then {
> + unsupported "runto main"
> + return
> + }
> +
> + gdb_breakpoint "blah"
> + gdb_continue_to_breakpoint "blah"
> +
> + gdb_test "backtrace" [multi_line \
> + "#0 blah \[^\n\r\]+" \
> + "#1 $hex in func \[^\n\r\]+" \
> + "#2 $hex in main \[^\n\r\]+"]
> +}
> +
> +foreach opt { O0 O1 O2 } {
> + with_test_prefix "optimize=-$opt" {
> + run_test $opt
> + }
> +}
> +
> diff --git a/gdb/testsuite/gdb.base/large-frame.h b/gdb/testsuite/gdb.base/large-frame.h
> new file mode 100644
> index 00000000000..17058a5efd2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/large-frame.h
> @@ -0,0 +1,24 @@
> +/* This file is part of GDB, the GNU debugger.
> +
> + Copyright 2018 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/>. */
> +
> +#ifndef LARGE_FRAME_H
> +#define LARGE_FRAME_H
> +
> +extern void blah (int *);
> +extern int func (void);
> +
> +#endif /* LARGE_FRAME_H */
> --
> 2.14.4
--
Joel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-26 13:49 ` Joel Brobecker
@ 2018-09-26 15:11 ` Andrew Burgess
2018-09-26 16:53 ` Joel Brobecker
2018-09-26 17:13 ` Palmer Dabbelt
0 siblings, 2 replies; 8+ messages in thread
From: Andrew Burgess @ 2018-09-26 15:11 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches, jimw, palmer
* Joel Brobecker <brobecker@adacore.com> [2018-09-26 06:49:16 -0700]:
> Hi Andrew,
>
> > This commit improves the prologue scanning stack unwinder, to better
> > support AUIPC, LUI, and more variants of ADD and ADDI.
> >
> > This allows unwinding over frames containing large local variables,
> > where the frame size does not fit into a single instruction immediate,
> > and is first loaded into a temporary register, before being added to
> > the stack pointer.
> >
> > A new test is added that tests this behaviour. As there's nothing
> > truely RiscV specific about this test I've added it into gdb.base, but
> > as this depends on target specific code to perform the unwind it is
> > possible that some targets might fail this new test.
> >
> > gdb/ChangeLog:
> >
> > * riscv-tdep.c (riscv_insn::decode): Decode c.lui.
> > (riscv_scan_prologue): Split handling of AUIPC, LUI, ADD, ADDI,
> > and NOP.
> >
> > gdb/testsuite/ChangeLog:
> >
> > * gdb.base/large-frame-1.c: New file.
> > * gdb.base/large-frame-2.c: New file.
> > * gdb.base/large-frame.exp: New file.
> > * gdb.base/large-frame.h: New file.
>
> I am OK with the patch.
>
> Are you sure you want to assert on the register number? What if
> the instruction was malformed and the register number ended up
> being larger than RISCV_NUM_INTEGER_REGS; is that possible?
Well, it shouldn't happen in the sense that, for a given instruction,
if it operates on integer registers then the space in the encoding
should only allow for up to RISCV_NUM_INTEGER_REGS registers (some
encodings allow for less registers).
Right now I can't imagine a situation where one of those asserts could
fire and it's not either a bug in GDB, or a bug in the libopcode field
extraction code - hence an assert rather than an error.
Thanks,
Andrew
>
>
> > ---
> > gdb/ChangeLog | 6 ++++
> > gdb/riscv-tdep.c | 59 ++++++++++++++++++++++------------
> > gdb/testsuite/ChangeLog | 7 ++++
> > gdb/testsuite/gdb.base/large-frame-1.c | 32 ++++++++++++++++++
> > gdb/testsuite/gdb.base/large-frame-2.c | 25 ++++++++++++++
> > gdb/testsuite/gdb.base/large-frame.exp | 57 ++++++++++++++++++++++++++++++++
> > gdb/testsuite/gdb.base/large-frame.h | 24 ++++++++++++++
> > 7 files changed, 189 insertions(+), 21 deletions(-)
> > create mode 100644 gdb/testsuite/gdb.base/large-frame-1.c
> > create mode 100644 gdb/testsuite/gdb.base/large-frame-2.c
> > create mode 100644 gdb/testsuite/gdb.base/large-frame.exp
> > create mode 100644 gdb/testsuite/gdb.base/large-frame.h
> >
> > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> > index 254914c9c7e..163c8ec231d 100644
> > --- a/gdb/riscv-tdep.c
> > +++ b/gdb/riscv-tdep.c
> > @@ -1227,7 +1227,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
> > m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);
> > }
> > else if (is_c_lui_insn (ival))
> > - m_opcode = OTHER;
> > + {
> > + m_opcode = LUI;
> > + m_rd = decode_register_index (ival, OP_SH_CRS1S);
> > + m_imm.s = EXTRACT_RVC_LUI_IMM (ival);
> > + }
> > /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
> > and C_FSW is RV32 only. */
> > else if (xlen != 4 && is_c_sd_insn (ival))
> > @@ -1359,28 +1363,41 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
> > gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> > regs[insn.rd ()] = pv_add_constant (regs[insn.rs1 ()], 0);
> > }
> > - else if ((insn.rd () == RISCV_GP_REGNUM
> > - && (insn.opcode () == riscv_insn::AUIPC
> > - || insn.opcode () == riscv_insn::LUI
> > - || (insn.opcode () == riscv_insn::ADDI
> > - && insn.rs1 () == RISCV_GP_REGNUM)
> > - || (insn.opcode () == riscv_insn::ADD
> > - && (insn.rs1 () == RISCV_GP_REGNUM
> > - || insn.rs2 () == RISCV_GP_REGNUM))))
> > - || (insn.opcode () == riscv_insn::ADDI
> > - && insn.rd () == RISCV_ZERO_REGNUM
> > - && insn.rs1 () == RISCV_ZERO_REGNUM
> > - && insn.imm_signed () == 0))
> > + else if ((insn.opcode () == riscv_insn::ADDI
> > + && insn.rd () == RISCV_ZERO_REGNUM
> > + && insn.rs1 () == RISCV_ZERO_REGNUM
> > + && insn.imm_signed () == 0))
> > {
> > - /* Handle: auipc gp, n
> > - or: addi gp, gp, n
> > - or: add gp, gp, reg
> > - or: add gp, reg, gp
> > - or: lui gp, n
> > - or: add x0, x0, 0 (NOP) */
> > - /* These instructions are part of the prologue, but we don't need
> > - to do anything special to handle them. */
> > + /* Handle: add x0, x0, 0 (NOP) */
> > }
> > + else if (insn.opcode () == riscv_insn::AUIPC)
> > + {
> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> > + regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
> > + }
> > + else if (insn.opcode () == riscv_insn::LUI)
> > + {
> > + /* Handle: lui REG, n
> > + Where REG is not gp register. */
> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> > + regs[insn.rd ()] = pv_constant (insn.imm_signed ());
> > + }
> > + else if (insn.opcode () == riscv_insn::ADDI)
> > + {
> > + /* Handle: addi REG1, REG2, IMM */
> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> > + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> > + regs[insn.rd ()]
> > + = pv_add_constant (regs[insn.rs1 ()], insn.imm_signed ());
> > + }
> > + else if (insn.opcode () == riscv_insn::ADD)
> > + {
> > + /* Handle: addi REG1, REG2, IMM */
> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
> > + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
> > + gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
> > + regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
> > + }
> > else
> > {
> > end_prologue_addr = cur_pc;
> > diff --git a/gdb/testsuite/gdb.base/large-frame-1.c b/gdb/testsuite/gdb.base/large-frame-1.c
> > new file mode 100644
> > index 00000000000..cd9c5ccdd0a
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/large-frame-1.c
> > @@ -0,0 +1,32 @@
> > +/* This file is part of GDB, the GNU debugger.
> > +
> > + Copyright 2018 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/>. */
> > +
> > +#include "large-frame.h"
> > +
> > +__attribute__ ((noinline)) void
> > +blah (int *a)
> > +{
> > + asm ("" :: "m" (a) : "memory");
> > +}
> > +
> > +int
> > +main ()
> > +{
> > + func ();
> > +
> > + return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.base/large-frame-2.c b/gdb/testsuite/gdb.base/large-frame-2.c
> > new file mode 100644
> > index 00000000000..2491c347292
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/large-frame-2.c
> > @@ -0,0 +1,25 @@
> > +/* This file is part of GDB, the GNU debugger.
> > +
> > + Copyright 2018 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/>. */
> > +
> > +#include "large-frame.h"
> > +
> > +__attribute__ ((noinline)) int
> > +func (void)
> > +{
> > + int a[4096];
> > + blah (a);
> > +}
> > diff --git a/gdb/testsuite/gdb.base/large-frame.exp b/gdb/testsuite/gdb.base/large-frame.exp
> > new file mode 100644
> > index 00000000000..00090da795e
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/large-frame.exp
> > @@ -0,0 +1,57 @@
> > +# Copyright 2018 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 file is part of the gdb testsuite.
> > +
> > +# This test was added to test GDB's ability to backtrace over a large
> > +# stack frame for which there is no debug information. This should
> > +# test the non-DWARF stack unwinder.
> > +#
> > +# The test was originally added for Risc-V where this case caused a
> > +# problem at one point, however, there's nothing Risc-V specific in
> > +# the test.
> > +
> > +proc run_test { opt_level } {
> > + global srcfile srcfile2 binfile hex
> > +
> > + standard_testfile large-frame-1.c large-frame-2.c
> > +
> > + if {[prepare_for_testing_full "failed to prepare" \
> > + [list ${binfile}-${opt_level} debug \
> > + $srcfile [list debug] \
> > + $srcfile2 [list nodebug optimize=-$opt_level]]]} {
> > + return
> > + }
> > +
> > + if { ![runto_main] } then {
> > + unsupported "runto main"
> > + return
> > + }
> > +
> > + gdb_breakpoint "blah"
> > + gdb_continue_to_breakpoint "blah"
> > +
> > + gdb_test "backtrace" [multi_line \
> > + "#0 blah \[^\n\r\]+" \
> > + "#1 $hex in func \[^\n\r\]+" \
> > + "#2 $hex in main \[^\n\r\]+"]
> > +}
> > +
> > +foreach opt { O0 O1 O2 } {
> > + with_test_prefix "optimize=-$opt" {
> > + run_test $opt
> > + }
> > +}
> > +
> > diff --git a/gdb/testsuite/gdb.base/large-frame.h b/gdb/testsuite/gdb.base/large-frame.h
> > new file mode 100644
> > index 00000000000..17058a5efd2
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/large-frame.h
> > @@ -0,0 +1,24 @@
> > +/* This file is part of GDB, the GNU debugger.
> > +
> > + Copyright 2018 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/>. */
> > +
> > +#ifndef LARGE_FRAME_H
> > +#define LARGE_FRAME_H
> > +
> > +extern void blah (int *);
> > +extern int func (void);
> > +
> > +#endif /* LARGE_FRAME_H */
> > --
> > 2.14.4
>
> --
> Joel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-26 15:11 ` Andrew Burgess
@ 2018-09-26 16:53 ` Joel Brobecker
2018-09-26 17:13 ` Palmer Dabbelt
1 sibling, 0 replies; 8+ messages in thread
From: Joel Brobecker @ 2018-09-26 16:53 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches, jimw, palmer
> > Are you sure you want to assert on the register number? What if
> > the instruction was malformed and the register number ended up
> > being larger than RISCV_NUM_INTEGER_REGS; is that possible?
>
> Well, it shouldn't happen in the sense that, for a given instruction,
> if it operates on integer registers then the space in the encoding
> should only allow for up to RISCV_NUM_INTEGER_REGS registers (some
> encodings allow for less registers).
>
> Right now I can't imagine a situation where one of those asserts could
> fire and it's not either a bug in GDB, or a bug in the libopcode field
> extraction code - hence an assert rather than an error.
OK, that's perfect. Thanks for the confirmation!
--
Joel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-26 15:11 ` Andrew Burgess
2018-09-26 16:53 ` Joel Brobecker
@ 2018-09-26 17:13 ` Palmer Dabbelt
2018-09-26 19:15 ` Joel Brobecker
1 sibling, 1 reply; 8+ messages in thread
From: Palmer Dabbelt @ 2018-09-26 17:13 UTC (permalink / raw)
To: andrew.burgess; +Cc: brobecker, gdb-patches, Jim Wilson
On Wed, 26 Sep 2018 08:11:12 PDT (-0700), andrew.burgess@embecosm.com wrote:
> * Joel Brobecker <brobecker@adacore.com> [2018-09-26 06:49:16 -0700]:
>
>> Hi Andrew,
>>
>> > This commit improves the prologue scanning stack unwinder, to better
>> > support AUIPC, LUI, and more variants of ADD and ADDI.
>> >
>> > This allows unwinding over frames containing large local variables,
>> > where the frame size does not fit into a single instruction immediate,
>> > and is first loaded into a temporary register, before being added to
>> > the stack pointer.
>> >
>> > A new test is added that tests this behaviour. As there's nothing
>> > truely RiscV specific about this test I've added it into gdb.base, but
>> > as this depends on target specific code to perform the unwind it is
>> > possible that some targets might fail this new test.
>> >
>> > gdb/ChangeLog:
>> >
>> > * riscv-tdep.c (riscv_insn::decode): Decode c.lui.
>> > (riscv_scan_prologue): Split handling of AUIPC, LUI, ADD, ADDI,
>> > and NOP.
>> >
>> > gdb/testsuite/ChangeLog:
>> >
>> > * gdb.base/large-frame-1.c: New file.
>> > * gdb.base/large-frame-2.c: New file.
>> > * gdb.base/large-frame.exp: New file.
>> > * gdb.base/large-frame.h: New file.
>>
>> I am OK with the patch.
>>
>> Are you sure you want to assert on the register number? What if
>> the instruction was malformed and the register number ended up
>> being larger than RISCV_NUM_INTEGER_REGS; is that possible?
>
> Well, it shouldn't happen in the sense that, for a given instruction,
> if it operates on integer registers then the space in the encoding
> should only allow for up to RISCV_NUM_INTEGER_REGS registers (some
> encodings allow for less registers).
>
> Right now I can't imagine a situation where one of those asserts could
> fire and it's not either a bug in GDB, or a bug in the libopcode field
> extraction code - hence an assert rather than an error.
While I agree this is true for I-based ISAs, I think this might be able to fire
for E-based ISAs because those can actually encode invalid register indices.
That said, these should be decoded as invalid instructions so I think we're
safe here. I'm OK either way (ie, abort or warn).
>
> Thanks,
> Andrew
>
>
>
>
>>
>>
>> > ---
>> > gdb/ChangeLog | 6 ++++
>> > gdb/riscv-tdep.c | 59 ++++++++++++++++++++++------------
>> > gdb/testsuite/ChangeLog | 7 ++++
>> > gdb/testsuite/gdb.base/large-frame-1.c | 32 ++++++++++++++++++
>> > gdb/testsuite/gdb.base/large-frame-2.c | 25 ++++++++++++++
>> > gdb/testsuite/gdb.base/large-frame.exp | 57 ++++++++++++++++++++++++++++++++
>> > gdb/testsuite/gdb.base/large-frame.h | 24 ++++++++++++++
>> > 7 files changed, 189 insertions(+), 21 deletions(-)
>> > create mode 100644 gdb/testsuite/gdb.base/large-frame-1.c
>> > create mode 100644 gdb/testsuite/gdb.base/large-frame-2.c
>> > create mode 100644 gdb/testsuite/gdb.base/large-frame.exp
>> > create mode 100644 gdb/testsuite/gdb.base/large-frame.h
>> >
>> > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
>> > index 254914c9c7e..163c8ec231d 100644
>> > --- a/gdb/riscv-tdep.c
>> > +++ b/gdb/riscv-tdep.c
>> > @@ -1227,7 +1227,11 @@ riscv_insn::decode (struct gdbarch *gdbarch, CORE_ADDR pc)
>> > m_imm.s = EXTRACT_RVC_ADDI4SPN_IMM (ival);
>> > }
>> > else if (is_c_lui_insn (ival))
>> > - m_opcode = OTHER;
>> > + {
>> > + m_opcode = LUI;
>> > + m_rd = decode_register_index (ival, OP_SH_CRS1S);
>> > + m_imm.s = EXTRACT_RVC_LUI_IMM (ival);
>> > + }
>> > /* C_SD and C_FSW have the same opcode. C_SD is RV64 and RV128 only,
>> > and C_FSW is RV32 only. */
>> > else if (xlen != 4 && is_c_sd_insn (ival))
>> > @@ -1359,28 +1363,41 @@ riscv_scan_prologue (struct gdbarch *gdbarch,
>> > gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
>> > regs[insn.rd ()] = pv_add_constant (regs[insn.rs1 ()], 0);
>> > }
>> > - else if ((insn.rd () == RISCV_GP_REGNUM
>> > - && (insn.opcode () == riscv_insn::AUIPC
>> > - || insn.opcode () == riscv_insn::LUI
>> > - || (insn.opcode () == riscv_insn::ADDI
>> > - && insn.rs1 () == RISCV_GP_REGNUM)
>> > - || (insn.opcode () == riscv_insn::ADD
>> > - && (insn.rs1 () == RISCV_GP_REGNUM
>> > - || insn.rs2 () == RISCV_GP_REGNUM))))
>> > - || (insn.opcode () == riscv_insn::ADDI
>> > - && insn.rd () == RISCV_ZERO_REGNUM
>> > - && insn.rs1 () == RISCV_ZERO_REGNUM
>> > - && insn.imm_signed () == 0))
>> > + else if ((insn.opcode () == riscv_insn::ADDI
>> > + && insn.rd () == RISCV_ZERO_REGNUM
>> > + && insn.rs1 () == RISCV_ZERO_REGNUM
>> > + && insn.imm_signed () == 0))
>> > {
>> > - /* Handle: auipc gp, n
>> > - or: addi gp, gp, n
>> > - or: add gp, gp, reg
>> > - or: add gp, reg, gp
>> > - or: lui gp, n
>> > - or: add x0, x0, 0 (NOP) */
>> > - /* These instructions are part of the prologue, but we don't need
>> > - to do anything special to handle them. */
>> > + /* Handle: add x0, x0, 0 (NOP) */
>> > }
>> > + else if (insn.opcode () == riscv_insn::AUIPC)
>> > + {
>> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
>> > + regs[insn.rd ()] = pv_constant (cur_pc + insn.imm_signed ());
>> > + }
>> > + else if (insn.opcode () == riscv_insn::LUI)
>> > + {
>> > + /* Handle: lui REG, n
>> > + Where REG is not gp register. */
>> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
>> > + regs[insn.rd ()] = pv_constant (insn.imm_signed ());
>> > + }
>> > + else if (insn.opcode () == riscv_insn::ADDI)
>> > + {
>> > + /* Handle: addi REG1, REG2, IMM */
>> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
>> > + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
>> > + regs[insn.rd ()]
>> > + = pv_add_constant (regs[insn.rs1 ()], insn.imm_signed ());
>> > + }
>> > + else if (insn.opcode () == riscv_insn::ADD)
>> > + {
>> > + /* Handle: addi REG1, REG2, IMM */
>> > + gdb_assert (insn.rd () < RISCV_NUM_INTEGER_REGS);
>> > + gdb_assert (insn.rs1 () < RISCV_NUM_INTEGER_REGS);
>> > + gdb_assert (insn.rs2 () < RISCV_NUM_INTEGER_REGS);
>> > + regs[insn.rd ()] = pv_add (regs[insn.rs1 ()], regs[insn.rs2 ()]);
>> > + }
>> > else
>> > {
>> > end_prologue_addr = cur_pc;
>> > diff --git a/gdb/testsuite/gdb.base/large-frame-1.c b/gdb/testsuite/gdb.base/large-frame-1.c
>> > new file mode 100644
>> > index 00000000000..cd9c5ccdd0a
>> > --- /dev/null
>> > +++ b/gdb/testsuite/gdb.base/large-frame-1.c
>> > @@ -0,0 +1,32 @@
>> > +/* This file is part of GDB, the GNU debugger.
>> > +
>> > + Copyright 2018 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/>. */
>> > +
>> > +#include "large-frame.h"
>> > +
>> > +__attribute__ ((noinline)) void
>> > +blah (int *a)
>> > +{
>> > + asm ("" :: "m" (a) : "memory");
>> > +}
>> > +
>> > +int
>> > +main ()
>> > +{
>> > + func ();
>> > +
>> > + return 0;
>> > +}
>> > diff --git a/gdb/testsuite/gdb.base/large-frame-2.c b/gdb/testsuite/gdb.base/large-frame-2.c
>> > new file mode 100644
>> > index 00000000000..2491c347292
>> > --- /dev/null
>> > +++ b/gdb/testsuite/gdb.base/large-frame-2.c
>> > @@ -0,0 +1,25 @@
>> > +/* This file is part of GDB, the GNU debugger.
>> > +
>> > + Copyright 2018 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/>. */
>> > +
>> > +#include "large-frame.h"
>> > +
>> > +__attribute__ ((noinline)) int
>> > +func (void)
>> > +{
>> > + int a[4096];
>> > + blah (a);
>> > +}
>> > diff --git a/gdb/testsuite/gdb.base/large-frame.exp b/gdb/testsuite/gdb.base/large-frame.exp
>> > new file mode 100644
>> > index 00000000000..00090da795e
>> > --- /dev/null
>> > +++ b/gdb/testsuite/gdb.base/large-frame.exp
>> > @@ -0,0 +1,57 @@
>> > +# Copyright 2018 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 file is part of the gdb testsuite.
>> > +
>> > +# This test was added to test GDB's ability to backtrace over a large
>> > +# stack frame for which there is no debug information. This should
>> > +# test the non-DWARF stack unwinder.
>> > +#
>> > +# The test was originally added for Risc-V where this case caused a
>> > +# problem at one point, however, there's nothing Risc-V specific in
>> > +# the test.
>> > +
>> > +proc run_test { opt_level } {
>> > + global srcfile srcfile2 binfile hex
>> > +
>> > + standard_testfile large-frame-1.c large-frame-2.c
>> > +
>> > + if {[prepare_for_testing_full "failed to prepare" \
>> > + [list ${binfile}-${opt_level} debug \
>> > + $srcfile [list debug] \
>> > + $srcfile2 [list nodebug optimize=-$opt_level]]]} {
>> > + return
>> > + }
>> > +
>> > + if { ![runto_main] } then {
>> > + unsupported "runto main"
>> > + return
>> > + }
>> > +
>> > + gdb_breakpoint "blah"
>> > + gdb_continue_to_breakpoint "blah"
>> > +
>> > + gdb_test "backtrace" [multi_line \
>> > + "#0 blah \[^\n\r\]+" \
>> > + "#1 $hex in func \[^\n\r\]+" \
>> > + "#2 $hex in main \[^\n\r\]+"]
>> > +}
>> > +
>> > +foreach opt { O0 O1 O2 } {
>> > + with_test_prefix "optimize=-$opt" {
>> > + run_test $opt
>> > + }
>> > +}
>> > +
>> > diff --git a/gdb/testsuite/gdb.base/large-frame.h b/gdb/testsuite/gdb.base/large-frame.h
>> > new file mode 100644
>> > index 00000000000..17058a5efd2
>> > --- /dev/null
>> > +++ b/gdb/testsuite/gdb.base/large-frame.h
>> > @@ -0,0 +1,24 @@
>> > +/* This file is part of GDB, the GNU debugger.
>> > +
>> > + Copyright 2018 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/>. */
>> > +
>> > +#ifndef LARGE_FRAME_H
>> > +#define LARGE_FRAME_H
>> > +
>> > +extern void blah (int *);
>> > +extern int func (void);
>> > +
>> > +#endif /* LARGE_FRAME_H */
>> > --
>> > 2.14.4
>>
>> --
>> Joel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gdb/riscv: Improve non-dwarf stack unwinding
2018-09-26 17:13 ` Palmer Dabbelt
@ 2018-09-26 19:15 ` Joel Brobecker
0 siblings, 0 replies; 8+ messages in thread
From: Joel Brobecker @ 2018-09-26 19:15 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: andrew.burgess, gdb-patches, Jim Wilson
> While I agree this is true for I-based ISAs, I think this might be able to
> fire for E-based ISAs because those can actually encode invalid register
> indices. That said, these should be decoded as invalid instructions so I
> think we're safe here. I'm OK either way (ie, abort or warn).
And FWIW, I agree that should the register number be invalid
in the instruction, the error should be reported during the decoding.
So the asserts here are good.
--
Joel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-09-26 19:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 16:41 [PATCH] gdb/riscv: Improve non-dwarf stack unwinding Andrew Burgess
2018-09-21 16:20 ` Palmer Dabbelt
2018-09-26 13:13 ` PING: " Andrew Burgess
2018-09-26 13:49 ` Joel Brobecker
2018-09-26 15:11 ` Andrew Burgess
2018-09-26 16:53 ` Joel Brobecker
2018-09-26 17:13 ` Palmer Dabbelt
2018-09-26 19:15 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox