[Bug testsuite/31705] New: New tests from g:c223d3738832011ced54e8415fa218934aebd0e4 fail on arm

[Bug testsuite/31705] New: New tests from g:c223d3738832011ced54e8415fa218934aebd0e4 fail on arm

[clyon at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

            Bug ID: 31705
           Summary: New tests from
                    g:c223d3738832011ced54e8415fa218934aebd0e4 fail on arm
           Product: gdb
           Version: unknown
            Status: NEW
          Severity: normal
          Priority: P2
         Component: testsuite
          Assignee: unassigned at sourceware dot org
          Reporter: clyon at gcc dot gnu.org
  Target Milestone: ---

Since g:c223d3738832011ced54e8415fa218934aebd0e4
(https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=c223d3738832011ced54e8415fa218934aebd0e4),
we have noticed that the new tests fail on arm:

Running gdb:gdb.base/watchpoint-running.exp ...
FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watch global_var
FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watchpoint hit
(timeout)
FAIL: gdb.base/watchpoint-running.exp: non-stop: hardware: watch global_var
FAIL: gdb.base/watchpoint-running.exp: non-stop: hardware: watchpoint hit
(timeout)

Running gdb:gdb.threads/signal-command-handle-nopass.exp ...
FAIL: gdb.threads/signal-command-handle-nopass.exp: step-over no: signal
SIGUSR1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] New tests from g:c223d3738832011ced54e8415fa218934aebd0e4 fail on arm

[clyon at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #1 from Christophe Lyon <clyon at gcc dot gnu.org> ---
From gdb.log:
[...]
PASS: gdb.base/watchpoint-running.exp: all-stop: hardware: values are different
watch global_var
Watchpoint 2: global_var
(gdb) FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watch
global_var
FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watchpoint hit
(timeout)

[...]
PASS: gdb.base/watchpoint-running.exp: non-stop: hardware: values are different
watch global_var
Watchpoint 2: global_var
(gdb) FAIL: gdb.base/watchpoint-running.exp: non-stop: hardware: watch
global_var
FAIL: gdb.base/watchpoint-running.exp: non-stop: hardware: watchpoint hit
(timeout)

[...]
(gdb) PASS: gdb.threads/signal-command-handle-nopass.exp: step-over no: thread
1 selected
signal SIGUSR1
Continuing with signal SIGUSR1.

Thread 1 "signal-command-" received signal SIGUSR1, User defined signal 1.
__libc_do_syscall () at ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47
warning: 47     ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S: No such file
or directory
(gdb) FAIL: gdb.threads/signal-command-handle-nopass.exp: step-over no: signal
SIGUSR1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] New tests from g:c223d3738832011ced54e8415fa218934aebd0e4 fail on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pedro at palves dot net

--- Comment #2 from Pedro Alves <pedro at palves dot net> ---
Is this 32-bit or 64-bit ARM?  I'm assuming 32-bit.

> (gdb) FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watch global_var
> FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watchpoint hit (timeout)

It very much looks like the ARM Linux backend is buggy wrt to setting
watchpoints.
It was always possible to set watchpoints while threads are running.  It just
wasn't possible if the _current_ thread wasn't stopped.  So it just looks like
the testcase uncovered some pre-existing bug.

The debug registers should be updated on all threads when the watchpoint is
created, in the low_prepare_to_resume implementation, if the dregs were
modified.   arm_linux_insert_hw_breakpoint1 does mark the debug registers as
modified, via update_registers_callback, and then forces a temporary pause. 
From a quick glance, the necessary pieces are in place.  Still, for some
reason, the watchpoint still doesn't trigger?  Someone who is motivated to work
on the ARM backend should look into that.
Probably "set debug infrun 1" + "set debug linux-nat 1" would help understand
what is going on.

> FAIL: gdb.threads/signal-command-handle-nopass.exp: step-over no: signal SIGUSR1

I don't see how the commit in question could be related to this.  It only
touched the code that creates watchpoints, and this testcase does not create
watchpoints.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

