* gdb: Implement the init_reg dwarf2_frame_ops for amd64
@ 2021-11-08 12:20 Djordje
2021-11-09 13:35 ` Andrew Burgess
0 siblings, 1 reply; 12+ messages in thread
From: Djordje @ 2021-11-08 12:20 UTC (permalink / raw)
To: gdb-patches; +Cc: asowda, nikola.tesic
Hi,
Please consider these two files:
$ cat test2.c
void f1(int);
int f2();
void f3();
void f() {
f1(0);
int local = f2();
f1(local);
f3();
}
$ cat main-for-core.c
#include <stdio.h>
void f1(int param) {
if (param % 2 == 0)
return;
printf("HELLO\n");
abort();
}
int f2() {
int x;
scanf("%d", &x);
return x;
}
void f3() {
int y;
scanf("%d", &y);
printf("%d\n", y);
}
int main() {
f();
return 0;
}
Compilation of the files:
$ clang -g -O2 test2.c -c -o object-file-test2.o
$ clang -g -O2 main-for-core.c object-file-test2.o -o main-for-core
By examining debug_loc from object-file-test2.o for the `local` var we see:
00000000 000000000000000f 0000000000000016 (DW_OP_reg0 (rax))
And the code for the f() is:
0000000000000000 <f>:
0: push %rax
1: xor %edi,%edi
3: callq 8 <f+0x8>
8: xor %eax,%eax
a: callq f <f+0xf>
f: mov %eax,%edi
11: callq 16 <f+0x16>
16: xor %eax,%eax
18: pop %rcx
19: jmpq 1e <f+0x1e>
While debugging it, by loading the core file generated due to abort() (as an input I typed 5):
$ gdb main-for-core
...
(gdb) bt
#0 0x00007fb2d7aeb377 in raise () from /lib64/libc.so.6
#1 0x00007fb2d7aeca68 in abort () from /lib64/libc.so.6
#2 0x0000000000401197 in f1 (param=<optimized out>) at main-for-core.c:7
#3 0x0000000000401216 in f () at test2.c:8
#4 0x00000000004011f8 in main () at main-for-core.c:23
(gdb) f 3
#3 0x0000000000401216 in f () at test2.c:8
8 f1(local);
(gdb) disassemble
Dump of assembler code for function f:
0x0000000000401200 <+0>: push %rax
0x0000000000401201 <+1>: xor %edi,%edi
0x0000000000401203 <+3>: callq 0x401180 <f1>
0x0000000000401208 <+8>: xor %eax,%eax
0x000000000040120a <+10>: callq 0x4011a0 <f2>
0x000000000040120f <+15>: mov %eax,%edi
0x0000000000401211 <+17>: callq 0x401180 <f1>
=> 0x0000000000401216 <+22>: xor %eax,%eax
0x0000000000401218 <+24>: pop %rcx
0x0000000000401219 <+25>: jmpq 0x4011c0 <f3>
End of assembler dump.
(gdb) p local
$1 = 0
But it should be reported as <optimized_out>, because the %eax was clobbered by the call.
GCC produces different high pc address to keep GDB working properly:
00000000 000000000000000f 0000000000000015 (DW_OP_reg0 (rax))
This patch fixes the problem inside GDB itself for X86_64 target.
Bug: https://bugs.llvm.org/show_bug.cgi?id=49641
---
gdb/amd64-tdep.c | 37 +++++++++++++
gdb/frame.c | 9 +++-
.../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
.../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
8 files changed, 136 insertions(+), 14 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 47761208088..a4ac36b18be 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,8 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* Implement the "init_reg" dwarf2_frame_ops method. */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
@@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
/* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
be in use on any of the supported AMD64 targets. */
diff --git a/gdb/frame.c b/gdb/frame.c
index 16673258373..a5b43c3f949 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2312,9 +2312,14 @@ get_prev_frame_always (struct frame_info *this_frame)
}
catch (const gdb_exception_error &ex)
{
- if (ex.error == MEMORY_ERROR)
+ if (ex.error == MEMORY_ERROR || ex.error == GENERIC_ERROR)
{
- this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ if (ex.error == MEMORY_ERROR)
+ this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ else
+ /* This is for the GENERIC_ERROR case. */
+ this_frame->stop_reason = UNWIND_UNAVAILABLE;
+
if (ex.message != NULL)
{
char *stop_string;
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index f9e590d83bb..6ae1c9c1322 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -42,11 +42,11 @@ if ![runto breakpt] {
return -1
}
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "first backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "first backtrace, with error message"
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "second backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "second backtrace, with error message"
clean_restart ${binfile}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
new file mode 100644
index 00000000000..5698e549f00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2019-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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
new file mode 100644
index 00000000000..07b870b5d30
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# Copyright 2010-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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64-*-* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
index 238830e711e..70f22cb9efe 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
@@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
# (2) struct_param_single_reg_loc
gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
-gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
+gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
# (3) struct_param_two_reg_pieces
gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
-gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
+gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
# (4) int_param_two_reg_pieces
gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index b1c28b2f41c..2b296c8d445 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
} else {
set pattern_rax_rbx_rcx_print "<not saved>"
set pattern_rax_rbx_rcx_info "<not saved>"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r8_r9_print "<not saved>"
+ set pattern_r8_r9_info "<not saved>"
}
# Select frame.
@@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
# "not saved" and not "optimized out".
gdb_test "set debug frame 1"
gdb_test {print $rax} [multi_line \
- { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
- { \[frame\] frame_unwind_register_value: -> <not saved>} \
+ { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
+ { \[frame\] frame_unwind_register_value: -> <not saved>} \
{.*}]
gdb_test "set debug frame 0"
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index f38e20003a3..3f193b1379b 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
}
mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
- "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
"register values, format x, frame ${f}"
mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
- "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
"register values, format r, frame ${f}"
}
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-11-08 12:20 gdb: Implement the init_reg dwarf2_frame_ops for amd64 Djordje
@ 2021-11-09 13:35 ` Andrew Burgess
2021-11-15 9:06 ` Djordje Todorovic
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Burgess @ 2021-11-09 13:35 UTC (permalink / raw)
To: Djordje, gdb-patches; +Cc: nikola.tesic, asowda
Djordje <djordje.todorovic@syrmia.com> writes:
> Hi,
>
> Please consider these two files:
>
> $ cat test2.c
> void f1(int);
> int f2();
> void f3();
>
> void f() {
> f1(0);
> int local = f2();
> f1(local);
> f3();
> }
>
> $ cat main-for-core.c
> #include <stdio.h>
>
> void f1(int param) {
> if (param % 2 == 0)
> return;
> printf("HELLO\n");
> abort();
> }
>
> int f2() {
> int x;
> scanf("%d", &x);
> return x;
> }
>
> void f3() {
> int y;
> scanf("%d", &y);
> printf("%d\n", y);
> }
>
> int main() {
> f();
> return 0;
> }
>
> Compilation of the files:
>
> $ clang -g -O2 test2.c -c -o object-file-test2.o
> $ clang -g -O2 main-for-core.c object-file-test2.o -o main-for-core
>
> By examining debug_loc from object-file-test2.o for the `local` var we see:
> 00000000 000000000000000f 0000000000000016 (DW_OP_reg0 (rax))
>
> And the code for the f() is:
> 0000000000000000 <f>:
> 0: push %rax
> 1: xor %edi,%edi
> 3: callq 8 <f+0x8>
> 8: xor %eax,%eax
> a: callq f <f+0xf>
> f: mov %eax,%edi
> 11: callq 16 <f+0x16>
> 16: xor %eax,%eax
> 18: pop %rcx
> 19: jmpq 1e <f+0x1e>
>
> While debugging it, by loading the core file generated due to abort() (as an input I typed 5):
>
> $ gdb main-for-core
> ...
> (gdb) bt
> #0 0x00007fb2d7aeb377 in raise () from /lib64/libc.so.6
> #1 0x00007fb2d7aeca68 in abort () from /lib64/libc.so.6
> #2 0x0000000000401197 in f1 (param=<optimized out>) at main-for-core.c:7
> #3 0x0000000000401216 in f () at test2.c:8
> #4 0x00000000004011f8 in main () at main-for-core.c:23
> (gdb) f 3
> #3 0x0000000000401216 in f () at test2.c:8
> 8 f1(local);
> (gdb) disassemble
> Dump of assembler code for function f:
> 0x0000000000401200 <+0>: push %rax
> 0x0000000000401201 <+1>: xor %edi,%edi
> 0x0000000000401203 <+3>: callq 0x401180 <f1>
> 0x0000000000401208 <+8>: xor %eax,%eax
> 0x000000000040120a <+10>: callq 0x4011a0 <f2>
> 0x000000000040120f <+15>: mov %eax,%edi
> 0x0000000000401211 <+17>: callq 0x401180 <f1>
> => 0x0000000000401216 <+22>: xor %eax,%eax
> 0x0000000000401218 <+24>: pop %rcx
> 0x0000000000401219 <+25>: jmpq 0x4011c0 <f3>
> End of assembler dump.
> (gdb) p local
> $1 = 0
>
> But it should be reported as <optimized_out>, because the %eax was clobbered by the call.
>
> GCC produces different high pc address to keep GDB working properly:
> 00000000 000000000000000f 0000000000000015 (DW_OP_reg0 (rax))
>
> This patch fixes the problem inside GDB itself for X86_64 target.
>
> Bug: https://bugs.llvm.org/show_bug.cgi?id=49641
> ---
> gdb/amd64-tdep.c | 37 +++++++++++++
> gdb/frame.c | 9 +++-
> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
> 8 files changed, 136 insertions(+), 14 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 47761208088..a4ac36b18be 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -25,6 +25,8 @@
> #include "arch-utils.h"
> #include "block.h"
> #include "dummy-frame.h"
> +#include "dwarf2.h"
> +#include "dwarf2/frame.h"
> #include "frame.h"
> #include "frame-base.h"
> #include "frame-unwind.h"
> @@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
> AMD64_RIP_REGNUM);
> }
>
> +/* Implement the "init_reg" dwarf2_frame_ops method. */
> +
> +static void
> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> + struct dwarf2_frame_state_reg *reg,
> + struct frame_info *this_frame)
> +{
> + switch (regnum)
> + {
> + case AMD64_RIP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_FN;
> + return;
> +
> + case AMD64_RSP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_CFA;
> + return;
> +
> + /* Caller-saved registers. */
> + case AMD64_RAX_REGNUM:
> + case AMD64_RDI_REGNUM:
> + case AMD64_RSI_REGNUM:
> + case AMD64_RDX_REGNUM:
> + case AMD64_RCX_REGNUM:
> + case AMD64_R8_REGNUM:
> + case AMD64_R9_REGNUM:
> + case AMD64_R10_REGNUM:
> + case AMD64_R11_REGNUM:
> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
> + return;
> + }
> +}
> +
> void
> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> const target_desc *default_tdesc)
> @@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>
> + /* Frame handling. */
> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
> +
> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
> be in use on any of the supported AMD64 targets. */
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 16673258373..a5b43c3f949 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2312,9 +2312,14 @@ get_prev_frame_always (struct frame_info *this_frame)
> }
> catch (const gdb_exception_error &ex)
> {
> - if (ex.error == MEMORY_ERROR)
> + if (ex.error == MEMORY_ERROR || ex.error == GENERIC_ERROR)
Thanks for working on this. I wasn't keen on this use of GENERIC_ERROR,
so I posted this patch:
https://sourceware.org/pipermail/gdb-patches/2021-November/183267.html
After that has been merged you can rebased this patch and this code
should be improved.
Thanks,
Andrew
> {
> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + if (ex.error == MEMORY_ERROR)
> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + else
> + /* This is for the GENERIC_ERROR case. */
> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
> +
> if (ex.message != NULL)
> {
> char *stop_string;
> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> index f9e590d83bb..6ae1c9c1322 100644
> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> @@ -42,11 +42,11 @@ if ![runto breakpt] {
> return -1
> }
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "first backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "first backtrace, with error message"
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "second backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "second backtrace, with error message"
>
> clean_restart ${binfile}
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> new file mode 100644
> index 00000000000..5698e549f00
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> @@ -0,0 +1,28 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2019-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/>. */
> +
> +int
> +foo (int a)
> +{
> + return a; /* break here. */
> +}
> +
> +int
> +main ()
> +{
> + return foo (5);
> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> new file mode 100644
> index 00000000000..07b870b5d30
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> @@ -0,0 +1,52 @@
> +# Copyright 2010-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/>.
> +
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
> +
> +# This file is part of the gdb testsuite.
> +
> +if { ![istarget x86_64-*-* ]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +# Build program with debug symbols.
> +standard_testfile
> +set compile_flags {debug}
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +gdb_breakpoint [ gdb_get_line_number "break here" ]
> +gdb_continue_to_breakpoint "break here" ".*break here.*"
> +
> +# Switch to frame 1.
> +gdb_test "frame 1" "#1.*in main ().*"
> +
> +gdb_test "info registers rax" "rax .* <not saved>"
> +gdb_test "info registers rcx" "rcx .* <not saved>"
> +gdb_test "info registers rdx" "rdx .* <not saved>"
> +gdb_test "info registers rsi" "rsi .* <not saved>"
> +gdb_test "info registers rdi" "rdi .* <not saved>"
> +gdb_test "info registers r8" "r8 .* <not saved>"
> +gdb_test "info registers r9" "r9 .* <not saved>"
> +gdb_test "info registers r10" "r10 .* <not saved>"
> +gdb_test "info registers r11" "r11 .* <not saved>"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> index 238830e711e..70f22cb9efe 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
>
> # (2) struct_param_single_reg_loc
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>
> # (3) struct_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
>
> # (4) int_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> index b1c28b2f41c..2b296c8d445 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
> } else {
> set pattern_rax_rbx_rcx_print "<not saved>"
> set pattern_rax_rbx_rcx_info "<not saved>"
> - set pattern_r8_r9_print "$hex"
> - set pattern_r8_r9_info "$hex\\s+$decimal"
> + set pattern_r8_r9_print "<not saved>"
> + set pattern_r8_r9_info "<not saved>"
> }
>
> # Select frame.
> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
> # "not saved" and not "optimized out".
> gdb_test "set debug frame 1"
> gdb_test {print $rax} [multi_line \
> - { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
> {.*}]
> gdb_test "set debug frame 0"
>
> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> index f38e20003a3..3f193b1379b 100644
> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
> }
>
> mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
> - "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format x, frame ${f}"
>
> mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
> - "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format r, frame ${f}"
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-11-09 13:35 ` Andrew Burgess
@ 2021-11-15 9:06 ` Djordje Todorovic
2021-11-18 12:42 ` Djordje Todorovic
0 siblings, 1 reply; 12+ messages in thread
From: Djordje Todorovic @ 2021-11-15 9:06 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: Nikola Tesic, asowda
Hi Andrew,
Thanks for looking into this! I'll wait for the official commit of the patch you've shared.
The patch for this, rebased on top of the change you are about to commit, looks as follows:
---
gdb/amd64-tdep.c | 37 +++++++++++++
gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
3 files changed, 117 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index de04bbd07c1..cb3caa7d709 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,8 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* Implement the "init_reg" dwarf2_frame_ops method. */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
@@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
/* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
be in use on any of the supported AMD64 targets. */
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
new file mode 100644
index 00000000000..5698e549f00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2019-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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
new file mode 100644
index 00000000000..07b870b5d30
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# Copyright 2010-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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64-*-* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
--
2.25.1
________________________________
From: Andrew Burgess <aburgess@redhat.com>
Sent: Tuesday, November 9, 2021 2:35 PM
To: Djordje Todorovic <Djordje.Todorovic@syrmia.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Nikola Tesic <Nikola.Tesic@syrmia.com>; asowda@cisco.com <asowda@cisco.com>
Subject: Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
Djordje <djordje.todorovic@syrmia.com> writes:
> Hi,
>
> Please consider these two files:
>
> $ cat test2.c
> void f1(int);
> int f2();
> void f3();
>
> void f() {
> f1(0);
> int local = f2();
> f1(local);
> f3();
> }
>
> $ cat main-for-core.c
> #include <stdio.h>
>
> void f1(int param) {
> if (param % 2 == 0)
> return;
> printf("HELLO\n");
> abort();
> }
>
> int f2() {
> int x;
> scanf("%d", &x);
> return x;
> }
>
> void f3() {
> int y;
> scanf("%d", &y);
> printf("%d\n", y);
> }
>
> int main() {
> f();
> return 0;
> }
>
> Compilation of the files:
>
> $ clang -g -O2 test2.c -c -o object-file-test2.o
> $ clang -g -O2 main-for-core.c object-file-test2.o -o main-for-core
>
> By examining debug_loc from object-file-test2.o for the `local` var we see:
> 00000000 000000000000000f 0000000000000016 (DW_OP_reg0 (rax))
>
> And the code for the f() is:
> 0000000000000000 <f>:
> 0: push %rax
> 1: xor %edi,%edi
> 3: callq 8 <f+0x8>
> 8: xor %eax,%eax
> a: callq f <f+0xf>
> f: mov %eax,%edi
> 11: callq 16 <f+0x16>
> 16: xor %eax,%eax
> 18: pop %rcx
> 19: jmpq 1e <f+0x1e>
>
> While debugging it, by loading the core file generated due to abort() (as an input I typed 5):
>
> $ gdb main-for-core
> ...
> (gdb) bt
> #0 0x00007fb2d7aeb377 in raise () from /lib64/libc.so.6
> #1 0x00007fb2d7aeca68 in abort () from /lib64/libc.so.6
> #2 0x0000000000401197 in f1 (param=<optimized out>) at main-for-core.c:7
> #3 0x0000000000401216 in f () at test2.c:8
> #4 0x00000000004011f8 in main () at main-for-core.c:23
> (gdb) f 3
> #3 0x0000000000401216 in f () at test2.c:8
> 8 f1(local);
> (gdb) disassemble
> Dump of assembler code for function f:
> 0x0000000000401200 <+0>: push %rax
> 0x0000000000401201 <+1>: xor %edi,%edi
> 0x0000000000401203 <+3>: callq 0x401180 <f1>
> 0x0000000000401208 <+8>: xor %eax,%eax
> 0x000000000040120a <+10>: callq 0x4011a0 <f2>
> 0x000000000040120f <+15>: mov %eax,%edi
> 0x0000000000401211 <+17>: callq 0x401180 <f1>
> => 0x0000000000401216 <+22>: xor %eax,%eax
> 0x0000000000401218 <+24>: pop %rcx
> 0x0000000000401219 <+25>: jmpq 0x4011c0 <f3>
> End of assembler dump.
> (gdb) p local
> $1 = 0
>
> But it should be reported as <optimized_out>, because the %eax was clobbered by the call.
>
> GCC produces different high pc address to keep GDB working properly:
> 00000000 000000000000000f 0000000000000015 (DW_OP_reg0 (rax))
>
> This patch fixes the problem inside GDB itself for X86_64 target.
>
> Bug: https://bugs.llvm.org/show_bug.cgi?id=49641
> ---
> gdb/amd64-tdep.c | 37 +++++++++++++
> gdb/frame.c | 9 +++-
> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
> 8 files changed, 136 insertions(+), 14 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 47761208088..a4ac36b18be 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -25,6 +25,8 @@
> #include "arch-utils.h"
> #include "block.h"
> #include "dummy-frame.h"
> +#include "dwarf2.h"
> +#include "dwarf2/frame.h"
> #include "frame.h"
> #include "frame-base.h"
> #include "frame-unwind.h"
> @@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
> AMD64_RIP_REGNUM);
> }
>
> +/* Implement the "init_reg" dwarf2_frame_ops method. */
> +
> +static void
> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> + struct dwarf2_frame_state_reg *reg,
> + struct frame_info *this_frame)
> +{
> + switch (regnum)
> + {
> + case AMD64_RIP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_FN;
> + return;
> +
> + case AMD64_RSP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_CFA;
> + return;
> +
> + /* Caller-saved registers. */
> + case AMD64_RAX_REGNUM:
> + case AMD64_RDI_REGNUM:
> + case AMD64_RSI_REGNUM:
> + case AMD64_RDX_REGNUM:
> + case AMD64_RCX_REGNUM:
> + case AMD64_R8_REGNUM:
> + case AMD64_R9_REGNUM:
> + case AMD64_R10_REGNUM:
> + case AMD64_R11_REGNUM:
> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
> + return;
> + }
> +}
> +
> void
> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> const target_desc *default_tdesc)
> @@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>
> + /* Frame handling. */
> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
> +
> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
> be in use on any of the supported AMD64 targets. */
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 16673258373..a5b43c3f949 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2312,9 +2312,14 @@ get_prev_frame_always (struct frame_info *this_frame)
> }
> catch (const gdb_exception_error &ex)
> {
> - if (ex.error == MEMORY_ERROR)
> + if (ex.error == MEMORY_ERROR || ex.error == GENERIC_ERROR)
Thanks for working on this. I wasn't keen on this use of GENERIC_ERROR,
so I posted this patch:
https://sourceware.org/pipermail/gdb-patches/2021-November/183267.html
After that has been merged you can rebased this patch and this code
should be improved.
Thanks,
Andrew
> {
> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + if (ex.error == MEMORY_ERROR)
> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + else
> + /* This is for the GENERIC_ERROR case. */
> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
> +
> if (ex.message != NULL)
> {
> char *stop_string;
> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> index f9e590d83bb..6ae1c9c1322 100644
> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> @@ -42,11 +42,11 @@ if ![runto breakpt] {
> return -1
> }
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "first backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "first backtrace, with error message"
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "second backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "second backtrace, with error message"
>
> clean_restart ${binfile}
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> new file mode 100644
> index 00000000000..5698e549f00
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> @@ -0,0 +1,28 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2019-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/>. */
> +
> +int
> +foo (int a)
> +{
> + return a; /* break here. */
> +}
> +
> +int
> +main ()
> +{
> + return foo (5);
> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> new file mode 100644
> index 00000000000..07b870b5d30
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> @@ -0,0 +1,52 @@
> +# Copyright 2010-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/>.
> +
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
> +
> +# This file is part of the gdb testsuite.
> +
> +if { ![istarget x86_64-*-* ]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +# Build program with debug symbols.
> +standard_testfile
> +set compile_flags {debug}
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +gdb_breakpoint [ gdb_get_line_number "break here" ]
> +gdb_continue_to_breakpoint "break here" ".*break here.*"
> +
> +# Switch to frame 1.
> +gdb_test "frame 1" "#1.*in main ().*"
> +
> +gdb_test "info registers rax" "rax .* <not saved>"
> +gdb_test "info registers rcx" "rcx .* <not saved>"
> +gdb_test "info registers rdx" "rdx .* <not saved>"
> +gdb_test "info registers rsi" "rsi .* <not saved>"
> +gdb_test "info registers rdi" "rdi .* <not saved>"
> +gdb_test "info registers r8" "r8 .* <not saved>"
> +gdb_test "info registers r9" "r9 .* <not saved>"
> +gdb_test "info registers r10" "r10 .* <not saved>"
> +gdb_test "info registers r11" "r11 .* <not saved>"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> index 238830e711e..70f22cb9efe 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
>
> # (2) struct_param_single_reg_loc
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>
> # (3) struct_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
>
> # (4) int_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> index b1c28b2f41c..2b296c8d445 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
> } else {
> set pattern_rax_rbx_rcx_print "<not saved>"
> set pattern_rax_rbx_rcx_info "<not saved>"
> - set pattern_r8_r9_print "$hex"
> - set pattern_r8_r9_info "$hex\\s+$decimal"
> + set pattern_r8_r9_print "<not saved>"
> + set pattern_r8_r9_info "<not saved>"
> }
>
> # Select frame.
> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
> # "not saved" and not "optimized out".
> gdb_test "set debug frame 1"
> gdb_test {print $rax} [multi_line \
> - { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
> {.*}]
> gdb_test "set debug frame 0"
>
> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> index f38e20003a3..3f193b1379b 100644
> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
> }
>
> mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
> - "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format x, frame ${f}"
>
> mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
> - "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format r, frame ${f}"
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-11-15 9:06 ` Djordje Todorovic
@ 2021-11-18 12:42 ` Djordje Todorovic
2021-12-02 10:07 ` Djordje Todorovic
2021-12-02 15:56 ` Andrew Burgess
0 siblings, 2 replies; 12+ messages in thread
From: Djordje Todorovic @ 2021-11-18 12:42 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: Nikola Tesic, asowda
Hi Andrew,
Please find the patch, rebased on top of your improvement.
Thanks,
Djordje
---
gdb/amd64-tdep.c | 37 +++++++++++++
gdb/frame.c | 9 +++-
.../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
.../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
8 files changed, 136 insertions(+), 14 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
?
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 7c67359678b..a6b544145fb 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,8 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* Implement the "init_reg" dwarf2_frame_ops method. */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
@@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
/* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
be in use on any of the supported AMD64 targets. */
diff --git a/gdb/frame.c b/gdb/frame.c
index 2a899fc494f..da12ed36e02 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
}
catch (const gdb_exception_error &ex)
{
- if (ex.error == MEMORY_ERROR)
+ if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
{
- this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ if (ex.error == MEMORY_ERROR)
+ this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ else
+ /* This is for the OPTIMIZED_OUT_ERROR case. */
+ this_frame->stop_reason = UNWIND_UNAVAILABLE;
+
if (ex.message != NULL)
{
char *stop_string;
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index f9e590d83bb..6ae1c9c1322 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -42,11 +42,11 @@ if ![runto breakpt] {
return -1
}
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "first backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "first backtrace, with error message"
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "second backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "second backtrace, with error message"
clean_restart ${binfile}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
new file mode 100644
index 00000000000..5698e549f00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2019-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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
new file mode 100644
index 00000000000..07b870b5d30
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# Copyright 2010-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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64-*-* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
index 238830e711e..70f22cb9efe 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
@@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
# (2) struct_param_single_reg_loc
gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
-gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
+gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
# (3) struct_param_two_reg_pieces
gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
-gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
+gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
# (4) int_param_two_reg_pieces
gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index b1c28b2f41c..2b296c8d445 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
} else {
set pattern_rax_rbx_rcx_print "<not saved>"
set pattern_rax_rbx_rcx_info "<not saved>"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r8_r9_print "<not saved>"
+ set pattern_r8_r9_info "<not saved>"
}
# Select frame.
@@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
# "not saved" and not "optimized out".
gdb_test "set debug frame 1"
gdb_test {print $rax} [multi_line \
- { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
- { \[frame\] frame_unwind_register_value: -> <not saved>} \
+ { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
+ { \[frame\] frame_unwind_register_value: -> <not saved>} \
{.*}]
gdb_test "set debug frame 0"
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index f38e20003a3..3f193b1379b 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
}
mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
- "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
"register values, format x, frame ${f}"
mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
- "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
"register values, format r, frame ${f}"
}
--
2.25.1
?
________________________________
From: Djordje Todorovic <Djordje.Todorovic@syrmia.com>
Sent: Monday, November 15, 2021 10:06 AM
To: Andrew Burgess <aburgess@redhat.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Nikola Tesic <Nikola.Tesic@syrmia.com>; asowda@cisco.com <asowda@cisco.com>
Subject: Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
Hi Andrew,
Thanks for looking into this! I'll wait for the official commit of the patch you've shared.
The patch for this, rebased on top of the change you are about to commit, looks as follows:
---
gdb/amd64-tdep.c | 37 +++++++++++++
gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
3 files changed, 117 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index de04bbd07c1..cb3caa7d709 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,8 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* Implement the "init_reg" dwarf2_frame_ops method. */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
@@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
/* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
be in use on any of the supported AMD64 targets. */
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
new file mode 100644
index 00000000000..5698e549f00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2019-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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
new file mode 100644
index 00000000000..07b870b5d30
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# Copyright 2010-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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64-*-* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
--
2.25.1
________________________________
From: Andrew Burgess <aburgess@redhat.com>
Sent: Tuesday, November 9, 2021 2:35 PM
To: Djordje Todorovic <Djordje.Todorovic@syrmia.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Nikola Tesic <Nikola.Tesic@syrmia.com>; asowda@cisco.com <asowda@cisco.com>
Subject: Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
Djordje <djordje.todorovic@syrmia.com> writes:
> Hi,
>
> Please consider these two files:
>
> $ cat test2.c
> void f1(int);
> int f2();
> void f3();
>
> void f() {
> f1(0);
> int local = f2();
> f1(local);
> f3();
> }
>
> $ cat main-for-core.c
> #include <stdio.h>
>
> void f1(int param) {
> if (param % 2 == 0)
> return;
> printf("HELLO\n");
> abort();
> }
>
> int f2() {
> int x;
> scanf("%d", &x);
> return x;
> }
>
> void f3() {
> int y;
> scanf("%d", &y);
> printf("%d\n", y);
> }
>
> int main() {
> f();
> return 0;
> }
>
> Compilation of the files:
>
> $ clang -g -O2 test2.c -c -o object-file-test2.o
> $ clang -g -O2 main-for-core.c object-file-test2.o -o main-for-core
>
> By examining debug_loc from object-file-test2.o for the `local` var we see:
> 00000000 000000000000000f 0000000000000016 (DW_OP_reg0 (rax))
>
> And the code for the f() is:
> 0000000000000000 <f>:
> 0: push %rax
> 1: xor %edi,%edi
> 3: callq 8 <f+0x8>
> 8: xor %eax,%eax
> a: callq f <f+0xf>
> f: mov %eax,%edi
> 11: callq 16 <f+0x16>
> 16: xor %eax,%eax
> 18: pop %rcx
> 19: jmpq 1e <f+0x1e>
>
> While debugging it, by loading the core file generated due to abort() (as an input I typed 5):
>
> $ gdb main-for-core
> ...
> (gdb) bt
> #0 0x00007fb2d7aeb377 in raise () from /lib64/libc.so.6
> #1 0x00007fb2d7aeca68 in abort () from /lib64/libc.so.6
> #2 0x0000000000401197 in f1 (param=<optimized out>) at main-for-core.c:7
> #3 0x0000000000401216 in f () at test2.c:8
> #4 0x00000000004011f8 in main () at main-for-core.c:23
> (gdb) f 3
> #3 0x0000000000401216 in f () at test2.c:8
> 8 f1(local);
> (gdb) disassemble
> Dump of assembler code for function f:
> 0x0000000000401200 <+0>: push %rax
> 0x0000000000401201 <+1>: xor %edi,%edi
> 0x0000000000401203 <+3>: callq 0x401180 <f1>
> 0x0000000000401208 <+8>: xor %eax,%eax
> 0x000000000040120a <+10>: callq 0x4011a0 <f2>
> 0x000000000040120f <+15>: mov %eax,%edi
> 0x0000000000401211 <+17>: callq 0x401180 <f1>
> => 0x0000000000401216 <+22>: xor %eax,%eax
> 0x0000000000401218 <+24>: pop %rcx
> 0x0000000000401219 <+25>: jmpq 0x4011c0 <f3>
> End of assembler dump.
> (gdb) p local
> $1 = 0
>
> But it should be reported as <optimized_out>, because the %eax was clobbered by the call.
>
> GCC produces different high pc address to keep GDB working properly:
> 00000000 000000000000000f 0000000000000015 (DW_OP_reg0 (rax))
>
> This patch fixes the problem inside GDB itself for X86_64 target.
>
> Bug: https://bugs.llvm.org/show_bug.cgi?id=49641
> ---
> gdb/amd64-tdep.c | 37 +++++++++++++
> gdb/frame.c | 9 +++-
> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
> 8 files changed, 136 insertions(+), 14 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 47761208088..a4ac36b18be 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -25,6 +25,8 @@
> #include "arch-utils.h"
> #include "block.h"
> #include "dummy-frame.h"
> +#include "dwarf2.h"
> +#include "dwarf2/frame.h"
> #include "frame.h"
> #include "frame-base.h"
> #include "frame-unwind.h"
> @@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
> AMD64_RIP_REGNUM);
> }
>
> +/* Implement the "init_reg" dwarf2_frame_ops method. */
> +
> +static void
> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> + struct dwarf2_frame_state_reg *reg,
> + struct frame_info *this_frame)
> +{
> + switch (regnum)
> + {
> + case AMD64_RIP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_FN;
> + return;
> +
> + case AMD64_RSP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_CFA;
> + return;
> +
> + /* Caller-saved registers. */
> + case AMD64_RAX_REGNUM:
> + case AMD64_RDI_REGNUM:
> + case AMD64_RSI_REGNUM:
> + case AMD64_RDX_REGNUM:
> + case AMD64_RCX_REGNUM:
> + case AMD64_R8_REGNUM:
> + case AMD64_R9_REGNUM:
> + case AMD64_R10_REGNUM:
> + case AMD64_R11_REGNUM:
> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
> + return;
> + }
> +}
> +
> void
> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> const target_desc *default_tdesc)
> @@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>
> + /* Frame handling. */
> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
> +
> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
> be in use on any of the supported AMD64 targets. */
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 16673258373..a5b43c3f949 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2312,9 +2312,14 @@ get_prev_frame_always (struct frame_info *this_frame)
> }
> catch (const gdb_exception_error &ex)
> {
> - if (ex.error == MEMORY_ERROR)
> + if (ex.error == MEMORY_ERROR || ex.error == GENERIC_ERROR)
Thanks for working on this. I wasn't keen on this use of GENERIC_ERROR,
so I posted this patch:
https://sourceware.org/pipermail/gdb-patches/2021-November/183267.html
After that has been merged you can rebased this patch and this code
should be improved.
Thanks,
Andrew
> {
> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + if (ex.error == MEMORY_ERROR)
> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + else
> + /* This is for the GENERIC_ERROR case. */
> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
> +
> if (ex.message != NULL)
> {
> char *stop_string;
> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> index f9e590d83bb..6ae1c9c1322 100644
> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> @@ -42,11 +42,11 @@ if ![runto breakpt] {
> return -1
> }
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "first backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "first backtrace, with error message"
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "second backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "second backtrace, with error message"
>
> clean_restart ${binfile}
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> new file mode 100644
> index 00000000000..5698e549f00
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> @@ -0,0 +1,28 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2019-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/>. */
> +
> +int
> +foo (int a)
> +{
> + return a; /* break here. */
> +}
> +
> +int
> +main ()
> +{
> + return foo (5);
> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> new file mode 100644
> index 00000000000..07b870b5d30
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> @@ -0,0 +1,52 @@
> +# Copyright 2010-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/>.
> +
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
> +
> +# This file is part of the gdb testsuite.
> +
> +if { ![istarget x86_64-*-* ]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +# Build program with debug symbols.
> +standard_testfile
> +set compile_flags {debug}
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +gdb_breakpoint [ gdb_get_line_number "break here" ]
> +gdb_continue_to_breakpoint "break here" ".*break here.*"
> +
> +# Switch to frame 1.
> +gdb_test "frame 1" "#1.*in main ().*"
> +
> +gdb_test "info registers rax" "rax .* <not saved>"
> +gdb_test "info registers rcx" "rcx .* <not saved>"
> +gdb_test "info registers rdx" "rdx .* <not saved>"
> +gdb_test "info registers rsi" "rsi .* <not saved>"
> +gdb_test "info registers rdi" "rdi .* <not saved>"
> +gdb_test "info registers r8" "r8 .* <not saved>"
> +gdb_test "info registers r9" "r9 .* <not saved>"
> +gdb_test "info registers r10" "r10 .* <not saved>"
> +gdb_test "info registers r11" "r11 .* <not saved>"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> index 238830e711e..70f22cb9efe 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
>
> # (2) struct_param_single_reg_loc
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>
> # (3) struct_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
>
> # (4) int_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> index b1c28b2f41c..2b296c8d445 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
> } else {
> set pattern_rax_rbx_rcx_print "<not saved>"
> set pattern_rax_rbx_rcx_info "<not saved>"
> - set pattern_r8_r9_print "$hex"
> - set pattern_r8_r9_info "$hex\\s+$decimal"
> + set pattern_r8_r9_print "<not saved>"
> + set pattern_r8_r9_info "<not saved>"
> }
>
> # Select frame.
> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
> # "not saved" and not "optimized out".
> gdb_test "set debug frame 1"
> gdb_test {print $rax} [multi_line \
> - { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
> {.*}]
> gdb_test "set debug frame 0"
>
> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> index f38e20003a3..3f193b1379b 100644
> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
> }
>
> mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
> - "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format x, frame ${f}"
>
> mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
> - "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format r, frame ${f}"
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-11-18 12:42 ` Djordje Todorovic
@ 2021-12-02 10:07 ` Djordje Todorovic
2021-12-02 15:56 ` Andrew Burgess
1 sibling, 0 replies; 12+ messages in thread
From: Djordje Todorovic @ 2021-12-02 10:07 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches; +Cc: Nikola Tesic, asowda
Ping. 🙂
Best,
Djordje
________________________________
From: Djordje Todorovic <Djordje.Todorovic@syrmia.com>
Sent: Thursday, November 18, 2021 1:42 PM
To: Andrew Burgess <aburgess@redhat.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Nikola Tesic <Nikola.Tesic@syrmia.com>; asowda@cisco.com <asowda@cisco.com>
Subject: Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
Hi Andrew,
Please find the patch, rebased on top of your improvement.
Thanks,
Djordje
---
gdb/amd64-tdep.c | 37 +++++++++++++
gdb/frame.c | 9 +++-
.../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
.../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
8 files changed, 136 insertions(+), 14 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 7c67359678b..a6b544145fb 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,8 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* Implement the "init_reg" dwarf2_frame_ops method. */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
@@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
/* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
be in use on any of the supported AMD64 targets. */
diff --git a/gdb/frame.c b/gdb/frame.c
index 2a899fc494f..da12ed36e02 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
}
catch (const gdb_exception_error &ex)
{
- if (ex.error == MEMORY_ERROR)
+ if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
{
- this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ if (ex.error == MEMORY_ERROR)
+ this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ else
+ /* This is for the OPTIMIZED_OUT_ERROR case. */
+ this_frame->stop_reason = UNWIND_UNAVAILABLE;
+
if (ex.message != NULL)
{
char *stop_string;
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index f9e590d83bb..6ae1c9c1322 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -42,11 +42,11 @@ if ![runto breakpt] {
return -1
}
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "first backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "first backtrace, with error message"
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "second backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "second backtrace, with error message"
clean_restart ${binfile}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
new file mode 100644
index 00000000000..5698e549f00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2019-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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
new file mode 100644
index 00000000000..07b870b5d30
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# Copyright 2010-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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64-*-* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
index 238830e711e..70f22cb9efe 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
@@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
# (2) struct_param_single_reg_loc
gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
-gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
+gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
# (3) struct_param_two_reg_pieces
gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
-gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
+gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
# (4) int_param_two_reg_pieces
gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index b1c28b2f41c..2b296c8d445 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
} else {
set pattern_rax_rbx_rcx_print "<not saved>"
set pattern_rax_rbx_rcx_info "<not saved>"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r8_r9_print "<not saved>"
+ set pattern_r8_r9_info "<not saved>"
}
# Select frame.
@@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
# "not saved" and not "optimized out".
gdb_test "set debug frame 1"
gdb_test {print $rax} [multi_line \
- { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
- { \[frame\] frame_unwind_register_value: -> <not saved>} \
+ { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
+ { \[frame\] frame_unwind_register_value: -> <not saved>} \
{.*}]
gdb_test "set debug frame 0"
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index f38e20003a3..3f193b1379b 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
}
mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
- "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
"register values, format x, frame ${f}"
mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
- "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
"register values, format r, frame ${f}"
}
--
2.25.1
________________________________
From: Djordje Todorovic <Djordje.Todorovic@syrmia.com>
Sent: Monday, November 15, 2021 10:06 AM
To: Andrew Burgess <aburgess@redhat.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Nikola Tesic <Nikola.Tesic@syrmia.com>; asowda@cisco.com <asowda@cisco.com>
Subject: Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
Hi Andrew,
Thanks for looking into this! I'll wait for the official commit of the patch you've shared.
The patch for this, rebased on top of the change you are about to commit, looks as follows:
---
gdb/amd64-tdep.c | 37 +++++++++++++
gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
3 files changed, 117 insertions(+)
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index de04bbd07c1..cb3caa7d709 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,8 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* Implement the "init_reg" dwarf2_frame_ops method. */
+
+static void
+amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
@@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
+
/* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
be in use on any of the supported AMD64 targets. */
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
new file mode 100644
index 00000000000..5698e549f00
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ Copyright 2019-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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
new file mode 100644
index 00000000000..07b870b5d30
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# Copyright 2010-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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64-*-* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
--
2.25.1
________________________________
From: Andrew Burgess <aburgess@redhat.com>
Sent: Tuesday, November 9, 2021 2:35 PM
To: Djordje Todorovic <Djordje.Todorovic@syrmia.com>; gdb-patches@sourceware.org <gdb-patches@sourceware.org>
Cc: Nikola Tesic <Nikola.Tesic@syrmia.com>; asowda@cisco.com <asowda@cisco.com>
Subject: Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
Djordje <djordje.todorovic@syrmia.com> writes:
> Hi,
>
> Please consider these two files:
>
> $ cat test2.c
> void f1(int);
> int f2();
> void f3();
>
> void f() {
> f1(0);
> int local = f2();
> f1(local);
> f3();
> }
>
> $ cat main-for-core.c
> #include <stdio.h>
>
> void f1(int param) {
> if (param % 2 == 0)
> return;
> printf("HELLO\n");
> abort();
> }
>
> int f2() {
> int x;
> scanf("%d", &x);
> return x;
> }
>
> void f3() {
> int y;
> scanf("%d", &y);
> printf("%d\n", y);
> }
>
> int main() {
> f();
> return 0;
> }
>
> Compilation of the files:
>
> $ clang -g -O2 test2.c -c -o object-file-test2.o
> $ clang -g -O2 main-for-core.c object-file-test2.o -o main-for-core
>
> By examining debug_loc from object-file-test2.o for the `local` var we see:
> 00000000 000000000000000f 0000000000000016 (DW_OP_reg0 (rax))
>
> And the code for the f() is:
> 0000000000000000 <f>:
> 0: push %rax
> 1: xor %edi,%edi
> 3: callq 8 <f+0x8>
> 8: xor %eax,%eax
> a: callq f <f+0xf>
> f: mov %eax,%edi
> 11: callq 16 <f+0x16>
> 16: xor %eax,%eax
> 18: pop %rcx
> 19: jmpq 1e <f+0x1e>
>
> While debugging it, by loading the core file generated due to abort() (as an input I typed 5):
>
> $ gdb main-for-core
> ...
> (gdb) bt
> #0 0x00007fb2d7aeb377 in raise () from /lib64/libc.so.6
> #1 0x00007fb2d7aeca68 in abort () from /lib64/libc.so.6
> #2 0x0000000000401197 in f1 (param=<optimized out>) at main-for-core.c:7
> #3 0x0000000000401216 in f () at test2.c:8
> #4 0x00000000004011f8 in main () at main-for-core.c:23
> (gdb) f 3
> #3 0x0000000000401216 in f () at test2.c:8
> 8 f1(local);
> (gdb) disassemble
> Dump of assembler code for function f:
> 0x0000000000401200 <+0>: push %rax
> 0x0000000000401201 <+1>: xor %edi,%edi
> 0x0000000000401203 <+3>: callq 0x401180 <f1>
> 0x0000000000401208 <+8>: xor %eax,%eax
> 0x000000000040120a <+10>: callq 0x4011a0 <f2>
> 0x000000000040120f <+15>: mov %eax,%edi
> 0x0000000000401211 <+17>: callq 0x401180 <f1>
> => 0x0000000000401216 <+22>: xor %eax,%eax
> 0x0000000000401218 <+24>: pop %rcx
> 0x0000000000401219 <+25>: jmpq 0x4011c0 <f3>
> End of assembler dump.
> (gdb) p local
> $1 = 0
>
> But it should be reported as <optimized_out>, because the %eax was clobbered by the call.
>
> GCC produces different high pc address to keep GDB working properly:
> 00000000 000000000000000f 0000000000000015 (DW_OP_reg0 (rax))
>
> This patch fixes the problem inside GDB itself for X86_64 target.
>
> Bug: https://bugs.llvm.org/show_bug.cgi?id=49641
> ---
> gdb/amd64-tdep.c | 37 +++++++++++++
> gdb/frame.c | 9 +++-
> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
> 8 files changed, 136 insertions(+), 14 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 47761208088..a4ac36b18be 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -25,6 +25,8 @@
> #include "arch-utils.h"
> #include "block.h"
> #include "dummy-frame.h"
> +#include "dwarf2.h"
> +#include "dwarf2/frame.h"
> #include "frame.h"
> #include "frame-base.h"
> #include "frame-unwind.h"
> @@ -3101,6 +3103,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
> AMD64_RIP_REGNUM);
> }
>
> +/* Implement the "init_reg" dwarf2_frame_ops method. */
> +
> +static void
> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> + struct dwarf2_frame_state_reg *reg,
> + struct frame_info *this_frame)
> +{
> + switch (regnum)
> + {
> + case AMD64_RIP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_FN;
> + return;
> +
> + case AMD64_RSP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_CFA;
> + return;
> +
> + /* Caller-saved registers. */
> + case AMD64_RAX_REGNUM:
> + case AMD64_RDI_REGNUM:
> + case AMD64_RSI_REGNUM:
> + case AMD64_RDX_REGNUM:
> + case AMD64_RCX_REGNUM:
> + case AMD64_R8_REGNUM:
> + case AMD64_R9_REGNUM:
> + case AMD64_R10_REGNUM:
> + case AMD64_R11_REGNUM:
> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
> + return;
> + }
> +}
> +
> void
> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> const target_desc *default_tdesc)
> @@ -3215,6 +3249,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>
> + /* Frame handling. */
> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
> +
> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
> be in use on any of the supported AMD64 targets. */
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 16673258373..a5b43c3f949 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2312,9 +2312,14 @@ get_prev_frame_always (struct frame_info *this_frame)
> }
> catch (const gdb_exception_error &ex)
> {
> - if (ex.error == MEMORY_ERROR)
> + if (ex.error == MEMORY_ERROR || ex.error == GENERIC_ERROR)
Thanks for working on this. I wasn't keen on this use of GENERIC_ERROR,
so I posted this patch:
https://sourceware.org/pipermail/gdb-patches/2021-November/183267.html
After that has been merged you can rebased this patch and this code
should be improved.
Thanks,
Andrew
> {
> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + if (ex.error == MEMORY_ERROR)
> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + else
> + /* This is for the GENERIC_ERROR case. */
> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
> +
> if (ex.message != NULL)
> {
> char *stop_string;
> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> index f9e590d83bb..6ae1c9c1322 100644
> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> @@ -42,11 +42,11 @@ if ![runto breakpt] {
> return -1
> }
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "first backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "first backtrace, with error message"
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "second backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "second backtrace, with error message"
>
> clean_restart ${binfile}
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> new file mode 100644
> index 00000000000..5698e549f00
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> @@ -0,0 +1,28 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2019-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/>. */
> +
> +int
> +foo (int a)
> +{
> + return a; /* break here. */
> +}
> +
> +int
> +main ()
> +{
> + return foo (5);
> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> new file mode 100644
> index 00000000000..07b870b5d30
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> @@ -0,0 +1,52 @@
> +# Copyright 2010-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/>.
> +
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
> +
> +# This file is part of the gdb testsuite.
> +
> +if { ![istarget x86_64-*-* ]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +# Build program with debug symbols.
> +standard_testfile
> +set compile_flags {debug}
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +gdb_breakpoint [ gdb_get_line_number "break here" ]
> +gdb_continue_to_breakpoint "break here" ".*break here.*"
> +
> +# Switch to frame 1.
> +gdb_test "frame 1" "#1.*in main ().*"
> +
> +gdb_test "info registers rax" "rax .* <not saved>"
> +gdb_test "info registers rcx" "rcx .* <not saved>"
> +gdb_test "info registers rdx" "rdx .* <not saved>"
> +gdb_test "info registers rsi" "rsi .* <not saved>"
> +gdb_test "info registers rdi" "rdi .* <not saved>"
> +gdb_test "info registers r8" "r8 .* <not saved>"
> +gdb_test "info registers r9" "r9 .* <not saved>"
> +gdb_test "info registers r10" "r10 .* <not saved>"
> +gdb_test "info registers r11" "r11 .* <not saved>"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> index 238830e711e..70f22cb9efe 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
>
> # (2) struct_param_single_reg_loc
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>
> # (3) struct_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
>
> # (4) int_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> index b1c28b2f41c..2b296c8d445 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
> } else {
> set pattern_rax_rbx_rcx_print "<not saved>"
> set pattern_rax_rbx_rcx_info "<not saved>"
> - set pattern_r8_r9_print "$hex"
> - set pattern_r8_r9_info "$hex\\s+$decimal"
> + set pattern_r8_r9_print "<not saved>"
> + set pattern_r8_r9_info "<not saved>"
> }
>
> # Select frame.
> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
> # "not saved" and not "optimized out".
> gdb_test "set debug frame 1"
> gdb_test {print $rax} [multi_line \
> - { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
> {.*}]
> gdb_test "set debug frame 0"
>
> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> index f38e20003a3..3f193b1379b 100644
> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
> }
>
> mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
> - "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format x, frame ${f}"
>
> mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
> - "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format r, frame ${f}"
> }
> --
> 2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-11-18 12:42 ` Djordje Todorovic
2021-12-02 10:07 ` Djordje Todorovic
@ 2021-12-02 15:56 ` Andrew Burgess
2021-12-02 17:32 ` John Baldwin
` (2 more replies)
1 sibling, 3 replies; 12+ messages in thread
From: Andrew Burgess @ 2021-12-02 15:56 UTC (permalink / raw)
To: Djordje Todorovic; +Cc: gdb-patches, Nikola Tesic, asowda
* Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18 12:42:32 +0000]:
> Hi Andrew,
>
> Please find the patch, rebased on top of your improvement.
>
> Thanks,
> Djordje
>
> ---
> gdb/amd64-tdep.c | 37 +++++++++++++
> gdb/frame.c | 9 +++-
> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
> 8 files changed, 136 insertions(+), 14 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> ?
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 7c67359678b..a6b544145fb 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -25,6 +25,8 @@
> #include "arch-utils.h"
> #include "block.h"
> #include "dummy-frame.h"
> +#include "dwarf2.h"
> +#include "dwarf2/frame.h"
> #include "frame.h"
> #include "frame-base.h"
> #include "frame-unwind.h"
> @@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
> AMD64_RIP_REGNUM);
> }
>
> +/* Implement the "init_reg" dwarf2_frame_ops method. */
> +
> +static void
> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> + struct dwarf2_frame_state_reg *reg,
> + struct frame_info *this_frame)
> +{
> + switch (regnum)
> + {
> + case AMD64_RIP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_FN;
> + return;
> +
> + case AMD64_RSP_REGNUM:
> + reg->how = DWARF2_FRAME_REG_CFA;
> + return;
> +
> + /* Caller-saved registers. */
> + case AMD64_RAX_REGNUM:
> + case AMD64_RDI_REGNUM:
> + case AMD64_RSI_REGNUM:
> + case AMD64_RDX_REGNUM:
> + case AMD64_RCX_REGNUM:
> + case AMD64_R8_REGNUM:
> + case AMD64_R9_REGNUM:
> + case AMD64_R10_REGNUM:
> + case AMD64_R11_REGNUM:
> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
> + return;
> + }
> +}
I believe that this is the System-V ABI, which is not the only ABI
used on x86-64. From what I've read I believe that Windows uses a
slightly different ABI, where $rsi and $rdi are not callee saved.
I think that we might consider moving this function from the general
amd64-tdep.c to something like amd64-linux-tdep.c, and register this
in the function amd64_linux_init_abi.
Of course, this will mean that other System-V like targets would miss
out for now, but maybe the function itself could be renamed to
something like amd64_system_v_dwarf2_frame_init_reg, and left in
amd64-tdep.c, then from amd64-linux-tdep.c we can register that
function. And in future, other system-v like targets can also
register the function.
> +
> void
> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> const target_desc *default_tdesc)
> @@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>
> + /* Frame handling. */
> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
> +
> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
> be in use on any of the supported AMD64 targets. */
>
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 2a899fc494f..da12ed36e02 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
> }
> catch (const gdb_exception_error &ex)
> {
> - if (ex.error == MEMORY_ERROR)
> + if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
> {
> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + if (ex.error == MEMORY_ERROR)
> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> + else
> + /* This is for the OPTIMIZED_OUT_ERROR case. */
> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
> +
> if (ex.message != NULL)
> {
> char *stop_string;
> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> index f9e590d83bb..6ae1c9c1322 100644
> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> @@ -42,11 +42,11 @@ if ![runto breakpt] {
> return -1
> }
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "first backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "first backtrace, with error message"
I think the new "value has been optimized out" message is better than
what we had previously ("Cannot access memory at address 0x....")
which I guess was caused by trying to access through an unavailable
register.
That said, I remember a long time ago there was a whole big discussion
about whether registers that were not saved should be described as
"optimized out" or not. In the end we added the "<not saved>" message
to cover this case.
So I wonder if we should instead be ending the backtrace with
something like: "Backtrace stopped: value is not available"? I took
that exact text from require_available (value.c) but anything that
talks about available rather than optimized out might be better.
Though I don't know how hard it would be to achieve that result. I
guess we'd need to start in dwarf2_frame_prev_register where we map
DWARF2_FRAME_REG_UNDEFINED onto optimized_out - maybe that needs to
map to an unavailable value instead, but that feels like it might have
huge knock on consequences ... we don't want optimized out variables
to suddenly start reporting themselves as unavailable instead...
I don't know if others have any opinions on this, but it might be
worth having a short investigation to see how hard such a change would
be, I guess if it turns out to be a real pain then we could punt it
for later.
>
> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> - "second backtrace, with error message"
> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> + "second backtrace, with error message"
>
> clean_restart ${binfile}
>
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> new file mode 100644
> index 00000000000..5698e549f00
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> @@ -0,0 +1,28 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> + Copyright 2019-2021 Free Software Foundation, Inc.
Should that copyright range just be 2021?
> +
> + 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/>. */
> +
> +int
> +foo (int a)
> +{
> + return a; /* break here. */
> +}
> +
> +int
> +main ()
> +{
> + return foo (5);
> +}
> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> new file mode 100644
> index 00000000000..07b870b5d30
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> @@ -0,0 +1,52 @@
> +# Copyright 2010-2021 Free Software Foundation, Inc.
I suspect the copyright range here is wrong too.
For new files you should only have a from date in the copyright range
if the content of the file is almost completely copied unchanged from
some other, older, file. If the content in the new file is mostly
new, then you should just place this years date as the range:
# 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/>.
> +
> +# Please email any bugs, comments, and/or additions to this file to:
> +# bug-gdb@gnu.org
> +
> +# This file is part of the gdb testsuite.
> +
> +if { ![istarget x86_64-*-* ]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
> +
> +# Build program with debug symbols.
> +standard_testfile
> +set compile_flags {debug}
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +gdb_breakpoint [ gdb_get_line_number "break here" ]
> +gdb_continue_to_breakpoint "break here" ".*break here.*"
> +
> +# Switch to frame 1.
> +gdb_test "frame 1" "#1.*in main ().*"
> +
> +gdb_test "info registers rax" "rax .* <not saved>"
> +gdb_test "info registers rcx" "rcx .* <not saved>"
> +gdb_test "info registers rdx" "rdx .* <not saved>"
> +gdb_test "info registers rsi" "rsi .* <not saved>"
> +gdb_test "info registers rdi" "rdi .* <not saved>"
> +gdb_test "info registers r8" "r8 .* <not saved>"
> +gdb_test "info registers r9" "r9 .* <not saved>"
> +gdb_test "info registers r10" "r10 .* <not saved>"
> +gdb_test "info registers r11" "r11 .* <not saved>"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> index 238830e711e..70f22cb9efe 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
>
> # (2) struct_param_single_reg_loc
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>
> # (3) struct_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
I'm worried that this test has lost some of its meaning with this
change. What I think we need to do is modify the .S file that
accompanies this test to mark all of the registers that the ABI marks
as undefined, to DW_CFA_same_value. If you're not comfortable doing
that I'm happy to help once the other issues have been addressed.
>
> # (4) int_param_two_reg_pieces
> gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> index b1c28b2f41c..2b296c8d445 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
> } else {
> set pattern_rax_rbx_rcx_print "<not saved>"
> set pattern_rax_rbx_rcx_info "<not saved>"
> - set pattern_r8_r9_print "$hex"
> - set pattern_r8_r9_info "$hex\\s+$decimal"
> + set pattern_r8_r9_print "<not saved>"
> + set pattern_r8_r9_info "<not saved>"
> }
Again, I think we've lost something here. The original choice of
r8/r9 was arbitrary, and, at the time didn't make any difference as
all the registers were treated as "same value".
I'd suggest that you add testing of r12/r13 here as they should show
the same behaviour that r8/r9 did before the patch.
>
> # Select frame.
> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
> # "not saved" and not "optimized out".
> gdb_test "set debug frame 1"
> gdb_test {print $rax} [multi_line \
> - { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
As above, this change is fine for rax, but you should add a test for
rbx too, that register should behave just as rax used too before your
patch.
> {.*}]
> gdb_test "set debug frame 0"
>
> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> index f38e20003a3..3f193b1379b 100644
> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
> }
>
> mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
> - "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format x, frame ${f}"
>
> mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
> - "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
> + "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
> "register values, format r, frame ${f}"
Like the above test, I think you need to extend what's being tested
here to cover registers that are similar to the old behaviour, I'd
suggest r12/r13 again.
Also, you should probably rename pattern_0_1_2 as this was referring
to register 0, 1, and 2, but now is being used more widely.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-12-02 15:56 ` Andrew Burgess
@ 2021-12-02 17:32 ` John Baldwin
2021-12-03 16:27 ` Djordje
2021-12-03 17:45 ` Pedro Alves
2 siblings, 0 replies; 12+ messages in thread
From: John Baldwin @ 2021-12-02 17:32 UTC (permalink / raw)
To: Andrew Burgess, Djordje Todorovic; +Cc: Nikola Tesic, gdb-patches, asowda
On 12/2/21 7:56 AM, Andrew Burgess via Gdb-patches wrote:
> * Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18 12:42:32 +0000]:
>
>> +/* Implement the "init_reg" dwarf2_frame_ops method. */
>> +
>> +static void
>> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>> + struct dwarf2_frame_state_reg *reg,
>> + struct frame_info *this_frame)
>> +{
>> + switch (regnum)
>> + {
>> + case AMD64_RIP_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_FN;
>> + return;
>> +
>> + case AMD64_RSP_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_CFA;
>> + return;
>> +
>> + /* Caller-saved registers. */
>> + case AMD64_RAX_REGNUM:
>> + case AMD64_RDI_REGNUM:
>> + case AMD64_RSI_REGNUM:
>> + case AMD64_RDX_REGNUM:
>> + case AMD64_RCX_REGNUM:
>> + case AMD64_R8_REGNUM:
>> + case AMD64_R9_REGNUM:
>> + case AMD64_R10_REGNUM:
>> + case AMD64_R11_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
>> + return;
>> + }
>> +}
>
> I believe that this is the System-V ABI, which is not the only ABI
> used on x86-64. From what I've read I believe that Windows uses a
> slightly different ABI, where $rsi and $rdi are not callee saved.
>
> I think that we might consider moving this function from the general
> amd64-tdep.c to something like amd64-linux-tdep.c, and register this
> in the function amd64_linux_init_abi.
>
> Of course, this will mean that other System-V like targets would miss
> out for now, but maybe the function itself could be renamed to
> something like amd64_system_v_dwarf2_frame_init_reg, and left in
> amd64-tdep.c, then from amd64-linux-tdep.c we can register that
> function. And in future, other system-v like targets can also
> register the function.
I would second this. Maybe call it 'amd64_svr4_dwarf2_frame_init_reg'
to match solib-svr4.c and it's functions? All of the *BSD's would want
this as well as Linux.
--
John Baldwin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-12-02 15:56 ` Andrew Burgess
2021-12-02 17:32 ` John Baldwin
@ 2021-12-03 16:27 ` Djordje
2021-12-03 16:29 ` Djordje
2021-12-03 17:45 ` Pedro Alves
2 siblings, 1 reply; 12+ messages in thread
From: Djordje @ 2021-12-03 16:27 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches, Nikola Tesic, asowda
Hi Andrew,
Thanks for your suggestions (some comments inline)!
Best,
Djordje
On 2.12.21. 16:56, Andrew Burgess wrote:
> * Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18 12:42:32 +0000]:
>
>> Hi Andrew,
>>
>> Please find the patch, rebased on top of your improvement.
>>
>> Thanks,
>> Djordje
>>
>> ---
>> gdb/amd64-tdep.c | 37 +++++++++++++
>> gdb/frame.c | 9 +++-
>> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
>> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
>> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
>> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
>> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
>> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
>> 8 files changed, 136 insertions(+), 14 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>> ?
>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
>> index 7c67359678b..a6b544145fb 100644
>> --- a/gdb/amd64-tdep.c
>> +++ b/gdb/amd64-tdep.c
>> @@ -25,6 +25,8 @@
>> #include "arch-utils.h"
>> #include "block.h"
>> #include "dummy-frame.h"
>> +#include "dwarf2.h"
>> +#include "dwarf2/frame.h"
>> #include "frame.h"
>> #include "frame-base.h"
>> #include "frame-unwind.h"
>> @@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
>> AMD64_RIP_REGNUM);
>> }
>>
>> +/* Implement the "init_reg" dwarf2_frame_ops method. */
>> +
>> +static void
>> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>> + struct dwarf2_frame_state_reg *reg,
>> + struct frame_info *this_frame)
>> +{
>> + switch (regnum)
>> + {
>> + case AMD64_RIP_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_FN;
>> + return;
>> +
>> + case AMD64_RSP_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_CFA;
>> + return;
>> +
>> + /* Caller-saved registers. */
>> + case AMD64_RAX_REGNUM:
>> + case AMD64_RDI_REGNUM:
>> + case AMD64_RSI_REGNUM:
>> + case AMD64_RDX_REGNUM:
>> + case AMD64_RCX_REGNUM:
>> + case AMD64_R8_REGNUM:
>> + case AMD64_R9_REGNUM:
>> + case AMD64_R10_REGNUM:
>> + case AMD64_R11_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
>> + return;
>> + }
>> +}
> I believe that this is the System-V ABI, which is not the only ABI
> used on x86-64. From what I've read I believe that Windows uses a
> slightly different ABI, where $rsi and $rdi are not callee saved.
>
> I think that we might consider moving this function from the general
> amd64-tdep.c to something like amd64-linux-tdep.c, and register this
> in the function amd64_linux_init_abi.
>
> Of course, this will mean that other System-V like targets would miss
> out for now, but maybe the function itself could be renamed to
> something like amd64_system_v_dwarf2_frame_init_reg, and left in
> amd64-tdep.c, then from amd64-linux-tdep.c we can register that
> function. And in future, other system-v like targets can also
> register the function.
I'd vote for this, thanks (also, John Baldwin agreed on this -- it is in
a separate mail).
>> +
>> void
>> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>> const target_desc *default_tdesc)
>> @@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>>
>> + /* Frame handling. */
>> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
>> +
>> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
>> be in use on any of the supported AMD64 targets. */
>>
>> diff --git a/gdb/frame.c b/gdb/frame.c
>> index 2a899fc494f..da12ed36e02 100644
>> --- a/gdb/frame.c
>> +++ b/gdb/frame.c
>> @@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
>> }
>> catch (const gdb_exception_error &ex)
>> {
>> - if (ex.error == MEMORY_ERROR)
>> + if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
>> {
>> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>> + if (ex.error == MEMORY_ERROR)
>> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>> + else
>> + /* This is for the OPTIMIZED_OUT_ERROR case. */
>> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
>> +
>> if (ex.message != NULL)
>> {
>> char *stop_string;
>> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>> index f9e590d83bb..6ae1c9c1322 100644
>> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>> @@ -42,11 +42,11 @@ if ![runto breakpt] {
>> return -1
>> }
>>
>> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
>> - "first backtrace, with error message"
>> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
>> + "first backtrace, with error message"
> I think the new "value has been optimized out" message is better than
> what we had previously ("Cannot access memory at address 0x....")
> which I guess was caused by trying to access through an unavailable
> register.
>
> That said, I remember a long time ago there was a whole big discussion
> about whether registers that were not saved should be described as
> "optimized out" or not. In the end we added the "<not saved>" message
> to cover this case.
>
> So I wonder if we should instead be ending the backtrace with
> something like: "Backtrace stopped: value is not available"? I took
> that exact text from require_available (value.c) but anything that
> talks about available rather than optimized out might be better.
>
> Though I don't know how hard it would be to achieve that result. I
> guess we'd need to start in dwarf2_frame_prev_register where we map
> DWARF2_FRAME_REG_UNDEFINED onto optimized_out - maybe that needs to
> map to an unavailable value instead, but that feels like it might have
> huge knock on consequences ... we don't want optimized out variables
> to suddenly start reporting themselves as unavailable instead...
>
> I don't know if others have any opinions on this, but it might be
> worth having a short investigation to see how hard such a change would
> be, I guess if it turns out to be a real pain then we could punt it
> for later.
I am with you there. Unfortunately, I won't be able to work on that now.
>
>> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
>> - "second backtrace, with error message"
>> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
>> + "second backtrace, with error message"
>>
>> clean_restart ${binfile}
>>
>> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>> new file mode 100644
>> index 00000000000..5698e549f00
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>> @@ -0,0 +1,28 @@
>> +/* This test program is part of GDB, the GNU debugger.
>> +
>> + Copyright 2019-2021 Free Software Foundation, Inc.
> Should that copyright range just be 2021?
Yep, thanks!
>
>> +
>> + 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/>. */
>> +
>> +int
>> +foo (int a)
>> +{
>> + return a; /* break here. */
>> +}
>> +
>> +int
>> +main ()
>> +{
>> + return foo (5);
>> +}
>> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>> new file mode 100644
>> index 00000000000..07b870b5d30
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>> @@ -0,0 +1,52 @@
>> +# Copyright 2010-2021 Free Software Foundation, Inc.
> I suspect the copyright range here is wrong too.
>
> For new files you should only have a from date in the copyright range
> if the content of the file is almost completely copied unchanged from
> some other, older, file. If the content in the new file is mostly
> new, then you should just place this years date as the range:
>
> # 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/>.
>> +
>> +# Please email any bugs, comments, and/or additions to this file to:
>> +# bug-gdb@gnu.org
>> +
>> +# This file is part of the gdb testsuite.
>> +
>> +if { ![istarget x86_64-*-* ]} {
>> + verbose "Skipping ${gdb_test_file_name}."
>> + return
>> +}
>> +
>> +# Build program with debug symbols.
>> +standard_testfile
>> +set compile_flags {debug}
>> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
>> + return -1
>> +}
>> +
>> +if ![runto_main] {
>> + untested "could not run to main"
>> + return -1
>> +}
>> +
>> +gdb_breakpoint [ gdb_get_line_number "break here" ]
>> +gdb_continue_to_breakpoint "break here" ".*break here.*"
>> +
>> +# Switch to frame 1.
>> +gdb_test "frame 1" "#1.*in main ().*"
>> +
>> +gdb_test "info registers rax" "rax .* <not saved>"
>> +gdb_test "info registers rcx" "rcx .* <not saved>"
>> +gdb_test "info registers rdx" "rdx .* <not saved>"
>> +gdb_test "info registers rsi" "rsi .* <not saved>"
>> +gdb_test "info registers rdi" "rdi .* <not saved>"
>> +gdb_test "info registers r8" "r8 .* <not saved>"
>> +gdb_test "info registers r9" "r9 .* <not saved>"
>> +gdb_test "info registers r10" "r10 .* <not saved>"
>> +gdb_test "info registers r11" "r11 .* <not saved>"
>> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>> index 238830e711e..70f22cb9efe 100644
>> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?int_param_single_
>>
>> # (2) struct_param_single_reg_loc
>> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_single_reg_loc"
>> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b = <optimized out>}, operand1=<optimized out>, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>>
>> # (3) struct_param_two_reg_pieces
>> gdb_continue_to_breakpoint "Stop in breakpt for struct_param_two_reg_pieces"
>> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b = <optimized out>}, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
>> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in )?struct_param_two_reg_pieces \\(operand0=<optimized out>, operand1={a = <optimized out>, b = 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
> I'm worried that this test has lost some of its meaning with this
> change. What I think we need to do is modify the .S file that
> accompanies this test to mark all of the registers that the ABI marks
> as undefined, to DW_CFA_same_value. If you're not comfortable doing
> that I'm happy to help once the other issues have been addressed.
I am not very familiar with incorporating that info from asm level, so
any help will be appreciated.
>> # (4) int_param_two_reg_pieces
>> gdb_continue_to_breakpoint "Stop in breakpt for int_param_two_reg_pieces"
>> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>> index b1c28b2f41c..2b296c8d445 100644
>> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
>> } else {
>> set pattern_rax_rbx_rcx_print "<not saved>"
>> set pattern_rax_rbx_rcx_info "<not saved>"
>> - set pattern_r8_r9_print "$hex"
>> - set pattern_r8_r9_info "$hex\\s+$decimal"
>> + set pattern_r8_r9_print "<not saved>"
>> + set pattern_r8_r9_info "<not saved>"
>> }
> Again, I think we've lost something here. The original choice of
> r8/r9 was arbitrary, and, at the time didn't make any difference as
> all the registers were treated as "same value".
>
> I'd suggest that you add testing of r12/r13 here as they should show
> the same behaviour that r8/r9 did before the patch.
Oh yes. I've updated it automatically.
>> # Select frame.
>> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
>> # "not saved" and not "optimized out".
>> gdb_test "set debug frame 1"
>> gdb_test {print $rax} [multi_line \
>> - { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
>> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
>> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
>> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
> As above, this change is fine for rax, but you should add a test for
> rbx too, that register should behave just as rax used too before your
> patch.
Thanks!
>> {.*}]
>> gdb_test "set debug frame 0"
>>
>> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>> index f38e20003a3..3f193b1379b 100644
>> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
>> }
>>
>> mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
>> - "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
>> + "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
>> "register values, format x, frame ${f}"
>>
>> mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
>> - "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
>> + "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]" \
>> "register values, format r, frame ${f}"
> Like the above test, I think you need to extend what's being tested
> here to cover registers that are similar to the old behaviour, I'd
> suggest r12/r13 again.
>
> Also, you should probably rename pattern_0_1_2 as this was referring
> to register 0, 1, and 2, but now is being used more widely.
Sure, thanks!
> Thanks,
> Andrew
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-12-03 16:27 ` Djordje
@ 2021-12-03 16:29 ` Djordje
0 siblings, 0 replies; 12+ messages in thread
From: Djordje @ 2021-12-03 16:29 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches, Nikola Tesic, asowda
Please find updated patch.
Djordje
---
gdb/amd64-linux-tdep.c | 4 ++
gdb/amd64-tdep.c | 33 ++++++++++++
gdb/amd64-tdep.h | 6 +++
gdb/frame.c | 9 +++-
.../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
.../gdb.arch/amd64-svr4-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-svr4-not-saved-regs.exp | 52 +++++++++++++++++++
gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S | 3 +-
.../gdb.dwarf2/dw2-reg-undefined.exp | 24 ++++-----
gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 12 ++---
10 files changed, 153 insertions(+), 26 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 817a197ceaa..dac8dfd2f1a 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -21,6 +21,7 @@
#include "defs.h"
#include "arch-utils.h"
#include "frame.h"
+#include "dwarf2/frame.h"
#include "gdbcore.h"
#include "regcache.h"
#include "osabi.h"
@@ -1835,6 +1836,9 @@ amd64_linux_init_abi_common(struct gdbarch_info
info, struct gdbarch *gdbarch,
set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
set_gdbarch_report_signal_info (gdbarch, i386_linux_report_signal_info);
+
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_svr4_dwarf2_frame_init_reg);
}
static void
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 7c67359678b..1e3e68c0c04 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,7 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3103,6 +3104,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch
*gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* System-V ABI for call-saved/clobbered register information. */
+
+void
+amd64_svr4_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 6faa399cebb..9bb09baad0b 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -109,6 +109,12 @@ extern void amd64_x32_init_abi (struct gdbarch_info
info,
extern const struct target_desc *amd64_target_description (uint64_t xcr0,
bool segments);
+/* System-V ABI for call-saved/clobbered register information. */
+extern void
+amd64_svr4_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame);
+
/* Fill register REGNUM in REGCACHE with the appropriate
floating-point or SSE register value from *FXSAVE. If REGNUM is
-1, do this for all registers. This function masks off any of the
diff --git a/gdb/frame.c b/gdb/frame.c
index 2a899fc494f..da12ed36e02 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
}
catch (const gdb_exception_error &ex)
{
- if (ex.error == MEMORY_ERROR)
+ if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
{
- this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ if (ex.error == MEMORY_ERROR)
+ this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ else
+ /* This is for the OPTIMIZED_OUT_ERROR case. */
+ this_frame->stop_reason = UNWIND_UNAVAILABLE;
+
if (ex.message != NULL)
{
char *stop_string;
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index f9e590d83bb..6ae1c9c1322 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -42,11 +42,11 @@ if ![runto breakpt] {
return -1
}
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
+0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "first backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
+0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
stopped: value has been optimized out" \
+ "first backtrace, with error message"
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
+0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "second backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
+0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
stopped: value has been optimized out" \
+ "second backtrace, with error message"
clean_restart ${binfile}
diff --git a/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
new file mode 100644
index 00000000000..27a7bf36fe9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ 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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
new file mode 100644
index 00000000000..78af000d0f7
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# 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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64*-linux* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}
${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
index 28445b4682c..99a55ef7891 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
@@ -281,8 +281,7 @@ int_param_two_reg_pieces:
.long .Lframe0 /* CIE reference */
.quad .Ltext8 /* start */
.quad .Ltext10-.Ltext8 /* length */
- /* Instructions */
- .byte 0x4
+ .byte 0x4
.long .Ltext9-.Ltext8
.byte 0xe
.uleb128 0x10
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index b1c28b2f41c..f4bb0ca1834 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -42,13 +42,13 @@ for {set f 0} {$f < 3} {incr f} {
if {${f} == 0} {
set pattern_rax_rbx_rcx_print "$hex"
set pattern_rax_rbx_rcx_info "$hex\\s+$decimal"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r12_r13_print "$hex"
+ set pattern_r12_r13_info "$hex\\s+$decimal"
} else {
set pattern_rax_rbx_rcx_print "<not saved>"
set pattern_rax_rbx_rcx_info "<not saved>"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r12_r13_print "$hex"
+ set pattern_r12_r13_info "$hex\\s+$decimal"
}
# Select frame.
@@ -61,26 +61,26 @@ for {set f 0} {$f < 3} {incr f} {
gdb_test "p/x \$rcx" "$pattern_rax_rbx_rcx_print" \
"print \$rcx in frame ${f}"
- gdb_test "p/x \$r8" "$pattern_r8_r9_print" "print \$r8 in frame ${f}"
- gdb_test "p/x \$r9" "$pattern_r8_r9_print" "print \$r9 in frame ${f}"
+ gdb_test "p/x \$r12" "$pattern_r12_r13_print" "print \$r12 in frame
${f}"
+ gdb_test "p/x \$r13" "$pattern_r12_r13_print" "print \$r13 in frame
${f}"
# Display register values.
- gdb_test "info registers rax rbx rcx r8 r9" \
+ gdb_test "info registers rax rbx rcx r12 r13" \
[multi_line "rax\\s+${pattern_rax_rbx_rcx_info}\\s*" \
"rbx\\s+${pattern_rax_rbx_rcx_info}\\s*" \
"rcx\\s+${pattern_rax_rbx_rcx_info}\\s*" \
- "r8\\s+${pattern_r8_r9_info}\\s*" \
- "r9\\s+${pattern_r8_r9_info}\\s*"] \
- "Check values of rax, rbx, rcx, r8, r9 in frame ${f}"
+ "r12\\s+${pattern_r12_r13_info}\\s*" \
+ "r13\\s+${pattern_r12_r13_info}\\s*"] \
+ "Check values of rax, rbx, rcx, r12, r13 in frame ${f}"
}
# Test that the debug log statement in frame_unwind_register_value
produces
# "not saved" and not "optimized out".
gdb_test "set debug frame 1"
gdb_test {print $rax} [multi_line \
- { \[frame\] frame_unwind_register_value: frame=0,
regnum=0\(rax\)} \
- { \[frame\] frame_unwind_register_value: -> <not saved>} \
+ { \[frame\] frame_unwind_register_value: frame=1,
regnum=0\(rax\)} \
+ { \[frame\] frame_unwind_register_value: -> <not saved>} \
{.*}]
gdb_test "set debug frame 0"
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index f38e20003a3..06d054ce8ac 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -56,16 +56,16 @@ set not_saved_pattern "<not saved>"
for {set f 0} {$f < 3} {incr f} {
if {${f} == 0} {
- set pattern_0_1_2 ${hex}
+ set pattern_caller_saved_regs ${hex}
} else {
- set pattern_0_1_2 ${not_saved_pattern}
+ set pattern_caller_saved_regs ${not_saved_pattern}
}
- mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame
${f} x 0 1 2 8 9" \
-
"22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]"
\
+ mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame
${f} x 0 1 2 12 13" \
+
"22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"1\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"2\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"12\",value=\"$hex\"\},\{number=\"13\",value=\"$hex\"\}\]"
\
"register values, format x, frame ${f}"
- mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame
${f} r 0 1 2 8 9" \
-
"33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]"
\
+ mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame
${f} r 0 1 2" \
+
"33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"1\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"2\",value=\"${pattern_caller_saved_regs}\"\}\]"
\
"register values, format r, frame ${f}"
}
--
2.25.1
On 3.12.21. 17:27, Djordje wrote:
> Hi Andrew,
>
> Thanks for your suggestions (some comments inline)!
>
> Best,
>
> Djordje
>
>
> On 2.12.21. 16:56, Andrew Burgess wrote:
>> * Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18
>> 12:42:32 +0000]:
>>
>>> Hi Andrew,
>>>
>>> Please find the patch, rebased on top of your improvement.
>>>
>>> Thanks,
>>> Djordje
>>>
>>> ---
>>> gdb/amd64-tdep.c | 37 +++++++++++++
>>> gdb/frame.c | 9 +++-
>>> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
>>> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
>>> .../gdb.arch/amd64-not-saved-regs.exp | 52
>>> +++++++++++++++++++
>>> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
>>> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
>>> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
>>> 8 files changed, 136 insertions(+), 14 deletions(-)
>>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>>> ?
>>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
>>> index 7c67359678b..a6b544145fb 100644
>>> --- a/gdb/amd64-tdep.c
>>> +++ b/gdb/amd64-tdep.c
>>> @@ -25,6 +25,8 @@
>>> #include "arch-utils.h"
>>> #include "block.h"
>>> #include "dummy-frame.h"
>>> +#include "dwarf2.h"
>>> +#include "dwarf2/frame.h"
>>> #include "frame.h"
>>> #include "frame-base.h"
>>> #include "frame-unwind.h"
>>> @@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct
>>> gdbarch *gdbarch, CORE_ADDR pc)
>>> AMD64_RIP_REGNUM);
>>> }
>>>
>>> +/* Implement the "init_reg" dwarf2_frame_ops method. */
>>> +
>>> +static void
>>> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>>> + struct dwarf2_frame_state_reg *reg,
>>> + struct frame_info *this_frame)
>>> +{
>>> + switch (regnum)
>>> + {
>>> + case AMD64_RIP_REGNUM:
>>> + reg->how = DWARF2_FRAME_REG_FN;
>>> + return;
>>> +
>>> + case AMD64_RSP_REGNUM:
>>> + reg->how = DWARF2_FRAME_REG_CFA;
>>> + return;
>>> +
>>> + /* Caller-saved registers. */
>>> + case AMD64_RAX_REGNUM:
>>> + case AMD64_RDI_REGNUM:
>>> + case AMD64_RSI_REGNUM:
>>> + case AMD64_RDX_REGNUM:
>>> + case AMD64_RCX_REGNUM:
>>> + case AMD64_R8_REGNUM:
>>> + case AMD64_R9_REGNUM:
>>> + case AMD64_R10_REGNUM:
>>> + case AMD64_R11_REGNUM:
>>> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
>>> + return;
>>> + }
>>> +}
>> I believe that this is the System-V ABI, which is not the only ABI
>> used on x86-64. From what I've read I believe that Windows uses a
>> slightly different ABI, where $rsi and $rdi are not callee saved.
>>
>> I think that we might consider moving this function from the general
>> amd64-tdep.c to something like amd64-linux-tdep.c, and register this
>> in the function amd64_linux_init_abi.
>>
>> Of course, this will mean that other System-V like targets would miss
>> out for now, but maybe the function itself could be renamed to
>> something like amd64_system_v_dwarf2_frame_init_reg, and left in
>> amd64-tdep.c, then from amd64-linux-tdep.c we can register that
>> function. And in future, other system-v like targets can also
>> register the function.
>
> I'd vote for this, thanks (also, John Baldwin agreed on this -- it is
> in a separate mail).
>
>>> +
>>> void
>>> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>>> const target_desc *default_tdesc)
>>> @@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info,
>>> struct gdbarch *gdbarch,
>>> set_gdbarch_stab_reg_to_regnum (gdbarch,
>>> amd64_dwarf_reg_to_regnum);
>>> set_gdbarch_dwarf2_reg_to_regnum (gdbarch,
>>> amd64_dwarf_reg_to_regnum);
>>>
>>> + /* Frame handling. */
>>> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
>>> +
>>> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
>>> be in use on any of the supported AMD64 targets. */
>>>
>>> diff --git a/gdb/frame.c b/gdb/frame.c
>>> index 2a899fc494f..da12ed36e02 100644
>>> --- a/gdb/frame.c
>>> +++ b/gdb/frame.c
>>> @@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info
>>> *this_frame)
>>> }
>>> catch (const gdb_exception_error &ex)
>>> {
>>> - if (ex.error == MEMORY_ERROR)
>>> + if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
>>> {
>>> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>>> + if (ex.error == MEMORY_ERROR)
>>> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>>> + else
>>> + /* This is for the OPTIMIZED_OUT_ERROR case. */
>>> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
>>> +
>>> if (ex.message != NULL)
>>> {
>>> char *stop_string;
>>> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>> b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>> index f9e590d83bb..6ae1c9c1322 100644
>>> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>> @@ -42,11 +42,11 @@ if ![runto breakpt] {
>>> return -1
>>> }
>>>
>>> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
>>> +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
>>> func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
>>> stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
>>> - "first backtrace, with error message"
>>> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
>>> +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
>>> func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
>>> stopped: value has been optimized out" \
>>> + "first backtrace, with error message"
>> I think the new "value has been optimized out" message is better than
>> what we had previously ("Cannot access memory at address 0x....")
>> which I guess was caused by trying to access through an unavailable
>> register.
>>
>> That said, I remember a long time ago there was a whole big discussion
>> about whether registers that were not saved should be described as
>> "optimized out" or not. In the end we added the "<not saved>" message
>> to cover this case.
>>
>> So I wonder if we should instead be ending the backtrace with
>> something like: "Backtrace stopped: value is not available"? I took
>> that exact text from require_available (value.c) but anything that
>> talks about available rather than optimized out might be better.
>>
>> Though I don't know how hard it would be to achieve that result. I
>> guess we'd need to start in dwarf2_frame_prev_register where we map
>> DWARF2_FRAME_REG_UNDEFINED onto optimized_out - maybe that needs to
>> map to an unavailable value instead, but that feels like it might have
>> huge knock on consequences ... we don't want optimized out variables
>> to suddenly start reporting themselves as unavailable instead...
>>
>> I don't know if others have any opinions on this, but it might be
>> worth having a short investigation to see how hard such a change would
>> be, I guess if it turns out to be a real pain then we could punt it
>> for later.
>
> I am with you there. Unfortunately, I won't be able to work on that now.
>
>>
>>> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
>>> +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
>>> func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
>>> stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
>>> - "second backtrace, with error message"
>>> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1
>>> +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in
>>> func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace
>>> stopped: value has been optimized out" \
>>> + "second backtrace, with error message"
>>>
>>> clean_restart ${binfile}
>>>
>>> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>>> b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>>> new file mode 100644
>>> index 00000000000..5698e549f00
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>>> @@ -0,0 +1,28 @@
>>> +/* This test program is part of GDB, the GNU debugger.
>>> +
>>> + Copyright 2019-2021 Free Software Foundation, Inc.
>> Should that copyright range just be 2021?
> Yep, thanks!
>>
>>> +
>>> + 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/>. */
>>> +
>>> +int
>>> +foo (int a)
>>> +{
>>> + return a; /* break here. */
>>> +}
>>> +
>>> +int
>>> +main ()
>>> +{
>>> + return foo (5);
>>> +}
>>> diff --git a/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>>> b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>>> new file mode 100644
>>> index 00000000000..07b870b5d30
>>> --- /dev/null
>>> +++ b/gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>>> @@ -0,0 +1,52 @@
>>> +# Copyright 2010-2021 Free Software Foundation, Inc.
>> I suspect the copyright range here is wrong too.
>>
>> For new files you should only have a from date in the copyright range
>> if the content of the file is almost completely copied unchanged from
>> some other, older, file. If the content in the new file is mostly
>> new, then you should just place this years date as the range:
>>
>> # 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/>.
>>> +
>>> +# Please email any bugs, comments, and/or additions to this file to:
>>> +# bug-gdb@gnu.org
>>> +
>>> +# This file is part of the gdb testsuite.
>>> +
>>> +if { ![istarget x86_64-*-* ]} {
>>> + verbose "Skipping ${gdb_test_file_name}."
>>> + return
>>> +}
>>> +
>>> +# Build program with debug symbols.
>>> +standard_testfile
>>> +set compile_flags {debug}
>>> +if { [prepare_for_testing "failed to prepare" ${testfile}
>>> ${srcfile} ${compile_flags}] } {
>>> + return -1
>>> +}
>>> +
>>> +if ![runto_main] {
>>> + untested "could not run to main"
>>> + return -1
>>> +}
>>> +
>>> +gdb_breakpoint [ gdb_get_line_number "break here" ]
>>> +gdb_continue_to_breakpoint "break here" ".*break here.*"
>>> +
>>> +# Switch to frame 1.
>>> +gdb_test "frame 1" "#1.*in main ().*"
>>> +
>>> +gdb_test "info registers rax" "rax .* <not saved>"
>>> +gdb_test "info registers rcx" "rcx .* <not saved>"
>>> +gdb_test "info registers rdx" "rdx .* <not saved>"
>>> +gdb_test "info registers rsi" "rsi .* <not saved>"
>>> +gdb_test "info registers rdi" "rdi .* <not saved>"
>>> +gdb_test "info registers r8" "r8 .* <not saved>"
>>> +gdb_test "info registers r9" "r9 .* <not saved>"
>>> +gdb_test "info registers r10" "r10 .* <not saved>"
>>> +gdb_test "info registers r11" "r11 .* <not saved>"
>>> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>>> b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>>> index 238830e711e..70f22cb9efe 100644
>>> --- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp
>>> @@ -50,11 +50,11 @@ gdb_test "bt" "#0 ($hex in )?breakpt
>>> \\(\\)\r\n#1 ($hex in )?int_param_single_
>>>
>>> # (2) struct_param_single_reg_loc
>>> gdb_continue_to_breakpoint "Stop in breakpt for
>>> struct_param_single_reg_loc"
>>> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in
>>> )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b
>>> = <optimized out>}, operand1={a = <optimized out>, b =
>>> 0xdeadbe04deadbe05}, operand2=<optimized out>\\)\r\n#2 ($hex in
>>> )?main \\(\\)" "backtrace for test struct_param_single_reg_loc"
>>> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in
>>> )?struct_param_single_reg_loc \\(operand0={a = 0xdeadbe00deadbe01, b
>>> = <optimized out>}, operand1=<optimized out>, operand2=<optimized
>>> out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace for test
>>> struct_param_single_reg_loc"
>>>
>>> # (3) struct_param_two_reg_pieces
>>> gdb_continue_to_breakpoint "Stop in breakpt for
>>> struct_param_two_reg_pieces"
>>> -gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in
>>> )?struct_param_two_reg_pieces \\(operand0={a = 0xdeadbe04deadbe05, b
>>> = <optimized out>}, operand1={a = <optimized out>, b =
>>> 0xdeadbe00deadbe01}, operand2=<optimized out>\\)\r\n#2 ($hex in
>>> )?main \\(\\)" "backtrace for test struct_param_two_reg_pieces"
>>> +gdb_test "bt" "#0 ($hex in )?breakpt \\(\\)\r\n#1 ($hex in
>>> )?struct_param_two_reg_pieces \\(operand0=<optimized out>,
>>> operand1={a = <optimized out>, b = 0xdeadbe00deadbe01},
>>> operand2=<optimized out>\\)\r\n#2 ($hex in )?main \\(\\)" "backtrace
>>> for test struct_param_two_reg_pieces"
>> I'm worried that this test has lost some of its meaning with this
>> change. What I think we need to do is modify the .S file that
>> accompanies this test to mark all of the registers that the ABI marks
>> as undefined, to DW_CFA_same_value. If you're not comfortable doing
>> that I'm happy to help once the other issues have been addressed.
> I am not very familiar with incorporating that info from asm level, so
> any help will be appreciated.
>>> # (4) int_param_two_reg_pieces
>>> gdb_continue_to_breakpoint "Stop in breakpt for
>>> int_param_two_reg_pieces"
>>> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>>> b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>>> index b1c28b2f41c..2b296c8d445 100644
>>> --- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
>>> @@ -47,8 +47,8 @@ for {set f 0} {$f < 3} {incr f} {
>>> } else {
>>> set pattern_rax_rbx_rcx_print "<not saved>"
>>> set pattern_rax_rbx_rcx_info "<not saved>"
>>> - set pattern_r8_r9_print "$hex"
>>> - set pattern_r8_r9_info "$hex\\s+$decimal"
>>> + set pattern_r8_r9_print "<not saved>"
>>> + set pattern_r8_r9_info "<not saved>"
>>> }
>> Again, I think we've lost something here. The original choice of
>> r8/r9 was arbitrary, and, at the time didn't make any difference as
>> all the registers were treated as "same value".
>>
>> I'd suggest that you add testing of r12/r13 here as they should show
>> the same behaviour that r8/r9 did before the patch.
> Oh yes. I've updated it automatically.
>>> # Select frame.
>>> @@ -79,8 +79,8 @@ for {set f 0} {$f < 3} {incr f} {
>>> # "not saved" and not "optimized out".
>>> gdb_test "set debug frame 1"
>>> gdb_test {print $rax} [multi_line \
>>> - { \[frame\] frame_unwind_register_value: frame=0,
>>> regnum=0\(rax\)} \
>>> - { \[frame\] frame_unwind_register_value: -> <not saved>} \
>>> + { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
>>> + { \[frame\] frame_unwind_register_value: -> <not saved>} \
>> As above, this change is fine for rax, but you should add a test for
>> rbx too, that register should behave just as rax used too before your
>> patch.
> Thanks!
>>> {.*}]
>>> gdb_test "set debug frame 0"
>>>
>>> diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>>> b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>>> index f38e20003a3..3f193b1379b 100644
>>> --- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>>> +++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
>>> @@ -62,10 +62,10 @@ for {set f 0} {$f < 3} {incr f} {
>>> }
>>>
>>> mi_gdb_test "22${f}-data-list-register-values --thread 1
>>> --frame ${f} x 0 1 2 8 9" \
>>> -
>>> "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]"
>>> \
>>> +
>>> "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]"
>>> \
>>> "register values, format x, frame ${f}"
>>>
>>> mi_gdb_test "33${f}-data-list-register-values --thread 1
>>> --frame ${f} r 0 1 2 8 9" \
>>> -
>>> "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]"
>>> \
>>> +
>>> "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"${pattern_0_1_2}\"\},\{number=\"9\",value=\"${pattern_0_1_2}\"\}\\\]"
>>> \
>>> "register values, format r, frame ${f}"
>> Like the above test, I think you need to extend what's being tested
>> here to cover registers that are similar to the old behaviour, I'd
>> suggest r12/r13 again.
>>
>> Also, you should probably rename pattern_0_1_2 as this was referring
>> to register 0, 1, and 2, but now is being used more widely.
>
> Sure, thanks!
>
>
>> Thanks,
>> Andrew
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-12-02 15:56 ` Andrew Burgess
2021-12-02 17:32 ` John Baldwin
2021-12-03 16:27 ` Djordje
@ 2021-12-03 17:45 ` Pedro Alves
2021-12-06 15:13 ` Andrew Burgess
2 siblings, 1 reply; 12+ messages in thread
From: Pedro Alves @ 2021-12-03 17:45 UTC (permalink / raw)
To: Andrew Burgess, Djordje Todorovic; +Cc: Nikola Tesic, gdb-patches, asowda
On 2021-12-02 15:56, Andrew Burgess via Gdb-patches wrote:
> * Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18 12:42:32 +0000]:
>
>> Hi Andrew,
>>
>> Please find the patch, rebased on top of your improvement.
>>
>> Thanks,
>> Djordje
>>
>> ---
>> gdb/amd64-tdep.c | 37 +++++++++++++
>> gdb/frame.c | 9 +++-
>> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
>> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
>> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
>> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
>> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
>> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
>> 8 files changed, 136 insertions(+), 14 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>> ?
>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
>> index 7c67359678b..a6b544145fb 100644
>> --- a/gdb/amd64-tdep.c
>> +++ b/gdb/amd64-tdep.c
>> @@ -25,6 +25,8 @@
>> #include "arch-utils.h"
>> #include "block.h"
>> #include "dummy-frame.h"
>> +#include "dwarf2.h"
>> +#include "dwarf2/frame.h"
>> #include "frame.h"
>> #include "frame-base.h"
>> #include "frame-unwind.h"
>> @@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
>> AMD64_RIP_REGNUM);
>> }
>>
>> +/* Implement the "init_reg" dwarf2_frame_ops method. */
>> +
>> +static void
>> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>> + struct dwarf2_frame_state_reg *reg,
>> + struct frame_info *this_frame)
>> +{
>> + switch (regnum)
>> + {
>> + case AMD64_RIP_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_FN;
>> + return;
>> +
>> + case AMD64_RSP_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_CFA;
>> + return;
>> +
>> + /* Caller-saved registers. */
>> + case AMD64_RAX_REGNUM:
>> + case AMD64_RDI_REGNUM:
>> + case AMD64_RSI_REGNUM:
>> + case AMD64_RDX_REGNUM:
>> + case AMD64_RCX_REGNUM:
>> + case AMD64_R8_REGNUM:
>> + case AMD64_R9_REGNUM:
>> + case AMD64_R10_REGNUM:
>> + case AMD64_R11_REGNUM:
>> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
>> + return;
>> + }
>> +}
>
> I believe that this is the System-V ABI, which is not the only ABI
> used on x86-64. From what I've read I believe that Windows uses a
> slightly different ABI, where $rsi and $rdi are not callee saved.
>
> I think that we might consider moving this function from the general
> amd64-tdep.c to something like amd64-linux-tdep.c, and register this
> in the function amd64_linux_init_abi.
>
> Of course, this will mean that other System-V like targets would miss
> out for now, but maybe the function itself could be renamed to
> something like amd64_system_v_dwarf2_frame_init_reg, and left in
> amd64-tdep.c, then from amd64-linux-tdep.c we can register that
> function. And in future, other system-v like targets can also
> register the function.
>
>> +
>> void
>> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>> const target_desc *default_tdesc)
>> @@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>>
>> + /* Frame handling. */
>> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
>> +
>> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
>> be in use on any of the supported AMD64 targets. */
>>
>> diff --git a/gdb/frame.c b/gdb/frame.c
>> index 2a899fc494f..da12ed36e02 100644
>> --- a/gdb/frame.c
>> +++ b/gdb/frame.c
>> @@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
>> }
>> catch (const gdb_exception_error &ex)
>> {
>> - if (ex.error == MEMORY_ERROR)
>> + if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
>> {
>> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>> + if (ex.error == MEMORY_ERROR)
>> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>> + else
>> + /* This is for the OPTIMIZED_OUT_ERROR case. */
>> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
Hmm? Nak on this part, it makes no sense to me, but I may be missing something.
Optimized out should never be converted to UNWIND_UNAVAILABLE.
The patch is missing a commit log describing it / providing a rationale,
so I'm kind of lost here. Could you add that, please?
>> +
>> if (ex.message != NULL)
>> {
>> char *stop_string;
>> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>> index f9e590d83bb..6ae1c9c1322 100644
>> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>> @@ -42,11 +42,11 @@ if ![runto breakpt] {
>> return -1
>> }
>>
>> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
>> - "first backtrace, with error message"
>> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
>> + "first backtrace, with error message"
>
> I think the new "value has been optimized out" message is better than
> what we had previously ("Cannot access memory at address 0x....")
> which I guess was caused by trying to access through an unavailable
> register.
>
> That said, I remember a long time ago there was a whole big discussion
> about whether registers that were not saved should be described as
> "optimized out" or not. In the end we added the "<not saved>" message
> to cover this case.
I don't recall a big discussion about it, but yes, an optimized out
register is presented as <not saved>, as that's what it means.
>
> So I wonder if we should instead be ending the backtrace with
> something like: "Backtrace stopped: value is not available"? I took
> that exact text from require_available (value.c) but anything that
> talks about available rather than optimized out might be better.
I would rather this said "register not saved" or something
around it. Maybe even mention the register name. That's because "unavailable"
has a different meaning from optimized out / not saved -- <unavailable> is for
when e.g., we're looking at a traceframe, and some register/memory we need to
display the value wasn't collected, it's not in the traceframe. Or a
similar thing with a trimmed corefile. Or e.g., the register exists in the machine,
but the kernel is running an older kernel missing a PTRACE op to get at the register.
I.e., it's for when the value actually exists, but we don't have a way to get at it.
>
> Though I don't know how hard it would be to achieve that result. I
> guess we'd need to start in dwarf2_frame_prev_register where we map
> DWARF2_FRAME_REG_UNDEFINED onto optimized_out - maybe that needs to
> map to an unavailable value instead, but that feels like it might have
> huge knock on consequences ... we don't want optimized out variables
> to suddenly start reporting themselves as unavailable instead...
Please don't.
>
> I don't know if others have any opinions on this, but it might be
> worth having a short investigation to see how hard such a change would
> be, I guess if it turns out to be a real pain then we could punt it
> for later.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-12-03 17:45 ` Pedro Alves
@ 2021-12-06 15:13 ` Andrew Burgess
2021-12-16 13:35 ` Djordje
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Burgess @ 2021-12-06 15:13 UTC (permalink / raw)
To: Pedro Alves; +Cc: Djordje Todorovic, Nikola Tesic, gdb-patches, asowda
* Pedro Alves <pedro@palves.net> [2021-12-03 17:45:04 +0000]:
> On 2021-12-02 15:56, Andrew Burgess via Gdb-patches wrote:
> > * Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18 12:42:32 +0000]:
> >
> >> Hi Andrew,
> >>
> >> Please find the patch, rebased on top of your improvement.
> >>
> >> Thanks,
> >> Djordje
> >>
> >> ---
> >> gdb/amd64-tdep.c | 37 +++++++++++++
> >> gdb/frame.c | 9 +++-
> >> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
> >> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
> >> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
> >> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
> >> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
> >> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
> >> 8 files changed, 136 insertions(+), 14 deletions(-)
> >> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
> >> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
> >> ?
> >> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> >> index 7c67359678b..a6b544145fb 100644
> >> --- a/gdb/amd64-tdep.c
> >> +++ b/gdb/amd64-tdep.c
> >> @@ -25,6 +25,8 @@
> >> #include "arch-utils.h"
> >> #include "block.h"
> >> #include "dummy-frame.h"
> >> +#include "dwarf2.h"
> >> +#include "dwarf2/frame.h"
> >> #include "frame.h"
> >> #include "frame-base.h"
> >> #include "frame-unwind.h"
> >> @@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
> >> AMD64_RIP_REGNUM);
> >> }
> >>
> >> +/* Implement the "init_reg" dwarf2_frame_ops method. */
> >> +
> >> +static void
> >> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
> >> + struct dwarf2_frame_state_reg *reg,
> >> + struct frame_info *this_frame)
> >> +{
> >> + switch (regnum)
> >> + {
> >> + case AMD64_RIP_REGNUM:
> >> + reg->how = DWARF2_FRAME_REG_FN;
> >> + return;
> >> +
> >> + case AMD64_RSP_REGNUM:
> >> + reg->how = DWARF2_FRAME_REG_CFA;
> >> + return;
> >> +
> >> + /* Caller-saved registers. */
> >> + case AMD64_RAX_REGNUM:
> >> + case AMD64_RDI_REGNUM:
> >> + case AMD64_RSI_REGNUM:
> >> + case AMD64_RDX_REGNUM:
> >> + case AMD64_RCX_REGNUM:
> >> + case AMD64_R8_REGNUM:
> >> + case AMD64_R9_REGNUM:
> >> + case AMD64_R10_REGNUM:
> >> + case AMD64_R11_REGNUM:
> >> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
> >> + return;
> >> + }
> >> +}
> >
> > I believe that this is the System-V ABI, which is not the only ABI
> > used on x86-64. From what I've read I believe that Windows uses a
> > slightly different ABI, where $rsi and $rdi are not callee saved.
> >
> > I think that we might consider moving this function from the general
> > amd64-tdep.c to something like amd64-linux-tdep.c, and register this
> > in the function amd64_linux_init_abi.
> >
> > Of course, this will mean that other System-V like targets would miss
> > out for now, but maybe the function itself could be renamed to
> > something like amd64_system_v_dwarf2_frame_init_reg, and left in
> > amd64-tdep.c, then from amd64-linux-tdep.c we can register that
> > function. And in future, other system-v like targets can also
> > register the function.
> >
> >> +
> >> void
> >> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> >> const target_desc *default_tdesc)
> >> @@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
> >> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> >> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
> >>
> >> + /* Frame handling. */
> >> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
> >> +
> >> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
> >> be in use on any of the supported AMD64 targets. */
> >>
> >> diff --git a/gdb/frame.c b/gdb/frame.c
> >> index 2a899fc494f..da12ed36e02 100644
> >> --- a/gdb/frame.c
> >> +++ b/gdb/frame.c
> >> @@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
> >> }
> >> catch (const gdb_exception_error &ex)
> >> {
> >> - if (ex.error == MEMORY_ERROR)
> >> + if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
> >> {
> >> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> >> + if (ex.error == MEMORY_ERROR)
> >> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
> >> + else
> >> + /* This is for the OPTIMIZED_OUT_ERROR case. */
> >> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
>
> Hmm? Nak on this part, it makes no sense to me, but I may be missing something.
> Optimized out should never be converted to UNWIND_UNAVAILABLE.
I had a misunderstanding about the meaning on unavailable within GDB
(see below), but given that, maybe we need another unwind stop reason,
UNWIND_OPTIMIZED_OUT? For when the information required to unwind has
been optimised away?
>
> The patch is missing a commit log describing it / providing a rationale,
> so I'm kind of lost here. Could you add that, please?
>
> >> +
> >> if (ex.message != NULL)
> >> {
> >> char *stop_string;
> >> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> >> index f9e590d83bb..6ae1c9c1322 100644
> >> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> >> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
> >> @@ -42,11 +42,11 @@ if ![runto breakpt] {
> >> return -1
> >> }
> >>
> >> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
> >> - "first backtrace, with error message"
> >> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
> >> + "first backtrace, with error message"
> >
> > I think the new "value has been optimized out" message is better than
> > what we had previously ("Cannot access memory at address 0x....")
> > which I guess was caused by trying to access through an unavailable
> > register.
> >
> > That said, I remember a long time ago there was a whole big discussion
> > about whether registers that were not saved should be described as
> > "optimized out" or not. In the end we added the "<not saved>" message
> > to cover this case.
>
> I don't recall a big discussion about it, but yes, an optimized out
> register is presented as <not saved>, as that's what it means.
>
> >
> > So I wonder if we should instead be ending the backtrace with
> > something like: "Backtrace stopped: value is not available"? I took
> > that exact text from require_available (value.c) but anything that
> > talks about available rather than optimized out might be better.
>
> I would rather this said "register not saved" or something
> around it. Maybe even mention the register name. That's because "unavailable"
> has a different meaning from optimized out / not saved -- <unavailable> is for
> when e.g., we're looking at a traceframe, and some register/memory we need to
> display the value wasn't collected, it's not in the traceframe. Or a
> similar thing with a trimmed corefile. Or e.g., the register exists in the machine,
> but the kernel is running an older kernel missing a PTRACE op to get at the register.
> I.e., it's for when the value actually exists, but we don't have a way to get at it.
>
> >
> > Though I don't know how hard it would be to achieve that result. I
> > guess we'd need to start in dwarf2_frame_prev_register where we map
> > DWARF2_FRAME_REG_UNDEFINED onto optimized_out - maybe that needs to
> > map to an unavailable value instead, but that feels like it might have
> > huge knock on consequences ... we don't want optimized out variables
> > to suddenly start reporting themselves as unavailable instead...
>
> Please don't.
No, that would be the wrong thing to do given the intended use of
unavailable. I knew unavailable was used for traceframes, but thought
that it was also used for registers that hadn't been saved between
frames. Given the explanation you gave above then using optimized out
for unsaved registers is what we expect.
Thanks for correcting me on this.
Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: gdb: Implement the init_reg dwarf2_frame_ops for amd64
2021-12-06 15:13 ` Andrew Burgess
@ 2021-12-16 13:35 ` Djordje
0 siblings, 0 replies; 12+ messages in thread
From: Djordje @ 2021-12-16 13:35 UTC (permalink / raw)
To: Andrew Burgess, Pedro Alves; +Cc: Nikola Tesic, gdb-patches, asowda
I am sending the updated patch (with the original commit message).
---
Please consider these two files:
$ cat test2.c
void f1(int);
int f2();
void f3();
void f() {
f1(0);
int local = f2();
f1(local);
f3();
}
$ cat main-for-core.c
#include <stdio.h>
void f1(int param) {
if (param % 2 == 0)
return;
printf("HELLO\n");
abort();
}
int f2() {
int x;
scanf("%d", &x);
return x;
}
void f3() {
int y;
scanf("%d", &y);
printf("%d\n", y);
}
int main() {
f();
return 0;
}
Compilation of the files:
$ clang -g -O2 test2.c -c -o object-file-test2.o
$ clang -g -O2 main-for-core.c object-file-test2.o -o main-for-core
By examining debug_loc from object-file-test2.o for the `local` var we see:
00000000 000000000000000f 0000000000000016 (DW_OP_reg0 (rax))
And the code for the f() is:
0000000000000000 <f>:
0: push %rax
1: xor %edi,%edi
3: callq 8 <f+0x8>
8: xor %eax,%eax
a: callq f <f+0xf>
f: mov %eax,%edi
11: callq 16 <f+0x16>
16: xor %eax,%eax
18: pop %rcx
19: jmpq 1e <f+0x1e>
While debugging it, by loading the core file generated due to abort() (as an input I typed 5):
$ gdb main-for-core
...
(gdb) bt
#0 0x00007fb2d7aeb377 in raise () from /lib64/libc.so.6
#1 0x00007fb2d7aeca68 in abort () from /lib64/libc.so.6
#2 0x0000000000401197 in f1 (param=<optimized out>) at main-for-core.c:7
#3 0x0000000000401216 in f () at test2.c:8
#4 0x00000000004011f8 in main () at main-for-core.c:23
(gdb) f 3
#3 0x0000000000401216 in f () at test2.c:8
8 f1(local);
(gdb) disassemble
Dump of assembler code for function f:
0x0000000000401200 <+0>: push %rax
0x0000000000401201 <+1>: xor %edi,%edi
0x0000000000401203 <+3>: callq 0x401180 <f1>
0x0000000000401208 <+8>: xor %eax,%eax
0x000000000040120a <+10>: callq 0x4011a0 <f2>
0x000000000040120f <+15>: mov %eax,%edi
0x0000000000401211 <+17>: callq 0x401180 <f1>
=> 0x0000000000401216 <+22>: xor %eax,%eax
0x0000000000401218 <+24>: pop %rcx
0x0000000000401219 <+25>: jmpq 0x4011c0 <f3>
End of assembler dump.
(gdb) p local
$1 = 0
But it should be reported as <optimized_out>, because the %eax was clobbered by the call.
GCC produces different high pc address to keep GDB working properly:
00000000 000000000000000f 0000000000000015 (DW_OP_reg0 (rax))
This patch fixes the problem inside GDB itself for X86_64 (System-V ABI) targets.
Bug:https://bugs.llvm.org/show_bug.cgi?id=49641
---
ChangeLog | 15 ++++++
gdb/amd64-linux-tdep.c | 4 ++
gdb/amd64-tdep.c | 33 ++++++++++++
gdb/amd64-tdep.h | 6 +++
gdb/frame.c | 9 +++-
.../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
.../gdb.arch/amd64-svr4-not-saved-regs.c | 28 ++++++++++
.../gdb.arch/amd64-svr4-not-saved-regs.exp | 52 +++++++++++++++++++
gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S | 12 +++--
.../gdb.dwarf2/dw2-reg-undefined.exp | 24 ++++-----
gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 12 ++---
11 files changed, 176 insertions(+), 27 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
create mode 100644 gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
diff --git a/ChangeLog b/ChangeLog
index 1aeef12e2d6..64b3bb6bc8a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2021-12-15 Djordje Todorovic <djordje.todorovic@syrmia.com>
+
+ * gdb/amd64-linux-tdep.c: Add dwarf2_frame_set_init_reg declaration.
+ * gdb/amd64-tdep.c(amd64_svr4_dwarf2_frame_init_reg): New.
+ * gdb/amd64-tdep.h(amd64_svr4_dwarf2_frame_init_reg): Likewise.
+ * gdb/frame.c(get_prev_frame_always): Handle output of the unavailable
+ registers when printing backtrace.
+ * gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp: Update.
+ * gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S: Likewise.
+ * gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp: Likewise.
+ * gdb/testsuite/gdb.mi/mi-reg-undefined.exp: Likewise.
+ * gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c: New.
+ * gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp: Likewise.
+
+
2021-10-29 Eli Zaretskii <eliz@gnu.org>
* gdb/doc/gdb.texinfo (Command Options): (Data): Document
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 817a197ceaa..dac8dfd2f1a 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -21,6 +21,7 @@
#include "defs.h"
#include "arch-utils.h"
#include "frame.h"
+#include "dwarf2/frame.h"
#include "gdbcore.h"
#include "regcache.h"
#include "osabi.h"
@@ -1835,6 +1836,9 @@ amd64_linux_init_abi_common(struct gdbarch_info info, struct gdbarch *gdbarch,
set_gdbarch_get_siginfo_type (gdbarch, x86_linux_get_siginfo_type);
set_gdbarch_report_signal_info (gdbarch, i386_linux_report_signal_info);
+
+ /* Frame handling. */
+ dwarf2_frame_set_init_reg (gdbarch, amd64_svr4_dwarf2_frame_init_reg);
}
static void
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 7c67359678b..1e3e68c0c04 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -25,6 +25,7 @@
#include "arch-utils.h"
#include "block.h"
#include "dummy-frame.h"
+#include "dwarf2/frame.h"
#include "frame.h"
#include "frame-base.h"
#include "frame-unwind.h"
@@ -3103,6 +3104,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
AMD64_RIP_REGNUM);
}
+/* System-V ABI for call-saved/clobbered register information. */
+
+void
+amd64_svr4_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame)
+{
+ switch (regnum)
+ {
+ case AMD64_RIP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_FN;
+ return;
+
+ case AMD64_RSP_REGNUM:
+ reg->how = DWARF2_FRAME_REG_CFA;
+ return;
+
+ /* Caller-saved registers. */
+ case AMD64_RAX_REGNUM:
+ case AMD64_RDI_REGNUM:
+ case AMD64_RSI_REGNUM:
+ case AMD64_RDX_REGNUM:
+ case AMD64_RCX_REGNUM:
+ case AMD64_R8_REGNUM:
+ case AMD64_R9_REGNUM:
+ case AMD64_R10_REGNUM:
+ case AMD64_R11_REGNUM:
+ reg->how = DWARF2_FRAME_REG_UNDEFINED;
+ return;
+ }
+}
+
void
amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
const target_desc *default_tdesc)
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 6faa399cebb..9bb09baad0b 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -109,6 +109,12 @@ extern void amd64_x32_init_abi (struct gdbarch_info info,
extern const struct target_desc *amd64_target_description (uint64_t xcr0,
bool segments);
+/* System-V ABI for call-saved/clobbered register information. */
+extern void
+amd64_svr4_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
+ struct dwarf2_frame_state_reg *reg,
+ struct frame_info *this_frame);
+
/* Fill register REGNUM in REGCACHE with the appropriate
floating-point or SSE register value from *FXSAVE. If REGNUM is
-1, do this for all registers. This function masks off any of the
diff --git a/gdb/frame.c b/gdb/frame.c
index 2a899fc494f..da12ed36e02 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
}
catch (const gdb_exception_error &ex)
{
- if (ex.error == MEMORY_ERROR)
+ if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
{
- this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ if (ex.error == MEMORY_ERROR)
+ this_frame->stop_reason = UNWIND_MEMORY_ERROR;
+ else
+ /* This is for the OPTIMIZED_OUT_ERROR case. */
+ this_frame->stop_reason = UNWIND_UNAVAILABLE;
+
if (ex.message != NULL)
{
char *stop_string;
diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
index f9e590d83bb..6ae1c9c1322 100644
--- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
+++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
@@ -42,11 +42,11 @@ if ![runto breakpt] {
return -1
}
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "first backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "first backtrace, with error message"
-gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
- "second backtrace, with error message"
+gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
+ "second backtrace, with error message"
clean_restart ${binfile}
diff --git a/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
new file mode 100644
index 00000000000..27a7bf36fe9
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.c
@@ -0,0 +1,28 @@
+/* This test program is part of GDB, the GNU debugger.
+
+ 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/>. */
+
+int
+foo (int a)
+{
+ return a; /* break here. */
+}
+
+int
+main ()
+{
+ return foo (5);
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
new file mode 100644
index 00000000000..78af000d0f7
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-svr4-not-saved-regs.exp
@@ -0,0 +1,52 @@
+# 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/>.
+
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
+
+# This file is part of the gdb testsuite.
+
+if { ![istarget x86_64*-linux* ]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+# Build program with debug symbols.
+standard_testfile
+set compile_flags {debug}
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} ${compile_flags}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint [ gdb_get_line_number "break here" ]
+gdb_continue_to_breakpoint "break here" ".*break here.*"
+
+# Switch to frame 1.
+gdb_test "frame 1" "#1.*in main ().*"
+
+gdb_test "info registers rax" "rax .* <not saved>"
+gdb_test "info registers rcx" "rcx .* <not saved>"
+gdb_test "info registers rdx" "rdx .* <not saved>"
+gdb_test "info registers rsi" "rsi .* <not saved>"
+gdb_test "info registers rdi" "rdi .* <not saved>"
+gdb_test "info registers r8" "r8 .* <not saved>"
+gdb_test "info registers r9" "r9 .* <not saved>"
+gdb_test "info registers r10" "r10 .* <not saved>"
+gdb_test "info registers r11" "r11 .* <not saved>"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
index 28445b4682c..c012c842c79 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
+++ b/gdb/testsuite/gdb.dwarf2/dw2-op-out-param.S
@@ -250,12 +250,18 @@ int_param_two_reg_pieces:
.quad .Ltext4 /* start */
.quad .Ltext5a-.Ltext4 /* length */
/* Instructions */
+ .byte 0x8 /* DW_CFA_same_value */
+ .uleb128 0x0 /* ULEB128 register */
+ .byte 0x8 /* DW_CFA_same_value */
+ .uleb128 0x1 /* ULEB128 register */
.byte 0x7 /* DW_CFA_undefined */
- .uleb128 0x2 /* reg# */
+ .uleb128 0x2 /* ULEB128 register */
+ .byte 0x8 /* DW_CFA_same_value */
+ .uleb128 0x3 /* ULEB128 register */
.byte 0x7 /* DW_CFA_undefined */
- .uleb128 0x4 /* reg# */
+ .uleb128 0x4 /* ULEB128 register */
.byte 0x7 /* DW_CFA_undefined */
- .uleb128 0x5 /* reg# */
+ .uleb128 0x5 /* ULEB128 register */
.align 8
.LEFDE0:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
index b1c28b2f41c..f4bb0ca1834 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-reg-undefined.exp
@@ -42,13 +42,13 @@ for {set f 0} {$f < 3} {incr f} {
if {${f} == 0} {
set pattern_rax_rbx_rcx_print "$hex"
set pattern_rax_rbx_rcx_info "$hex\\s+$decimal"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r12_r13_print "$hex"
+ set pattern_r12_r13_info "$hex\\s+$decimal"
} else {
set pattern_rax_rbx_rcx_print "<not saved>"
set pattern_rax_rbx_rcx_info "<not saved>"
- set pattern_r8_r9_print "$hex"
- set pattern_r8_r9_info "$hex\\s+$decimal"
+ set pattern_r12_r13_print "$hex"
+ set pattern_r12_r13_info "$hex\\s+$decimal"
}
# Select frame.
@@ -61,26 +61,26 @@ for {set f 0} {$f < 3} {incr f} {
gdb_test "p/x \$rcx" "$pattern_rax_rbx_rcx_print" \
"print \$rcx in frame ${f}"
- gdb_test "p/x \$r8" "$pattern_r8_r9_print" "print \$r8 in frame ${f}"
- gdb_test "p/x \$r9" "$pattern_r8_r9_print" "print \$r9 in frame ${f}"
+ gdb_test "p/x \$r12" "$pattern_r12_r13_print" "print \$r12 in frame ${f}"
+ gdb_test "p/x \$r13" "$pattern_r12_r13_print" "print \$r13 in frame ${f}"
# Display register values.
- gdb_test "info registers rax rbx rcx r8 r9" \
+ gdb_test "info registers rax rbx rcx r12 r13" \
[multi_line "rax\\s+${pattern_rax_rbx_rcx_info}\\s*" \
"rbx\\s+${pattern_rax_rbx_rcx_info}\\s*" \
"rcx\\s+${pattern_rax_rbx_rcx_info}\\s*" \
- "r8\\s+${pattern_r8_r9_info}\\s*" \
- "r9\\s+${pattern_r8_r9_info}\\s*"] \
- "Check values of rax, rbx, rcx, r8, r9 in frame ${f}"
+ "r12\\s+${pattern_r12_r13_info}\\s*" \
+ "r13\\s+${pattern_r12_r13_info}\\s*"] \
+ "Check values of rax, rbx, rcx, r12, r13 in frame ${f}"
}
# Test that the debug log statement in frame_unwind_register_value produces
# "not saved" and not "optimized out".
gdb_test "set debug frame 1"
gdb_test {print $rax} [multi_line \
- { \[frame\] frame_unwind_register_value: frame=0, regnum=0\(rax\)} \
- { \[frame\] frame_unwind_register_value: -> <not saved>} \
+ { \[frame\] frame_unwind_register_value: frame=1, regnum=0\(rax\)} \
+ { \[frame\] frame_unwind_register_value: -> <not saved>} \
{.*}]
gdb_test "set debug frame 0"
diff --git a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
index f38e20003a3..06d054ce8ac 100644
--- a/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
+++ b/gdb/testsuite/gdb.mi/mi-reg-undefined.exp
@@ -56,16 +56,16 @@ set not_saved_pattern "<not saved>"
for {set f 0} {$f < 3} {incr f} {
if {${f} == 0} {
- set pattern_0_1_2 ${hex}
+ set pattern_caller_saved_regs ${hex}
} else {
- set pattern_0_1_2 ${not_saved_pattern}
+ set pattern_caller_saved_regs ${not_saved_pattern}
}
- mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 8 9" \
- "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ mi_gdb_test "22${f}-data-list-register-values --thread 1 --frame ${f} x 0 1 2 12 13" \
+ "22${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"1\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"2\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"12\",value=\"$hex\"\},\{number=\"13\",value=\"$hex\"\}\]" \
"register values, format x, frame ${f}"
- mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2 8 9" \
- "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_0_1_2}\"\},\{number=\"1\",value=\"${pattern_0_1_2}\"\},\{number=\"2\",value=\"${pattern_0_1_2}\"\},\{number=\"8\",value=\"$hex\"\},\{number=\"9\",value=\"$hex\"\}\\\]" \
+ mi_gdb_test "33${f}-data-list-register-values --thread 1 --frame ${f} r 0 1 2" \
+ "33${f}\\^done,register-values=\\\[\{number=\"0\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"1\",value=\"${pattern_caller_saved_regs}\"\},\{number=\"2\",value=\"${pattern_caller_saved_regs}\"\}\]" \
"register values, format r, frame ${f}"
}
--
2.25.1
Thanks,
Djordje
On 6.12.21. 16:13, Andrew Burgess wrote:
> * Pedro Alves <pedro@palves.net> [2021-12-03 17:45:04 +0000]:
>
>> On 2021-12-02 15:56, Andrew Burgess via Gdb-patches wrote:
>>> * Djordje Todorovic <Djordje.Todorovic@syrmia.com> [2021-11-18 12:42:32 +0000]:
>>>
>>>> Hi Andrew,
>>>>
>>>> Please find the patch, rebased on top of your improvement.
>>>>
>>>> Thanks,
>>>> Djordje
>>>>
>>>> ---
>>>> gdb/amd64-tdep.c | 37 +++++++++++++
>>>> gdb/frame.c | 9 +++-
>>>> .../gdb.arch/amd64-invalid-stack-middle.exp | 8 +--
>>>> gdb/testsuite/gdb.arch/amd64-not-saved-regs.c | 28 ++++++++++
>>>> .../gdb.arch/amd64-not-saved-regs.exp | 52 +++++++++++++++++++
>>>> gdb/testsuite/gdb.dwarf2/dw2-op-out-param.exp | 4 +-
>>>> .../gdb.dwarf2/dw2-reg-undefined.exp | 8 +--
>>>> gdb/testsuite/gdb.mi/mi-reg-undefined.exp | 4 +-
>>>> 8 files changed, 136 insertions(+), 14 deletions(-)
>>>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.c
>>>> create mode 100644 gdb/testsuite/gdb.arch/amd64-not-saved-regs.exp
>>>> ?
>>>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
>>>> index 7c67359678b..a6b544145fb 100644
>>>> --- a/gdb/amd64-tdep.c
>>>> +++ b/gdb/amd64-tdep.c
>>>> @@ -25,6 +25,8 @@
>>>> #include "arch-utils.h"
>>>> #include "block.h"
>>>> #include "dummy-frame.h"
>>>> +#include "dwarf2.h"
>>>> +#include "dwarf2/frame.h"
>>>> #include "frame.h"
>>>> #include "frame-base.h"
>>>> #include "frame-unwind.h"
>>>> @@ -3103,6 +3105,38 @@ amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
>>>> AMD64_RIP_REGNUM);
>>>> }
>>>>
>>>> +/* Implement the "init_reg" dwarf2_frame_ops method. */
>>>> +
>>>> +static void
>>>> +amd64_dwarf2_frame_init_reg (struct gdbarch *gdbarch, int regnum,
>>>> + struct dwarf2_frame_state_reg *reg,
>>>> + struct frame_info *this_frame)
>>>> +{
>>>> + switch (regnum)
>>>> + {
>>>> + case AMD64_RIP_REGNUM:
>>>> + reg->how = DWARF2_FRAME_REG_FN;
>>>> + return;
>>>> +
>>>> + case AMD64_RSP_REGNUM:
>>>> + reg->how = DWARF2_FRAME_REG_CFA;
>>>> + return;
>>>> +
>>>> + /* Caller-saved registers. */
>>>> + case AMD64_RAX_REGNUM:
>>>> + case AMD64_RDI_REGNUM:
>>>> + case AMD64_RSI_REGNUM:
>>>> + case AMD64_RDX_REGNUM:
>>>> + case AMD64_RCX_REGNUM:
>>>> + case AMD64_R8_REGNUM:
>>>> + case AMD64_R9_REGNUM:
>>>> + case AMD64_R10_REGNUM:
>>>> + case AMD64_R11_REGNUM:
>>>> + reg->how = DWARF2_FRAME_REG_UNDEFINED;
>>>> + return;
>>>> + }
>>>> +}
>>> I believe that this is the System-V ABI, which is not the only ABI
>>> used on x86-64. From what I've read I believe that Windows uses a
>>> slightly different ABI, where $rsi and $rdi are not callee saved.
>>>
>>> I think that we might consider moving this function from the general
>>> amd64-tdep.c to something like amd64-linux-tdep.c, and register this
>>> in the function amd64_linux_init_abi.
>>>
>>> Of course, this will mean that other System-V like targets would miss
>>> out for now, but maybe the function itself could be renamed to
>>> something like amd64_system_v_dwarf2_frame_init_reg, and left in
>>> amd64-tdep.c, then from amd64-linux-tdep.c we can register that
>>> function. And in future, other system-v like targets can also
>>> register the function.
>>>
>>>> +
>>>> void
>>>> amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>>>> const target_desc *default_tdesc)
>>>> @@ -3217,6 +3251,9 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
>>>> set_gdbarch_stab_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>>>> set_gdbarch_dwarf2_reg_to_regnum (gdbarch, amd64_dwarf_reg_to_regnum);
>>>>
>>>> + /* Frame handling. */
>>>> + dwarf2_frame_set_init_reg (gdbarch, amd64_dwarf2_frame_init_reg);
>>>> +
>>>> /* We don't override SDB_REG_RO_REGNUM, since COFF doesn't seem to
>>>> be in use on any of the supported AMD64 targets. */
>>>>
>>>> diff --git a/gdb/frame.c b/gdb/frame.c
>>>> index 2a899fc494f..da12ed36e02 100644
>>>> --- a/gdb/frame.c
>>>> +++ b/gdb/frame.c
>>>> @@ -2315,9 +2315,14 @@ get_prev_frame_always (struct frame_info *this_frame)
>>>> }
>>>> catch (const gdb_exception_error &ex)
>>>> {
>>>> - if (ex.error == MEMORY_ERROR)
>>>> + if (ex.error == MEMORY_ERROR || ex.error == OPTIMIZED_OUT_ERROR)
>>>> {
>>>> - this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>>>> + if (ex.error == MEMORY_ERROR)
>>>> + this_frame->stop_reason = UNWIND_MEMORY_ERROR;
>>>> + else
>>>> + /* This is for the OPTIMIZED_OUT_ERROR case. */
>>>> + this_frame->stop_reason = UNWIND_UNAVAILABLE;
>> Hmm? Nak on this part, it makes no sense to me, but I may be missing something.
>> Optimized out should never be converted to UNWIND_UNAVAILABLE.
> I had a misunderstanding about the meaning on unavailable within GDB
> (see below), but given that, maybe we need another unwind stop reason,
> UNWIND_OPTIMIZED_OUT? For when the information required to unwind has
> been optimised away?
>
>> The patch is missing a commit log describing it / providing a rationale,
>> so I'm kind of lost here. Could you add that, please?
>>
>>>> +
>>>> if (ex.message != NULL)
>>>> {
>>>> char *stop_string;
>>>> diff --git a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>>> index f9e590d83bb..6ae1c9c1322 100644
>>>> --- a/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>>> +++ b/gdb/testsuite/gdb.arch/amd64-invalid-stack-middle.exp
>>>> @@ -42,11 +42,11 @@ if ![runto breakpt] {
>>>> return -1
>>>> }
>>>>
>>>> -gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: Cannot access memory at address 0x\[0-9a-f\]+" \
>>>> - "first backtrace, with error message"
>>>> +gdb_test "bt" "^bt\r\n#0 +breakpt *\\(\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in func5\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in func4\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in func3\[^\r\n\]*\r\nBacktrace stopped: value has been optimized out" \
>>>> + "first backtrace, with error message"
>>> I think the new "value has been optimized out" message is better than
>>> what we had previously ("Cannot access memory at address 0x....")
>>> which I guess was caused by trying to access through an unavailable
>>> register.
>>>
>>> That said, I remember a long time ago there was a whole big discussion
>>> about whether registers that were not saved should be described as
>>> "optimized out" or not. In the end we added the "<not saved>" message
>>> to cover this case.
>> I don't recall a big discussion about it, but yes, an optimized out
>> register is presented as <not saved>, as that's what it means.
>>
>>> So I wonder if we should instead be ending the backtrace with
>>> something like: "Backtrace stopped: value is not available"? I took
>>> that exact text from require_available (value.c) but anything that
>>> talks about available rather than optimized out might be better.
>> I would rather this said "register not saved" or something
>> around it. Maybe even mention the register name. That's because "unavailable"
>> has a different meaning from optimized out / not saved -- <unavailable> is for
>> when e.g., we're looking at a traceframe, and some register/memory we need to
>> display the value wasn't collected, it's not in the traceframe. Or a
>> similar thing with a trimmed corefile. Or e.g., the register exists in the machine,
>> but the kernel is running an older kernel missing a PTRACE op to get at the register.
>> I.e., it's for when the value actually exists, but we don't have a way to get at it.
>>
>>> Though I don't know how hard it would be to achieve that result. I
>>> guess we'd need to start in dwarf2_frame_prev_register where we map
>>> DWARF2_FRAME_REG_UNDEFINED onto optimized_out - maybe that needs to
>>> map to an unavailable value instead, but that feels like it might have
>>> huge knock on consequences ... we don't want optimized out variables
>>> to suddenly start reporting themselves as unavailable instead...
>> Please don't.
> No, that would be the wrong thing to do given the intended use of
> unavailable. I knew unavailable was used for traceframes, but thought
> that it was also used for registers that hadn't been saved between
> frames. Given the explanation you gave above then using optimized out
> for unsaved registers is what we expect.
>
> Thanks for correcting me on this.
>
> Andrew
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-16 13:35 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 12:20 gdb: Implement the init_reg dwarf2_frame_ops for amd64 Djordje
2021-11-09 13:35 ` Andrew Burgess
2021-11-15 9:06 ` Djordje Todorovic
2021-11-18 12:42 ` Djordje Todorovic
2021-12-02 10:07 ` Djordje Todorovic
2021-12-02 15:56 ` Andrew Burgess
2021-12-02 17:32 ` John Baldwin
2021-12-03 16:27 ` Djordje
2021-12-03 16:29 ` Djordje
2021-12-03 17:45 ` Pedro Alves
2021-12-06 15:13 ` Andrew Burgess
2021-12-16 13:35 ` Djordje
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).