From 9df25c346f5517c0228d067c68ee2f6bfe1728ad Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 8 Mar 2023 10:58:35 -0700 Subject: [PATCH] Handle erroneous DW_AT_call_return_pc On PPC64, with the test case included in an earlier patch, we found that "finish" would still not correctly find the return value via entry values. The issue is simple. The compiler emits: 0x00000000100032b8 <+28>: bl 0x1000320c 0x00000000100032bc <+32>: nop 0x00000000100032c0 <+36>: li r9,42 ... but the DWARF says: <162a> DW_AT_call_return_pc: 0x100032c0 That is, the declared return PC is one instruction past the actual return PC. This patch adds a new arch hook to handle this scenario, and implements it for PPC64. Some care is taken so that GDB will continue to work if this compiler bug is fixed. A GCC patch is here: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613336.html No check for 'nop' is done, as subsequent discussion revealed that the linker might replace this with another instruction. --- gdb/arch-utils.c | 6 ++++++ gdb/gdbarch-gen.h | 13 +++++++++++++ gdb/gdbarch.c | 22 ++++++++++++++++++++++ gdb/gdbarch_components.py | 17 +++++++++++++++++ gdb/rs6000-tdep.c | 19 +++++++++++++++++++ gdb/symtab.c | 13 +++++++++++++ 6 files changed, 90 insertions(+) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 997a292e9ef..1de5981765a 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -1105,6 +1105,12 @@ default_dwarf2_omit_typedef_p (struct type *target_type, const char *producer, return false; } +static CORE_ADDR +default_update_call_site_pc (struct gdbarch *gdbarch, CORE_ADDR pc) +{ + return pc; +} + /* Non-zero if we want to trace architecture code. */ #ifndef GDBARCH_DEBUG diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index 56b0a917168..7f83bf4e214 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -476,6 +476,19 @@ typedef bool (gdbarch_dwarf2_omit_typedef_p_ftype) (struct type *target_type, co extern bool gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch, struct type *target_type, const char *producer, const char *name); extern void set_gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch, gdbarch_dwarf2_omit_typedef_p_ftype *dwarf2_omit_typedef_p); +/* Update PC when trying to find a call site. This is useful on + architectures where the call site PC, as reported in the DWARF, can be + incorrect for some reason. + + The passed-in PC will be an address in the inferior. GDB will have + already failed to find a call site at this PC. This function may + simply return its parameter if it thinks that should be the correct + address. */ + +typedef CORE_ADDR (gdbarch_update_call_site_pc_ftype) (struct gdbarch *gdbarch, CORE_ADDR pc); +extern CORE_ADDR gdbarch_update_call_site_pc (struct gdbarch *gdbarch, CORE_ADDR pc); +extern void set_gdbarch_update_call_site_pc (struct gdbarch *gdbarch, gdbarch_update_call_site_pc_ftype *update_call_site_pc); + /* Return true if the return value of function is stored in the first hidden parameter. In theory, this feature should be language-dependent, specified by language and its ABI, such as C++. Unfortunately, compiler may diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 9406458189f..30199a0b03d 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -115,6 +115,7 @@ struct gdbarch gdbarch_return_value_as_value_ftype *return_value_as_value = default_gdbarch_return_value; gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = default_get_return_buf_addr; gdbarch_dwarf2_omit_typedef_p_ftype *dwarf2_omit_typedef_p = default_dwarf2_omit_typedef_p; + gdbarch_update_call_site_pc_ftype *update_call_site_pc = default_update_call_site_pc; gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = default_return_in_first_hidden_param_p; gdbarch_skip_prologue_ftype *skip_prologue = nullptr; gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; @@ -372,6 +373,7 @@ verify_gdbarch (struct gdbarch *gdbarch) log.puts ("\n\treturn_value_as_value"); /* Skip verify of get_return_buf_addr, invalid_p == 0 */ /* Skip verify of dwarf2_omit_typedef_p, invalid_p == 0 */ + /* Skip verify of update_call_site_pc, invalid_p == 0 */ /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */ if (gdbarch->skip_prologue == 0) log.puts ("\n\tskip_prologue"); @@ -793,6 +795,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) gdb_printf (file, "gdbarch_dump: dwarf2_omit_typedef_p = <%s>\n", host_address_to_string (gdbarch->dwarf2_omit_typedef_p)); + gdb_printf (file, + "gdbarch_dump: update_call_site_pc = <%s>\n", + host_address_to_string (gdbarch->update_call_site_pc)); gdb_printf (file, "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n", host_address_to_string (gdbarch->return_in_first_hidden_param_p)); @@ -2639,6 +2644,23 @@ set_gdbarch_dwarf2_omit_typedef_p (struct gdbarch *gdbarch, gdbarch->dwarf2_omit_typedef_p = dwarf2_omit_typedef_p; } +CORE_ADDR +gdbarch_update_call_site_pc (struct gdbarch *gdbarch, CORE_ADDR pc) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->update_call_site_pc != NULL); + if (gdbarch_debug >= 2) + gdb_printf (gdb_stdlog, "gdbarch_update_call_site_pc called\n"); + return gdbarch->update_call_site_pc (gdbarch, pc); +} + +void +set_gdbarch_update_call_site_pc (struct gdbarch *gdbarch, + gdbarch_update_call_site_pc_ftype update_call_site_pc) +{ + gdbarch->update_call_site_pc = update_call_site_pc; +} + int gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type) { diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py index 62cd430cbae..07010b13311 100644 --- a/gdb/gdbarch_components.py +++ b/gdb/gdbarch_components.py @@ -927,6 +927,23 @@ Return 0 by default""", invalid=False, ) +Method( + comment=""" +Update PC when trying to find a call site. This is useful on +architectures where the call site PC, as reported in the DWARF, can be +incorrect for some reason. + +The passed-in PC will be an address in the inferior. GDB will have +already failed to find a call site at this PC. This function may +simply return its parameter if it thinks that should be the correct +address.""", + type="CORE_ADDR", + name="update_call_site_pc", + params=[("CORE_ADDR", "pc")], + predefault="default_update_call_site_pc", + invalid=False, +) + Method( comment=""" Return true if the return value of function is stored in the first hidden diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index afeea02a84d..ee18b495d4c 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -7471,6 +7471,24 @@ rs6000_program_breakpoint_here_p (gdbarch *gdbarch, CORE_ADDR address) return false; } +/* Implement the update_call_site_pc arch hook. */ + +static CORE_ADDR +ppc64_update_call_site_pc (struct gdbarch *gdbarch, CORE_ADDR pc) +{ + /* Some versions of GCC emit: + + . bl function + . nop + . ... + + but emit DWARF where the DW_AT_call_return_pc points to + instruction after the 'nop'. Note that while the compiler emits + a 'nop', the linker might put some other instruction there -- so + we just unconditionally check the next instruction. */ + return pc + 4; +} + /* Initialize the current architecture based on INFO. If possible, re-use an architecture from ARCHES, which is a list of architectures already created during this debugging session. @@ -8257,6 +8275,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); set_gdbarch_get_return_buf_addr (gdbarch, ppc64_sysv_get_return_buf_addr); + set_gdbarch_update_call_site_pc (gdbarch, ppc64_update_call_site_pc); } else set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); diff --git a/gdb/symtab.c b/gdb/symtab.c index 9e9798676cb..6b6905d5489 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -349,6 +349,19 @@ compunit_symtab::find_call_site (CORE_ADDR pc) const struct call_site call_site_local (unrelocated_pc, nullptr, nullptr); void **slot = htab_find_slot (m_call_site_htab, &call_site_local, NO_INSERT); + if (slot != nullptr) + return (call_site *) *slot; + + /* See if the arch knows another PC we should try. On some + platforms, GCC emits a DWARF call site that is offset from the + actual return location. */ + struct gdbarch *arch = objfile ()->arch (); + CORE_ADDR new_pc = gdbarch_update_call_site_pc (arch, pc); + if (pc == new_pc) + return nullptr; + + call_site new_call_site_local (new_pc - delta, nullptr, nullptr); + slot = htab_find_slot (m_call_site_htab, &new_call_site_local, NO_INSERT); if (slot == nullptr) return nullptr; -- 2.30.2