[v2,2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp

Message ID 20240421222657.1052635-3-thiago.bauermann@linaro.org
State New
Headers
Series Add testcase for libc memory operations |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Thiago Jung Bauermann April 21, 2024, 10:26 p.m. UTC
  Test behaviour of watchpoints triggered by libc's memset/memcpy/memmove.
These functions are frequently optimized with specialized instructions
that favor larger memory access operations, so make sure GDB behaves
correctly in their presence.

There's a separate watched variable for each function so that the testcase
can test whether GDB correctly identified the watchpoint that triggered.

Also, the watchpoint is 28 bytes away from the beginning of the buffer
being modified, so that large memory accesses (if present) are exercised.

PR testsuite/31484
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31484
---
 gdb/testsuite/gdb.base/memops-watchpoint.c   | 45 ++++++++++
 gdb/testsuite/gdb.base/memops-watchpoint.exp | 86 ++++++++++++++++++++
 2 files changed, 131 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/memops-watchpoint.c
 create mode 100644 gdb/testsuite/gdb.base/memops-watchpoint.exp

NB: As with v1, please let me know if you think there are more interesting
tests regarding watchpoint and memory accesses that can be done.  I tried
to make it cover the interesting scenarios but the testcase is small, so
maybe I'm not very creative.

Changes in v2:
- Ensure watchpoints are aligned to 4 bytes.
- Add kfail for arm-linux.
  

Comments

Luis Machado April 22, 2024, 2:39 p.m. UTC | #1
Hi,

On 4/21/24 23:26, Thiago Jung Bauermann wrote:
> Test behaviour of watchpoints triggered by libc's memset/memcpy/memmove.
> These functions are frequently optimized with specialized instructions
> that favor larger memory access operations, so make sure GDB behaves
> correctly in their presence.
> 
> There's a separate watched variable for each function so that the testcase
> can test whether GDB correctly identified the watchpoint that triggered.
> 
> Also, the watchpoint is 28 bytes away from the beginning of the buffer
> being modified, so that large memory accesses (if present) are exercised.
> 
> PR testsuite/31484
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31484
> ---
>  gdb/testsuite/gdb.base/memops-watchpoint.c   | 45 ++++++++++
>  gdb/testsuite/gdb.base/memops-watchpoint.exp | 86 ++++++++++++++++++++
>  2 files changed, 131 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/memops-watchpoint.c
>  create mode 100644 gdb/testsuite/gdb.base/memops-watchpoint.exp
> 
> NB: As with v1, please let me know if you think there are more interesting
> tests regarding watchpoint and memory accesses that can be done.  I tried
> to make it cover the interesting scenarios but the testcase is small, so
> maybe I'm not very creative.
> 
> Changes in v2:
> - Ensure watchpoints are aligned to 4 bytes.
> - Add kfail for arm-linux.
> 
> diff --git a/gdb/testsuite/gdb.base/memops-watchpoint.c b/gdb/testsuite/gdb.base/memops-watchpoint.c
> new file mode 100644
> index 000000000000..0255cfb43404
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/memops-watchpoint.c
> @@ -0,0 +1,45 @@
> +/* This test program is part of GDB, the GNU debugger.
> +
> +   Copyright 2024 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 <string.h>
> +
> +int
> +main (void)
> +{
> +  /* Some targets need 4-byte alignment for hardware watchpoints.  */
> +  char s[40] __attribute__ ((aligned (4)))
> +    = "This is a relatively long string...";
> +  char a[40] __attribute__ ((aligned (4)))
> +    = "String to be overwritten with zeroes";
> +  char b[40] __attribute__ ((aligned (4)))
> +    = "Another string to be memcopied...";
> +  char c[40] __attribute__ ((aligned (4)))
> +    = "Another string to be memmoved...";
> +
> +  /* Break here.  */
> +  memset (a, 0, sizeof (a));
> +
> +  memcpy (b, s, sizeof (b));
> +
> +  memmove (c, s, sizeof (c));
> +
> +  printf ("b = '%s'\n", b);
> +  printf ("c = '%s'\n", c);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/memops-watchpoint.exp b/gdb/testsuite/gdb.base/memops-watchpoint.exp
> new file mode 100644
> index 000000000000..0edc646e67aa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/memops-watchpoint.exp
> @@ -0,0 +1,86 @@
> +# Copyright 2024 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/>.
> +
> +# Test a binary that uses standard libc memory operation functions.  They are
> +# frequently optimized with specialized instructions, so make sure GDB behaves
> +# correctly in their presence.
> +
> +# It's not possible to check in which libc function the watchpoint triggers
> +# without its debug info.
> +require libc_has_debug_info
> +
> +standard_testfile
> +
> +set options "-fno-builtin-memset -fno-builtin-memcpy -fno-builtin-memmove"
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> +	  [list debug additional_flags=$options]] } {
> +    return -1
> +}
> +
> +set linespec ${srcfile}:[gdb_get_line_number "Break here"]
> +
> +if ![runto ${linespec}] {
> +    return
> +}
> +
> +gdb_test "watch -location a\[28\]" \
> +    "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
> +    "set watch on a"
> +gdb_test "watch -location b\[28\]" \
> +    "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
> +    "set watchpoint on b"
> +gdb_test "watch -location c\[28\]" \
> +    "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
> +    "set watchpoint on c"
> +
> +setup_kfail breakpoints/31665 arm*-*-linux*
> +gdb_test "continue" \
> +    [multi_line \
> +	 "Continuing\\." \
> +	 "" \
> +	 "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
> +	 "" \
> +	 "Old value = 104 'h'" \
> +	 "New value = 0 '\\\\000'" \
> +	 ".*memset.* \\(\\) at .*:$decimal" \
> +	 ".*"] \
> +    "continue until memset watchpoint hits"
> +
> +setup_kfail breakpoints/31665 arm*-*-linux*
> +gdb_test "continue" \
> +    [multi_line \
> +	 "Continuing\\." \
> +	 "" \
> +	 "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
> +	 "" \
> +	 "Old value = 101 'e'" \
> +	 "New value = 114 'r'" \
> +	 ".*memcpy.* \\(\\) at .*:$decimal" \
> +	".*"] \
> +    "continue until memcpy watchpoint hits"
> +
> +# Note: Some architectures use memcpy for memmove.
> +setup_kfail breakpoints/31665 arm*-*-linux*
> +gdb_test "continue" \
> +    [multi_line \
> +	 "Continuing\\." \
> +	 "" \
> +	 "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
> +	 "" \
> +	 "Old value = 100 'd'" \
> +	 "New value = 114 'r'" \
> +	 ".*(memmove|memcpy).* \\(\\) at .*:$decimal" \
> +	".*"] \
> +    "continue until memmove watchpoint hits"

