[PATCH 2/3] Handle erroneous DW_AT_call_return_pc

[Tom Tromey <tromey@adacore.com>]

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 <pck__create_large>
   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

I'm not completely sure that the nop check in the new arch hook is
correct; but if it is incorrect it just means that some invocations of
'finish' won't find a value -- which is what happens without the
patch.
---
 gdb/arch-utils.c          |  6 ++++++
 gdb/gdbarch-gen.h         | 13 +++++++++++++
 gdb/gdbarch.c             | 22 ++++++++++++++++++++++
 gdb/gdbarch_components.py | 17 +++++++++++++++++
 gdb/rs6000-tdep.c         | 22 ++++++++++++++++++++++
 gdb/symtab.c              | 13 +++++++++++++
 6 files changed, 93 insertions(+)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e3af9ce2dbc..f9d0e73a362 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -1098,6 +1098,12 @@ default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame)
   return 0;
 }
 
+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 76d12a15317..bec3221202c 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -468,6 +468,19 @@ typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, fr
 extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame);
 extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr);
 
+/* 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 b4763aa6bf4..b4ee88b61cb 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -114,6 +114,7 @@ struct gdbarch
   gdbarch_return_value_ftype *return_value = nullptr;
   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_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;
@@ -370,6 +371,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   if ((gdbarch->return_value_as_value == default_gdbarch_return_value) == (gdbarch->return_value == nullptr))
     log.puts ("\n\treturn_value_as_value");
   /* Skip verify of get_return_buf_addr, 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");
@@ -787,6 +789,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
 	      "gdbarch_dump: get_return_buf_addr = <%s>\n",
 	      host_address_to_string (gdbarch->get_return_buf_addr));
+  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));
@@ -2619,6 +2624,23 @@ set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch,
   gdbarch->get_return_buf_addr = get_return_buf_addr;
 }
 
+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 92c501d2a72..8b5512c647e 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -901,6 +901,23 @@ May return 0 when unable to determine that address.""",
     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 9859a7d8b85..d1a16b3430b 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -7460,6 +7460,27 @@ 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'.  If PC points to a nop, return the
+     following instruction instead.  */
+
+  unsigned long op = rs6000_fetch_instruction (gdbarch, pc);
+  if (op == 0x60000000)
+    pc += 4;
+  return pc;
+}
+
 /* 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.
@@ -8246,6 +8267,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 e11f9262a22..219a1fd4671 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.39.1