* [PATCH v2 0/2] Add testcase for libc memory operations @ 2024-04-21 22:26 Thiago Jung Bauermann 2024-04-21 22:26 ` [PATCH v2 1/2] gdb/testsuite: Add libc_has_debug_info require helper Thiago Jung Bauermann 2024-04-21 22:26 ` [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp Thiago Jung Bauermann 0 siblings, 2 replies; 5+ messages in thread From: Thiago Jung Bauermann @ 2024-04-21 22:26 UTC (permalink / raw) To: gdb-patches Hello, v2 addresses a test failure on armv8l-linux-gnueabihf found by the Linaro CI (I forgot to try the testcase on that platform...). The problem was that the machine used by the CI needs hardware watchpoints to be aligned to 4 bytes, or else GDB fails with: (gdb) watch -location c[31] Hardware watchpoint 4: -location c[31] (gdb) continue Continuing. Unexpected error setting watchpoint: Invalid argument. (gdb) After fixing this problem, the testcase uncovered a real bug, filed as PR breakpoints/31665 and kfailed in the testcase. Patch 1 is the same as v1, only patch 2 has changes. Original cover letter below: In PR testsuite/31484, Tom de Vries suggests adding testcases that excercise memset/memcpy/memmove and set watchpoints. Patch 2 adds such testcase. See its description for a bit more on the motivation. Patch 1 factors out code to check whether libc debug info is available, which is needed for the new testcase. Thiago Jung Bauermann (2): gdb/testsuite: Add libc_has_debug_info require helper gdb/testsuite: Add gdb.base/memops-watchpoint.exp gdb/testsuite/gdb.base/memops-watchpoint.c | 45 ++++++++++ gdb/testsuite/gdb.base/memops-watchpoint.exp | 86 ++++++++++++++++++++ gdb/testsuite/gdb.base/relativedebug.exp | 13 +-- gdb/testsuite/lib/gdb.exp | 54 ++++++++++++ 4 files changed, 186 insertions(+), 12 deletions(-) create mode 100644 gdb/testsuite/gdb.base/memops-watchpoint.c create mode 100644 gdb/testsuite/gdb.base/memops-watchpoint.exp base-commit: 20eee7540b9f2615f7661393756fec0bb62a1495 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] gdb/testsuite: Add libc_has_debug_info require helper 2024-04-21 22:26 [PATCH v2 0/2] Add testcase for libc memory operations Thiago Jung Bauermann @ 2024-04-21 22:26 ` Thiago Jung Bauermann 2024-04-21 22:26 ` [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp Thiago Jung Bauermann 1 sibling, 0 replies; 5+ messages in thread From: Thiago Jung Bauermann @ 2024-04-21 22:26 UTC (permalink / raw) To: gdb-patches Factor the test for libc debug info out of gdb.base/relativedebug.exp to a new procedure. Also, change the "info sharedlibrary" test to explicitly detect when libc has debug info. --- gdb/testsuite/gdb.base/relativedebug.exp | 13 +----- gdb/testsuite/lib/gdb.exp | 54 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/gdb/testsuite/gdb.base/relativedebug.exp b/gdb/testsuite/gdb.base/relativedebug.exp index bf8d76887122..f882a5cf1676 100644 --- a/gdb/testsuite/gdb.base/relativedebug.exp +++ b/gdb/testsuite/gdb.base/relativedebug.exp @@ -13,7 +13,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -require {!target_info exists gdb,nosignals} +require {!target_info exists gdb,nosignals} libc_has_debug_info standard_testfile .c @@ -28,17 +28,6 @@ clean_restart ${binfile} runto_main -set test "info sharedlibrary" -gdb_test_multiple $test $test { - -re ".*\(\\*\)\[^\r\n\]*/libc\.so.*$gdb_prompt $" { - # Skip the test below if libc doesn't have debug info. - unsupported "libc doesn't have debug info" - return -1 - } - -re ".*$gdb_prompt $" { - } -} - # pause () -> SIGALRM -> handler () -> abort () gdb_test "continue" "Program received signal SIGABRT.*" diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index ddee928d5104..9af73bef8f09 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -3699,6 +3699,60 @@ proc support_displaced_stepping {} { return 0 } +# Return 1 if GDB can find the libc debug info, or 0 and a reason string if it +# can't. This procedure is meant to be called by the require procedure. +gdb_caching_proc libc_has_debug_info {} { + global srcdir subdir gdb_prompt inferior_exited_re + + set me "libc_has_debug_info" + + # Compile a test program. + set src { + int main (void) { + printf ("Hello, world!\n"); + return 0; + } + } + if {![gdb_simple_compile $me $src executable {debug}]} { + return [list 0 "failed to compile test program"] + } + + # No error message, compilation succeeded so now run it via gdb. + + gdb_exit + gdb_start + gdb_reinitialize_dir $srcdir/$subdir + gdb_load "$obj" + runto_main + set test "info sharedlibrary libc.so" + gdb_test_multiple $test $test { + -re ".*\(\\*\)\[^\r\n\]*/libc\.so.*$gdb_prompt $" { + # Matched the "(*)" in the "Syms Read" columns which means: + # "(*): Shared library is missing debugging information." + verbose -log "$me: libc doesn't have debug info" + set libc_has_debug_info 0 + set message "libc doesn't have debug info" + } + -re ".*Yes\[ \t\]+\[^\r\n\]*/libc\.so.*$gdb_prompt $" { + verbose -log "$me: libc has debug info" + set libc_has_debug_info 1 + } + default { + set libc_has_debug_info 0 + set message "libc not found in the inferior" + } + } + gdb_exit + remote_file build delete $obj + + verbose "$me: returning $libc_has_debug_info" 2 + if { $libc_has_debug_info } { + return $libc_has_debug_info + } else { + return [list $libc_has_debug_info $message] + } +} + # Run a test on the target to see if it supports vmx hardware. Return 1 if so, # 0 if it does not. Based on 'check_vmx_hw_available' from the GCC testsuite. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp 2024-04-21 22:26 [PATCH v2 0/2] Add testcase for libc memory operations Thiago Jung Bauermann 2024-04-21 22:26 ` [PATCH v2 1/2] gdb/testsuite: Add libc_has_debug_info require helper Thiago Jung Bauermann @ 2024-04-21 22:26 ` Thiago Jung Bauermann 2024-04-22 14:39 ` Luis Machado 1 sibling, 1 reply; 5+ messages in thread From: Thiago Jung Bauermann @ 2024-04-21 22:26 UTC (permalink / raw) To: gdb-patches 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" ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp 2024-04-21 22:26 ` [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp Thiago Jung Bauermann @ 2024-04-22 14:39 ` Luis Machado 2024-04-22 16:07 ` Thiago Jung Bauermann 0 siblings, 1 reply; 5+ messages in thread From: Luis Machado @ 2024-04-22 14:39 UTC (permalink / raw) To: Thiago Jung Bauermann, gdb-patches 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? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp 2024-04-22 14:39 ` Luis Machado @ 2024-04-22 16:07 ` Thiago Jung Bauermann 0 siblings, 0 replies; 5+ messages in thread From: Thiago Jung Bauermann @ 2024-04-22 16:07 UTC (permalink / raw) To: Luis Machado; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-22 16:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-21 22:26 [PATCH v2 0/2] Add testcase for libc memory operations Thiago Jung Bauermann 2024-04-21 22:26 ` [PATCH v2 1/2] gdb/testsuite: Add libc_has_debug_info require helper Thiago Jung Bauermann 2024-04-21 22:26 ` [PATCH v2 2/2] gdb/testsuite: Add gdb.base/memops-watchpoint.exp Thiago Jung Bauermann 2024-04-22 14:39 ` Luis Machado 2024-04-22 16:07 ` Thiago Jung Bauermann
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).