Pedro Alves <pedro at palves dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|New tests from              |gdb.base/watchpoint-running
                   |g:c223d3738832011ced54e8415 |.exp fails on arm
                   |fa218934aebd0e4 fail on arm |

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #3 from Pedro Alves <pedro at palves dot net> ---
Wait a second ...

> watch global_var
> Watchpoint 2: global_var
> (gdb) FAIL: gdb.base/watchpoint-running.exp: all-stop: hardware: watch global_var

... this above shows that GDB inserted a software watchpoint, not a hardware
watchpoint.  Ah.  Does that machine not have hardware watchpoints?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

Luis Machado <luis.machado at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |luis.machado at arm dot com

--- Comment #4 from Luis Machado <luis.machado at arm dot com> ---
Probably the backend thinks this can't be honored with a hardware watchpoint.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #5 from Pedro Alves <pedro at palves dot net> ---
There is only one watchpoint, and it is set on a 64-bit global variable.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #6 from Luis Machado <luis.machado at arm dot com> ---
I'll try to reproduce this. Watchpoints on 32-bit Arm have been a bit
unreliable due to bugs in the kernel and particularities on the interface
itself.

There is also a compat layer with 32-bit-on-64-bit-systems that makes things a
bit more difficult to reproduce.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vries at gcc dot gnu.org

--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 15556
  --> https://sourceware.org/bugzilla/attachment.cgi?id=15556&action=edit
gdb.log

Reproduced in debian-armhf chroot on aarch64-linux chromebook running debian.

Indeed global_var is a 64-bit variable, but it's not a problem to put a
hardware breakpoint on it.

The problem specifically is when the current thread is running. 

In arm_linux_get_hwbp_cap this call:
...
      if (ptrace (PTRACE_GETHBPREGS, tid, 0, &val) < 0)
...
sets errno to ESRCH.

Then there's the problem that this value gets cached, filed as PR31837.

If we bypass this problem by calling arm_linux_get_hwbp_cap while the thread is
still stopped:
...
diff --git a/gdb/testsuite/gdb.base/watchpoint-running.exp
b/gdb/testsuite/gdb.base/watchpoint-running.exp
index c3aae30ffba..9b08bc2e630 100644
--- a/gdb/testsuite/gdb.base/watchpoint-running.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-running.exp
@@ -52,6 +52,8 @@ proc test {stop_mode hw} {
        return
     }

+    delete_breakpoints
+    gdb_test "watch global_var"
     delete_breakpoints

     # Continue the program in the background.
...
then the test runs fine, so the mechanism in arm_linux_insert_hw_breakpoint1
seems to work.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
Fixed by:
...
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 50c24ecfcd2..9b738e5464f 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -611,7 +611,11 @@ arm_linux_get_hwbp_cap (void)
       unsigned int val;

       tid = inferior_ptid.lwp ();
