* [PATCH] Enable hardware watchpoints on attach for aarch64
@ 2018-05-23 10:32 Richard Bunt
2018-06-07 8:54 ` [PING] " Richard Bunt
0 siblings, 1 reply; 5+ messages in thread
From: Richard Bunt @ 2018-05-23 10:32 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, dirk.schubert
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PING] [PATCH] Enable hardware watchpoints on attach for aarch64
2018-05-23 10:32 [PATCH] Enable hardware watchpoints on attach for aarch64 Richard Bunt
@ 2018-06-07 8:54 ` Richard Bunt
2018-06-07 13:19 ` Alan Hayward
0 siblings, 1 reply; 5+ messages in thread
From: Richard Bunt @ 2018-06-07 8:54 UTC (permalink / raw)
To: gdb-patches; +Cc: nd, dirk.schubert
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PING] [PATCH] Enable hardware watchpoints on attach for aarch64
2018-06-07 8:54 ` [PING] " Richard Bunt
@ 2018-06-07 13:19 ` Alan Hayward
2018-06-12 1:38 ` Simon Marchi
0 siblings, 1 reply; 5+ messages in thread
From: Alan Hayward @ 2018-06-07 13:19 UTC (permalink / raw)
To: Richard Bunt; +Cc: gdb-patches, nd, Dirk Schubert
Aarch64 changes look good to me.
I’m not up to speed enough with .exp files to comment on that part.
I tested it on an aarch64 box, and it works fine for me.
Alan.
> On 7 Jun 2018, at 09:54, Richard Bunt <richard.bunt@arm.com> wrote:
>
> 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
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PING] [PATCH] Enable hardware watchpoints on attach for aarch64
2018-06-07 13:19 ` Alan Hayward
@ 2018-06-12 1:38 ` Simon Marchi
2018-06-18 14:41 ` Richard Bunt
0 siblings, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2018-06-12 1:38 UTC (permalink / raw)
To: Alan Hayward; +Cc: Richard Bunt, gdb-patches, nd, Dirk Schubert
On 2018-06-07 09:19, Alan Hayward wrote:
> Aarch64 changes look good to me.
> Iâm not up to speed enough with .exp files to comment on that part.
> I tested it on an aarch64 box, and it works fine for me.
Hi,
I looked at the test, if Alan is fine with the code changes then I am
fine with them. I noted some nits, but otherwise it looks good. Please
push after addressing them.
>>> +# 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
>>> + }
>>> +}
You can use get_integer_valueof for this.
>>> +set test "attach $testpid"
>>> +gdb_test_multiple $test $test {
>>> + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt $"
>>> {
>>> + pass "$test"
>>> + }
>>> + timeout {
>>> + fail "$test"
>>> + }
>>> +}
Just wondering, can't simple gdb_test be used here?
>>> +
>>> +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.*"
8 spaces -> tab on the last line. Though for test patterns, it's fine
to go over 80 columns, IMO.
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Enable hardware watchpoints on attach for aarch64
2018-06-12 1:38 ` Simon Marchi
@ 2018-06-18 14:41 ` Richard Bunt
0 siblings, 0 replies; 5+ messages in thread
From: Richard Bunt @ 2018-06-18 14:41 UTC (permalink / raw)
To: gdb-patches; +Cc: Dirk.Schubert, nd, Richard Bunt
This commit 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.
---
Updated based on Simon's feedback; thanks for the review.
The feedback has been addressed as detailed below.
>On 2018-06-07 09:19, Alan Hayward wrote:
>> Aarch64 changes look good to me.
>> I'm not up to speed enough with .exp files to comment on that part.
>> I tested it on an aarch64 box, and it works fine for me.
>
>Hi,
>
>I looked at the test, if Alan is fine with the code changes then I am
>fine with them. I noted some nits, but otherwise it looks good. Please
>push after addressing them.
>
>>>> +# 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
>>>> + }
>>>> +}
>
>You can use get_integer_valueof for this.
>
Fixed, and good to know about this one.
>>>> +set test "attach $testpid"
>>>> +gdb_test_multiple $test $test {
>>>> + -re "Attaching to program.*process $testpid\r\n.*$gdb_prompt $"
>>>> {
>>>> + pass "$test"
>>>> + }
>>>> + timeout {
>>>> + fail "$test"
>>>> + }
>>>> +}
>
>Just wondering, can't simple gdb_test be used here?
>
Changed. There is no reason that I am aware of that this cannot be a
gdb_test. I think this was just the structure used in the attach test that I
borrowed from.
>>>> +
>>>> +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.*"
>
>8 spaces -> tab on the last line. Though for test patterns, it's fine
>to go over 80 columns, IMO.
>
Fixed, that certainly makes it easier to read.
>Simon
Will push (as per the review) in a few days if there are no more objections.
Many thanks,
Rich
gdb/aarch64-linux-nat.c | 18 +++++++
gdb/testsuite/gdb.base/watchpoint-hw-attach.c | 44 +++++++++++++++
gdb/testsuite/gdb.base/watchpoint-hw-attach.exp | 71 +++++++++++++++++++++++++
3 files changed, 133 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 1e7db29..c9fd062 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -77,6 +77,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); }
@@ -568,6 +571,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..0c5037c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-hw-attach.exp
@@ -0,0 +1,71 @@
+# 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 [get_integer_valueof "mypid" 0]
+
+gdb_test "detach" "Detaching from program: .*, process $testpid\r\n\\\[Inferior $decimal \\(process $testpid\\) detached\\\]"
+
+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
+
+gdb_test "attach $testpid" "Attaching to program: .*, process $testpid.*"
+
+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.*watched_variable\\);"
--
1.8.5.6
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-18 14:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-23 10:32 [PATCH] Enable hardware watchpoints on attach for aarch64 Richard Bunt
2018-06-07 8:54 ` [PING] " Richard Bunt
2018-06-07 13:19 ` Alan Hayward
2018-06-12 1:38 ` Simon Marchi
2018-06-18 14:41 ` Richard Bunt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).