[PING] [PATCH] Enable hardware watchpoints on attach for aarch64

[Richard Bunt <richard.bunt@arm.com>]

Polite ping.


On 05/23/2018 09:48 AM, Richard Bunt wrote:
> This patch fixes a bug whereby hardware watchpoints are not used on
> aarch64 when attaching to a target.  The fix adds an aarch64 
> specialization
> of post_attach which records the number of available hardware debug 
> registers
> using aarch64_linux_get_debug_reg_capacity.  This implementation 
> mirrors that
> of aarch64_linux_child_post_startup_inferior which successfully 
> enables the
> use of hardware watchpoints when launching the target under the debugger.
>
> 2018-05-14  Richard Bunt<richard.bunt@arm.com>
>         Dirk Schubert<dirk.schubert@arm.com>
>
>     * aarch64-linux-nat.c (post_attach): New.
>       (aarch64_linux_nat_target::post_attach): Override post_attach to
>       record the number of hardware debug registers.
>
> gdb/testsuite/ChangeLog:
>
> 2018-05-14  Richard Bunt<richard.bunt@arm.com>
>
>     * gdb.base/watchpoint-hw-attach.c: New test.
>     * gdb.base/watchpoint-hw-attach.exp: New file.
> ---
>  Annotation for this patch
>  gdb/aarch64-linux-nat.c                         | 18 ++++++
>  gdb/testsuite/gdb.base/watchpoint-hw-attach.c   | 44 +++++++++++++
>  gdb/testsuite/gdb.base/watchpoint-hw-attach.exp | 85 
> +++++++++++++++++++++++++
>  3 files changed, 147 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-attach.c
>  create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-attach.exp
>
> diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
> index 908b83a..e0f75d0 100644
> --- a/gdb/aarch64-linux-nat.c
> +++ b/gdb/aarch64-linux-nat.c
> @@ -76,6 +76,9 @@ public:
>    /* 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); }
> @@ -521,6 +524,21 @@ aarch64_linux_nat_target::post_startup_inferior 
> (ptid_t ptid)
>    linux_nat_target::post_startup_inferior (ptid);
>  }
>
> +/* Implement the "post_attach" target_ops method.  */
> +
> +void
> +aarch64_linux_nat_target::post_attach (int 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.  */
> +  aarch64_linux_get_debug_reg_capacity (pid);
> +  linux_nat_target::post_attach (pid);
> +}
> +
>  extern struct target_desc *tdesc_arm_with_neon;
>
>  /* Implement the "read_description" target_ops method.  */
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-attach.c 
> b/gdb/testsuite/gdb.base/watchpoint-hw-attach.c
> new file mode 100644
> index 0000000..9d55b28
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-attach.c
> @@ -0,0 +1,44 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2018 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, 
> see<http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +/* This is set to 1 by the debugger post attach to continue to the
> + watchpoint trigger.  */
> +volatile int should_continue = 0;
> +/* The variable to place a watchpoint on.  */
> +volatile int watched_variable = 0;
> +
> +int
> +main (void)
> +{
> +  unsigned int counter = 1;
> +  int mypid = getpid ();
> +
> +  /* Wait for the debugger to attach, but not indefinitely so this
> +     test program is not left hanging around.  */
> +  for (counter = 0; !should_continue && counter < 100; counter++)
> +    sleep (1);            /* pidacquired */
> +
> +  watched_variable = 0;        /* prewatchtrigger */
> +  /* Trigger a watchpoint.  */
> +  watched_variable = 4;
> +  printf ("My variable is %d\n", watched_variable);
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp 
> b/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp
> new file mode 100644
> index 0000000..a030cb7
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp
> @@ -0,0 +1,85 @@
> +# Copyright 2018 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see<http://www.gnu.org/licenses/>.
> +
> +# watchpoint-hw-attach.exp -- Test if hardware watchpoints are used
> +# when attaching to a target.
> +
> +if {[skip_hw_watchpoint_tests]} {
> +    return 0
> +}
> +
> +if {![can_spawn_for_attach]} {
> +    return 0
> +}
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile 
> debug]} {
> +    return -1
> +}
> +
> +clean_restart $binfile
> +
> +if ![runto_main] {
> +    untested "can't run to main"
> +    return -1
> +}
> +# Run to the point where mypid in the test program has been
> +# populated.
> +gdb_breakpoint [gdb_get_line_number "pidacquired"]
> +gdb_continue_to_breakpoint "pidacquired"
> +
> +# Get the PID of the test process.
> +set testpid ""
> +set test "get inferior process ID"
> +    gdb_test_multiple "p mypid" $test {
> +    -re " = ($decimal)\r\n$gdb_prompt $" {
> +    set testpid $expect_out(1,string)
> +    pass $test
> +    }
> +}
> +
> +gdb_test "detach" "Detaching from program: .*"
> +
> +if {$testpid == ""} {
> +    return -1
> +}
> +
> +# A clean restart is needed to force the hardware watchpoint setup
> +# logic to run post attach rather than post inferior launch.
> +clean_restart $binfile
> +
> +set test "attach $testpid"
> +gdb_test_multiple $test $test {
> +    -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt $" {
> +    pass "$test"
> +    }
> +    timeout {
> +    fail "$test"
> +    }
> +}
> +
> +gdb_test_no_output "set should_continue = 1"
> +# Ensure the test program is in the top frame so the required
> +# variables are in scope.
> +gdb_breakpoint [gdb_get_line_number "prewatchtrigger"]
> +gdb_continue_to_breakpoint "prewatchtrigger"
> +
> +gdb_test "watch watched_variable" \
> +     "Hardware watchpoint $decimal: watched_variable"
> +
> +gdb_test "continue" \
> +     "continue.*Continuing.*\.Hardware watchpoint $decimal:\
> +         watched_variable.*Old value = 0.*New value = 4.*"
> -- 1.8.5.6
>