-      if (ptrace (PTRACE_GETHBPREGS, tid, 0, &val) < 0)
+      int res;
+      linux_stop_and_wait_all_lwps ();
+      res = ptrace (PTRACE_GETHBPREGS, tid, 0, &val);
+      linux_unstop_all_lwps ();
+      if (res < 0)
        available = 0;
       else
        {
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #9 from Luis Machado <luis.machado at arm dot com> ---
Is this only for the initial discovery of features or every time a hardware
watchpoint is inserted?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Luis Machado from comment #9)
> Is this only for the initial discovery of features or every time a hardware
> watchpoint is inserted?

Initial, the result is cached in the "available" variable, as can be seen in
arm_linux_get_hwbp_cap at the ptrace call.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #11 from Pedro Alves <pedro at palves dot net> ---
(In reply to Tom de Vries from comment #8)
> Fixed by:
> ...
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 50c24ecfcd2..9b738e5464f 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -611,7 +611,11 @@ arm_linux_get_hwbp_cap (void)
>        unsigned int val;
>  
>        tid = inferior_ptid.lwp ();
> -      if (ptrace (PTRACE_GETHBPREGS, tid, 0, &val) < 0)
> +      int res;
> +      linux_stop_and_wait_all_lwps ();
> +      res = ptrace (PTRACE_GETHBPREGS, tid, 0, &val);
> +      linux_unstop_all_lwps ();
> +      if (res < 0)
>         available = 0;
>        else
>         {
> ...

It would be much better if we called arm_linux_get_hwbp_cap earlier, when we
know the inferior stopped, e.g., just after starting the inferior or after
attaching.  

Or alternatively, do a dance similar to linux_check_ptrace_features, or even
reusing / hooking into that somehow.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[luis.machado at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #12 from Luis Machado <luis.machado at arm dot com> ---
When we attach to a server running non-stop processes, do we have the guarantee
things will stop so we can check this properly? Or can we stop the processes
without disrupting things too much?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #13 from Pedro Alves <pedro at palves dot net> ---
When remote debugging, it's the server that does the check (in its linux
backend), and in that scenario, the server will have originally started or
attached to the process itself before gdb connected, and the server would have
cached the info then (when it knew the process was stopped).  Right?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #14 from Tom de Vries <vries at gcc dot gnu.org> ---
Copied this approach from aarch64-linux-nat.c:
...
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 50c24ecfcd2..f24e3b45ffb 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -92,6 +92,12 @@ class arm_linux_nat_target final : public linux_nat_target

   bool watchpoint_addr_within_range (CORE_ADDR, CORE_ADDR, int) 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;
+
   const struct target_desc *read_description () override;

   /* Override linux_nat_target low methods.  */
@@ -108,6 +114,8 @@ class arm_linux_nat_target final : public linux_nat_target

 static arm_linux_nat_target the_arm_linux_nat_target;

+static const struct arm_linux_hwbp_cap *arm_linux_get_hwbp_cap (void);
+
 /* Get the whole floating point state of the process and store it
    into regcache.  */

@@ -527,6 +535,24 @@ ps_get_thread_area (struct ps_prochandle *ph,
   return PS_OK;
 }

+/* Implement the virtual inf_ptrace_target::post_startup_inferior method.  */
+
+void
+arm_linux_nat_target::post_startup_inferior (ptid_t ptid)
+{
+  arm_linux_get_hwbp_cap ();
+  linux_nat_target::post_startup_inferior (ptid);
+}
+
+/* Implement the "post_attach" target_ops method.  */
+
+void
+arm_linux_nat_target::post_attach (int pid)
+{
+  arm_linux_get_hwbp_cap ();
+  linux_nat_target::post_attach (pid);
+}
+
 const struct target_desc *
 arm_linux_nat_target::read_description ()
 {
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #15 from Pedro Alves <pedro at palves dot net> ---
Much better, thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug testsuite/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #16 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Pedro Alves from comment #15)
> Much better, thanks!

Alternatively, this approach works for both arm and ppc (see PR31834):
...
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index c95d420d416..d8b5a99269b 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -454,6 +454,18 @@ linux_init_ptrace_procfs (pid_t pid, int attached)
   linux_ptrace_init_warnings ();
   linux_proc_init_warnings ();
   proc_mem_file_is_writable ();
+
+  /* Some targets (for instance ppc and arm) may call ptrace to answer a
+     target_can_use_hardware_watchpoint query, and cache the result.  However,
+     the ptrace call will fail with errno ESRCH if the tracee is not
+     ptrace-stopped, making the query fail.  And if the caching mechanism does
+     not disregard an ESRCH result, all subsequent queries will also fail.
+     Call it now, where we known the tracee is ptrace-stopped.
+
+     Other targets (for instance aarch64) do the relevant ptrace call and
+     caching in their implementation of post_attach and post_startup_inferior,
+     in which case this call is expected to have no effect.  */
+  target_can_use_hardware_watchpoint (bp_hardware_watchpoint, 1, 0);
 }

 linux_nat_target::~linux_nat_target ()
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tdep/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |HEAD
          Component|testsuite                   |tdep

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tdep/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #17 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/pipermail/gdb-patches/2024-June/209736.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug tdep/31705] gdb.base/watchpoint-running.exp fails on arm

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=31705

--- Comment #18 from Tom de Vries <vries at gcc dot gnu.org> ---
*** Bug 31839 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are on the CC list for the bug.