For this test, on aarch64 I see full passes, but on 32-bit Arm I see
3 passes and 3 kfail's.

Is that the expected outcome?
  
Thiago Jung Bauermann April 22, 2024, 4:07 p.m. UTC | #2
Hello Luis,

Thanks for reviewing the patches!

Luis Machado <luis.machado@arm.com> writes:

> On 4/21/24 23:26, Thiago Jung Bauermann wrote:
>> +setup_kfail breakpoints/31665 arm*-*-linux*
>> +gdb_test "continue" \
>> +    [multi_line \
>> +	 "Continuing\\." \
>> +	 "" \
>> +	 "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
>> +	 "" \
>> +	 "Old value = 104 'h'" \
>> +	 "New value = 0 '\\\\000'" \
>> +	 ".*memset.* \\(\\) at .*:$decimal" \
>> +	 ".*"] \
>> +    "continue until memset watchpoint hits"
>> +
>> +setup_kfail breakpoints/31665 arm*-*-linux*
>> +gdb_test "continue" \
>> +    [multi_line \
>> +	 "Continuing\\." \
>> +	 "" \
>> +	 "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
>> +	 "" \
>> +	 "Old value = 101 'e'" \
>> +	 "New value = 114 'r'" \
>> +	 ".*memcpy.* \\(\\) at .*:$decimal" \
>> +	".*"] \
>> +    "continue until memcpy watchpoint hits"
>> +
>> +# Note: Some architectures use memcpy for memmove.
>> +setup_kfail breakpoints/31665 arm*-*-linux*
>> +gdb_test "continue" \
>> +    [multi_line \
>> +	 "Continuing\\." \
>> +	 "" \
>> +	 "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
>> +	 "" \
>> +	 "Old value = 100 'd'" \
>> +	 "New value = 114 'r'" \
>> +	 ".*(memmove|memcpy).* \\(\\) at .*:$decimal" \
>> +	".*"] \
>> +    "continue until memmove watchpoint hits"
>
> For this test, on aarch64 I see full passes, but on 32-bit Arm I see
> 3 passes and 3 kfail's.
>
> Is that the expected outcome?

