[PATCH v3] gdb: register frame_destroyed function for amd64 gdbarch

[Guinevere Larsen <blarsen@redhat.com>]

gdbarches usually register functions to check when a frame is destroyed
which is used with software watchpoints, since the expression of the
watchpoint is no longer vlaid at this point.  On amd64, this wasn't done
anymore because GCC started using CFA for variable locations instead.

However, clang doesn't use the CFA and instead relies on specifying when
an epilogue has started, meaning software watchpoints get a spurious hit
when a frame is destroyed. This patch re-adds the code to register the
function that detects when a frame is destroyed, but only uses this when
the producer is LLVM, so gcc code isn't affected. The logic that
identifies the epilogue has been factored out into the new function
amd64_stack_frame_destroyed_p_1, so the frame sniffer can call it
directly, and its behavior isn't changed.

This can also remove the XFAIL added to gdb.python/pq-watchpoint tests
that handled this exact flaw in clang

Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
---
 gdb/amd64-tdep.c                           | 32 ++++++++++++++++------
 gdb/testsuite/gdb.python/py-watchpoint.exp | 17 +-----------
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index e6feee677b3..2e101b4fca1 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -2886,6 +2886,22 @@ static const struct frame_base amd64_frame_base =
   amd64_frame_base_address
 };
 
+/* Implement core of the stack_frame_destroyed_p gdbarch method.  */
+
+static int
+amd64_stack_frame_destroyed_p_1 (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  gdb_byte insn;
+
+  if (target_read_memory (pc, &insn, 1))
+    return 0;   /* Can't read memory at pc.  */
+
+  if (insn != 0xc3)     /* 'ret' instruction.  */
+    return 0;
+
+  return 1;
+}
+
 /* Normal frames, but in a function epilogue.  */
 
 /* Implement the stack_frame_destroyed_p gdbarch method.
@@ -2897,15 +2913,13 @@ static const struct frame_base amd64_frame_base =
 static int
 amd64_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  gdb_byte insn;
+  struct compunit_symtab *cust = find_pc_compunit_symtab (pc);
 
-  if (target_read_memory (pc, &insn, 1))
-    return 0;   /* Can't read memory at pc.  */
+  if (cust != nullptr && cust->producer () != nullptr
+      && producer_is_llvm (cust->producer ()))
+    return amd64_stack_frame_destroyed_p_1 (gdbarch, pc);
 
-  if (insn != 0xc3)     /* 'ret' instruction.  */
-    return 0;
-
-  return 1;
+  return 0;
 }
 
 static int
@@ -2938,7 +2952,7 @@ amd64_epilogue_frame_sniffer_1 (const struct frame_unwind *self,
     }
 
   /* Check whether we're in an epilogue.  */
-  return amd64_stack_frame_destroyed_p (gdbarch, pc);
+  return amd64_stack_frame_destroyed_p_1 (gdbarch, pc);
 }
 
 static int
@@ -3310,6 +3324,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
 
   set_gdbarch_gen_return_address (gdbarch, amd64_gen_return_address);
 
+  set_gdbarch_stack_frame_destroyed_p (gdbarch, amd64_stack_frame_destroyed_p);
+
   /* SystemTap variables and functions.  */
   set_gdbarch_stap_integer_prefixes (gdbarch, stap_integer_prefixes);
   set_gdbarch_stap_register_prefixes (gdbarch, stap_register_prefixes);
diff --git a/gdb/testsuite/gdb.python/py-watchpoint.exp b/gdb/testsuite/gdb.python/py-watchpoint.exp
index 5ff61285979..9a6ef447572 100644
--- a/gdb/testsuite/gdb.python/py-watchpoint.exp
+++ b/gdb/testsuite/gdb.python/py-watchpoint.exp
@@ -42,20 +42,5 @@ gdb_test "source $pyfile" ".*Python script imported.*" \
     "import python scripts"
 gdb_test "python print(len(gdb.breakpoints()))" "2" "check modified BP count"
 gdb_test "continue" ".*" "run until program stops"
-# Clang doesn't use CFA location information for variables (despite generating
-# them), meaning when the instruction "pop rbp" happens, we get a false hit
-# on the watchpoint. for more details, see:
-# https://github.com/llvm/llvm-project/issues/64390
-gdb_test_multiple "python print(bpt.n)" "check watchpoint hits" {
-    -re -wrap "5" {
-	pass $gdb_test_name
-    }
-    -re -wrap "6" {
-	if {[test_compiler_info "clang-*"]} {
-	    xfail "$gdb_test_name (clang issue 64390)"
-	} else {
-	    fail $gdb_test_name
-	}
-    }
-}
+gdb_test "python print(bpt.n)" "5" "check watchpoint hits"
 gdb_test "python print(len(gdb.breakpoints()))" "1" "check BP count"
-- 
2.41.0