[binutils-gdb/gdb-15-branch] [gdb/tdep] Fix gdb.base/watchpoint-running.exp on {arm,ppc64le}-linux

[binutils-gdb/gdb-15-branch] [gdb/tdep] Fix gdb.base/watchpoint-running.exp on {arm,ppc64le}-linux

[Tom de Vries]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f33b87956ffa9d153d39caf625b35f29597f0326

commit f33b87956ffa9d153d39caf625b35f29597f0326
Author: Pedro Alves <pedro@palves.net>
Date:   Fri Jun 21 16:46:50 2024 +0200

    [gdb/tdep] Fix gdb.base/watchpoint-running.exp on {arm,ppc64le}-linux
    
    When running test-case gdb.base/watchpoint-running on ppc64le-linux (and
    similar on arm-linux), we get:
    ...
    (gdb) watch global_var^M
    warning: Error when detecting the debug register interface. \
      Debug registers will be unavailable.^M
    Watchpoint 2: global_var^M
    (gdb) FAIL: $exp: all-stop: hardware: watch global_var
    FAIL: $exp: all-stop: hardware: watchpoint hit (timeout)
    ...
    
    The problem is that ppc_linux_dreg_interface::detect fails to detect the
    hardware watchpoint interface, because the calls to ptrace return with errno
    set to ESRCH.
    
    This is a feature of ptrace: if a call is done while the tracee is not
    ptrace-stopped, it returns ESRCH.
    
    Indeed, in the test-case "watch global_var" is executed while the inferior is
    running, and that triggers the first call to ppc_linux_dreg_interface::detect.
    
    And because the detection failure is cached, subsequent attempts at setting
    hardware watchpoints will also fail, even if the tracee is ptrace-stopped.
    
    The way to fix this is to make sure that ppc_linux_dreg_interface::detect is
    called when we know that the thread is ptrace-stopped, which in the current
    setup is best addressed by using target-specific post_attach and
    post_startup_inferior overrides.  However, as we can see in
    aarch64_linux_nat_target, that causes code duplication.
    
    Fix this by:
    - defining a new target hook low_init_process, called from
      linux_init_ptrace_procfs, which is called from both
      linux_nat_target::post_attach and linux_nat_target::post_startup_inferior,
    - adding implementations for ppc_linux_nat_target and arm_linux_nat_target
      that detect the hardware watchpoint interface,
    - replacing the aarch64_linux_nat_target implementations of post_attach and
      post_startup_inferior with a low_init_process implementation.
    
    Tested on ppc64le-linux, arm-linux, aarch64-linux and x86_64-linux.
    
    Co-Authored-By: Tom de Vries <tdevries@suse.de>
    Approved-By: Luis Machado <luis.machado@arm.com>
    
    PR tdep/31834
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31834
    PR tdep/31705
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31705
    
    (cherry picked from commit 50de502a4f843310e231b3174804e95a9e7de4fc)

Diff:
---
 gdb/aarch64-linux-nat.c | 32 ++++++++------------------------
 gdb/arm-linux-nat.c     | 14 ++++++++++++++
 gdb/linux-nat.c         |  6 ++++++
 gdb/linux-nat.h         |  6 ++++++
 gdb/ppc-linux-nat.c     | 15 +++++++++++++++
 5 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 4b2a0ba9f7b..2e6541f53c3 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -78,12 +78,6 @@ class aarch64_linux_nat_target final
 
   int can_do_single_step () override;
 
-  /* Override the GNU/Linux inferior startup hook.  */
-  void post_startup_inferior (ptid_t) override;
-
-  /* Override the GNU/Linux post attach hook.  */
-  void post_attach (int pid) override;
-
   /* These three defer to common nat/ code.  */
   void low_new_thread (struct lwp_info *lp) override
   { aarch64_linux_new_thread (lp); }
@@ -93,6 +87,7 @@ class aarch64_linux_nat_target final
   { aarch64_linux_prepare_to_resume (lp); }
 
   void low_new_fork (struct lwp_info *parent, pid_t child_pid) override;
+  void low_init_process (pid_t pid) override;
   void low_forget_process (pid_t pid) override;
 
   /* Add our siginfo layout converter.  */
@@ -844,29 +839,18 @@ ps_get_thread_area (struct ps_prochandle *ph,
 }
 \f
 
-/* Implement the virtual inf_ptrace_target::post_startup_inferior method.  */
-
-void
-aarch64_linux_nat_target::post_startup_inferior (ptid_t ptid)
-{
-  low_forget_process (ptid.pid ());
-  aarch64_linux_get_debug_reg_capacity (ptid.pid ());
-  linux_nat_target::post_startup_inferior (ptid);
-}
-
-/* Implement the "post_attach" target_ops method.  */
+/* Implement the "low_init_process" target_ops method.  */
 
 void
