gdb: Implement the init_reg dwarf2_frame_ops for amd64
Djordje
djordje.todorovic@syrmia.com
Thu Dec 16 13:35:32 GMT 2021
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
>
More information about the Gdb-patches
mailing list