Yes, the testcase uncovered a bug in in hardware watchpoints with 32-bit
Arm (the testcase fully passes when the testcase is modified to use
software watchpoints) so I opened PR breakpoints/31665 to track it.

I didn't have the chance to investigate the problem yet, but it's in my
TODO list.

--
Thiago
  

Patch

diff --git a/gdb/testsuite/gdb.base/memops-watchpoint.c b/gdb/testsuite/gdb.base/memops-watchpoint.c
new file mode 100644
index 000000000000..0255cfb43404
--- /dev/null
+++ b/gdb/testsuite/gdb.base/memops-watchpoint.c
@@ -0,0 +1,45 @@ 
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2024 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 <string.h>
+
+int
+main (void)
+{
+  /* Some targets need 4-byte alignment for hardware watchpoints.  */
+  char s[40] __attribute__ ((aligned (4)))
+    = "This is a relatively long string...";
+  char a[40] __attribute__ ((aligned (4)))
+    = "String to be overwritten with zeroes";
+  char b[40] __attribute__ ((aligned (4)))
+    = "Another string to be memcopied...";
+  char c[40] __attribute__ ((aligned (4)))
+    = "Another string to be memmoved...";
+
+  /* Break here.  */
+  memset (a, 0, sizeof (a));
+
+  memcpy (b, s, sizeof (b));
+
+  memmove (c, s, sizeof (c));
+
+  printf ("b = '%s'\n", b);
+  printf ("c = '%s'\n", c);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/memops-watchpoint.exp b/gdb/testsuite/gdb.base/memops-watchpoint.exp
new file mode 100644
index 000000000000..0edc646e67aa
--- /dev/null
+++ b/gdb/testsuite/gdb.base/memops-watchpoint.exp
@@ -0,0 +1,86 @@ 
+# Copyright 2024 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/>.
+
+# Test a binary that uses standard libc memory operation functions.  They are
+# frequently optimized with specialized instructions, so make sure GDB behaves
+# correctly in their presence.
+
+# It's not possible to check in which libc function the watchpoint triggers
+# without its debug info.
+require libc_has_debug_info
+
+standard_testfile
+
+set options "-fno-builtin-memset -fno-builtin-memcpy -fno-builtin-memmove"
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  [list debug additional_flags=$options]] } {
+    return -1
+}
+
+set linespec ${srcfile}:[gdb_get_line_number "Break here"]
+
+if ![runto ${linespec}] {
+    return
+}
+
+gdb_test "watch -location a\[28\]" \
+    "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
+    "set watch on a"
+gdb_test "watch -location b\[28\]" \
+    "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
+    "set watchpoint on b"
+gdb_test "watch -location c\[28\]" \
+    "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
+    "set watchpoint on c"
+
+setup_kfail breakpoints/31665 arm*-*-linux*
+gdb_test "continue" \
+    [multi_line \
+	 "Continuing\\." \
+	 "" \
+	 "(Hardware w|W)atchpoint ${decimal}: -location a\\\[28\\\]" \
+	 "" \
+	 "Old value = 104 'h'" \
+	 "New value = 0 '\\\\000'" \
+	 ".*memset.* \\(\\) at .*:$decimal" \
+	 ".*"] \
+    "continue until memset watchpoint hits"
+
+setup_kfail breakpoints/31665 arm*-*-linux*
+gdb_test "continue" \
+    [multi_line \
+	 "Continuing\\." \
+	 "" \
+	 "(Hardware w|W)atchpoint ${decimal}: -location b\\\[28\\\]" \
+	 "" \
+	 "Old value = 101 'e'" \
+	 "New value = 114 'r'" \
+	 ".*memcpy.* \\(\\) at .*:$decimal" \
+	".*"] \
+    "continue until memcpy watchpoint hits"
+
+# Note: Some architectures use memcpy for memmove.
+setup_kfail breakpoints/31665 arm*-*-linux*
+gdb_test "continue" \
+    [multi_line \
+	 "Continuing\\." \
+	 "" \
+	 "(Hardware w|W)atchpoint ${decimal}: -location c\\\[28\\\]" \
+	 "" \
+	 "Old value = 100 'd'" \
+	 "New value = 114 'r'" \
+	 ".*(memmove|memcpy).* \\(\\) at .*:$decimal" \
+	".*"] \
+    "continue until memmove watchpoint hits"