-aarch64_linux_nat_target::post_attach (int pid)
+aarch64_linux_nat_target::low_init_process (pid_t pid)
 {
   low_forget_process (pid);
-  /* Set the hardware debug register capacity.  If
-     aarch64_linux_get_debug_reg_capacity is not called
-     (as it is in aarch64_linux_child_post_startup_inferior) then
-     software watchpoints will be used instead of hardware
-     watchpoints when attaching to a target.  */
+  /* Set the hardware debug register capacity.  This requires the process to be
+     ptrace-stopped, otherwise detection will fail and software watchpoints will
+     be used instead of hardware.  If we allow this to be done lazily, we
+     cannot guarantee that it's called when the process is ptrace-stopped, so
+     do it now.  */
   aarch64_linux_get_debug_reg_capacity (pid);
-  linux_nat_target::post_attach (pid);
 }
 
 /* Implement the "read_description" target_ops method.  */
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 50c24ecfcd2..ac53bed72d7 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -103,6 +103,7 @@ class arm_linux_nat_target final : public linux_nat_target
 
   /* Handle process creation and exit.  */
   void low_new_fork (struct lwp_info *parent, pid_t child_pid) override;
+  void low_init_process (pid_t pid) override;
   void low_forget_process (pid_t pid) override;
 };
 
@@ -805,6 +806,19 @@ arm_linux_process_info_get (pid_t pid)
   return proc;
 }
 
+/* Implement the "low_init_process" target_ops method.  */
+
+void
+arm_linux_nat_target::low_init_process (pid_t pid)
+{
+  /* Set the hardware debug register capacity.  This requires the process to be
+     ptrace-stopped, otherwise detection will fail and software watchpoints will
+     be used instead of hardware.  If we allow this to be done lazily, we
+     cannot guarantee that it's called when the process is ptrace-stopped, so
+     do it now.  */
+  arm_linux_get_hwbp_cap ();
+}
+
 /* Called whenever GDB is no longer debugging process PID.  It deletes
    data structures that keep track of debug register state.  */
 
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 48ecd3627ca..218593c23cc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -454,6 +454,12 @@ linux_init_ptrace_procfs (pid_t pid, int attached)
   linux_ptrace_init_warnings ();
   linux_proc_init_warnings ();
   proc_mem_file_is_writable ();
+
+  /* Let the arch-specific native code do any needed initialization.
+     Some architectures need to call ptrace to check for hardware
+     watchpoints support, etc.  Call it now, when we know the tracee
+     is ptrace-stopped.  */
+  linux_target->low_init_process (pid);
 }
 
 linux_nat_target::~linux_nat_target ()
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 4dcbe9e170a..e3cefff97e3 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -164,6 +164,12 @@ class linux_nat_target : public inf_ptrace_target
   virtual void low_new_clone (struct lwp_info *parent, pid_t child_lwp)
   {}
 
+  /* The method to call, if any, when we have a new (from run/attach,
+     not fork) process to debug.  The process is ptrace-stopped when
+     this is called.  */
+  virtual void low_init_process (pid_t pid)
+  {}
+
   /* The method to call, if any, when a process is no longer
      attached.  */
   virtual void low_forget_process (pid_t pid)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index c73c7c90b4c..40a5665a18f 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -545,6 +545,8 @@ struct ppc_linux_nat_target final : public linux_nat_target
 
   void low_new_clone (struct lwp_info *, pid_t) override;
 
+  void low_init_process (pid_t pid) override;
+
   void low_forget_process (pid_t pid) override;
 
   void low_prepare_to_resume (struct lwp_info *) override;
@@ -2705,6 +2707,19 @@ ppc_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len,
   return 0;
 }
 
+/* Implement the "low_init_process" target_ops method.  */
+
+void
+ppc_linux_nat_target::low_init_process (pid_t pid)
+{
+  /* Set the hardware debug register capacity.  This requires the process to be
+     ptrace-stopped, otherwise detection will fail and software watchpoints will
+     be used instead of hardware.  If we allow this to be done lazily, we
+     cannot guarantee that it's called when the process is ptrace-stopped, so
+     do it now.  */
+  m_dreg_interface.detect (ptid_t (pid, pid));
+}
+
 /* Clean up the per-process info associated with PID.  When using the
    HWDEBUG interface, we also erase the per-thread state of installed
    debug registers for all the threads that belong to the group of PID.