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

[Luis Machado <luis.machado@arm.com>]

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?