| Message ID | 20250405005003.752820-1-thiago.bauermann@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gdb/testsuite: Add gdb.arch/aarch64-sve-vg-sigunwind.exp | expand |
Hi, The patch looks good to me. There is just one corner case that we should address. I gave this a try on QEMU and restricted the SVE vector length choices to just one, 128-bit. You can set it like so: -cpu max,sve=on,sve128=on Running the test in that setup gives me a FAIL: Running /work/builds/binutils-gdb/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp ... FAIL: gdb.arch/aarch64-sve-vg-sigunwind.exp: runto: run to aarch64-sve-vg-sigunwind.c:55 That happens because... On 4/5/25 01:50, Thiago Jung Bauermann wrote: > There's currently not test for unwinding the VG register from a signal > frame, so add one. All tests in the testcase pass. > > Tested on aarch64-linux-gnu native, native-gdbserver and > native-extended-gdbserver. > --- > .../gdb.arch/aarch64-sve-vg-sigunwind.c | 87 +++++++++++++++++++ > .../gdb.arch/aarch64-sve-vg-sigunwind.exp | 55 ++++++++++++ > 2 files changed, 142 insertions(+) > create mode 100644 gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c > create mode 100644 gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp > > diff --git a/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c > new file mode 100644 > index 000000000000..f2403ae448d1 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c > @@ -0,0 +1,87 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2025 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/>. */ > + > +/* Exercise unwinding AArch64's Scalable Vector Extension VG register. */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <signal.h> /* For SVE_VL_MAX. */ > +#include <sys/prctl.h> > +#include <sys/ptrace.h> > +#include <unistd.h> > + > +static int max_vl = 0; > + > +/* Set new value for the SVE vector length. > + Return the value that was set. */ > + > +static int > +set_vl (int vl) > +{ > + int rc; > + > + rc = prctl (PR_SVE_SET_VL, vl, 0, 0, 0); > + if (rc < 0) > + { > + perror ("FAILED to PR_SVE_SET_VL"); > + exit (EXIT_FAILURE); > + } > + > + return rc & PR_SVE_VL_LEN_MASK; > +} > + > +static void > +sighandler (int) > +{ > + int current_vl; > + > + /* Decrease vector length by 16 bytes. */ > + printf ("sighandler: setting to %d\n", max_vl - 16); > + current_vl = set_vl (max_vl - 16); > + printf ("sighandler: current_vl = %d\n", current_vl); /* Break here. */ > +} > + > +int > +main (int argc, char *argv[]) > +{ > + /* Set vector length to the maximum size allowed. */ > + max_vl = set_vl (SVE_VL_MAX); > + > + printf ("main: max_vl = %d\n", max_vl); > + > + /* If maximum VL is less than 32 bytes we can't decrease it in the signal > + handler. */ > + if (max_vl < 32) > + return 1; We bail out above if we have a vector length smaller than 32 bytes. But the testcase puts a breakpoint within sighandler, which isn't registered as a handler until... > + > + unsigned char buf[256]; > + > + /* Use an SVE register to make the kernel start saving the SVE bank. */ > + asm volatile ("mov z0.b, #255\n\t" > + "str z0, %0" > + : > + : "m" (buf) > + : "z0", "memory"); > + > + struct sigaction sigact; > + sigact.sa_handler = sighandler; > + sigaction (SIGUSR1, &sigact, NULL); ... here. So the testcase exits early and results in a FAIL. For SVE/SME tests, we have some helpers that extract the supported vector lengths. I'm not sure we record the default length though. But maybe we could use that information to bail out of the test for this 128-bit vector length corner case. I also tried with a setup where we only support a 256-bit vector length, but that passed just fine by setting vg to 2 (from 4), even though 128-bit isn't a supported vector length. I'm not sure if that's of concern or not. But I thought I'd mention. > + > + kill (getpid (), SIGUSR1); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp > new file mode 100644 > index 000000000000..7fea93ffbd58 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp > @@ -0,0 +1,55 @@ > +# Copyright 2025 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/>. > + > +# Exercise unwinding AArch64's Scalable Vector Extension VG register. > + > +require allow_aarch64_sve_tests > + > +standard_testfile > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ > + [list debug additional_flags=-march=armv9-a]] } { > + return > +} > + > +# We want SIGUSR1 to be delivered normally. > +gdb_test "handle SIGUSR1 nostop" \ > + [multi_line {Signal Stop Print Pass to program Description} \ > + {SIGUSR1 No Yes Yes User defined signal 1}] \ > + "don't stop for SIGUSR1" > + > +set linespec ${srcfile}:[gdb_get_line_number "Break here."] > + > +if ![runto ${linespec}] { > + return > +} > + > +set max_vl [get_valueof "" "max_vl" 0] > +set max_vg [expr $max_vl/8] > + > +# If maximum VL is less than 256 bits we can't decrease it in the signal > +# handler and the test can't work. > +if { $max_vl < 32 } { > + untested "Maximum vector length is too small." > + return > +} > + > +set reduced_vg [expr "$max_vg - 2"] > +gdb_test "print \$vg" ". = ${reduced_vg}" "vg was reduced" > + > +gdb_test "frame function main" \ > + [multi_line "#$decimal $hex in main \[^\r\n\]+" \ > + "$decimal\[ \t\]+kill \\(getpid \\(\\), SIGUSR1\\);"] > + > +gdb_test "print \$vg" ". = $max_vg" "vg was correctly unwound" > > base-commit: 4fa9476ad23d5868c2bc4c02dc1a73acaefbcbc2
Hello, Luis Machado <luis.machado@arm.com> writes: > The patch looks good to me. There is just one corner case that we should address. Thank you for the review! I'll post a v2 shortly fixing the problem you found, as well as other changes. > I gave this a try on QEMU and restricted the SVE vector length choices to just one, > 128-bit. > > You can set it like so: > > -cpu max,sve=on,sve128=on Ah, I wasn't aware of this. > Running the test in that setup gives me a FAIL: > > Running > /work/builds/binutils-gdb/gdb/testsuite/../../../../repos/binutils-gdb/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp > ... > FAIL: gdb.arch/aarch64-sve-vg-sigunwind.exp: runto: run to aarch64-sve-vg-sigunwind.c:55 > > That happens because... Thanks for doing this test! I tried to handle this case but evidently failed. > On 4/5/25 01:50, Thiago Jung Bauermann wrote: >> There's currently not test for unwinding the VG register from a signal >> frame, so add one. All tests in the testcase pass. >> >> Tested on aarch64-linux-gnu native, native-gdbserver and >> native-extended-gdbserver. >> --- >> .../gdb.arch/aarch64-sve-vg-sigunwind.c | 87 +++++++++++++++++++ >> .../gdb.arch/aarch64-sve-vg-sigunwind.exp | 55 ++++++++++++ >> 2 files changed, 142 insertions(+) >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c >> create mode 100644 gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp >> >> diff --git a/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c >> b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c >> new file mode 100644 >> index 000000000000..f2403ae448d1 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c >> @@ -0,0 +1,87 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2025 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/>. */ >> + >> +/* Exercise unwinding AArch64's Scalable Vector Extension VG register. */ >> + >> +#include <stdio.h> >> +#include <stdlib.h> >> +#include <signal.h> /* For SVE_VL_MAX. */ >> +#include <sys/prctl.h> >> +#include <sys/ptrace.h> >> +#include <unistd.h> >> + >> +static int max_vl = 0; >> + >> +/* Set new value for the SVE vector length. >> + Return the value that was set. */ >> + >> +static int >> +set_vl (int vl) >> +{ >> + int rc; >> + >> + rc = prctl (PR_SVE_SET_VL, vl, 0, 0, 0); >> + if (rc < 0) >> + { >> + perror ("FAILED to PR_SVE_SET_VL"); >> + exit (EXIT_FAILURE); >> + } >> + >> + return rc & PR_SVE_VL_LEN_MASK; >> +} >> + >> +static void >> +sighandler (int) >> +{ >> + int current_vl; >> + >> + /* Decrease vector length by 16 bytes. */ >> + printf ("sighandler: setting to %d\n", max_vl - 16); >> + current_vl = set_vl (max_vl - 16); >> + printf ("sighandler: current_vl = %d\n", current_vl); /* Break here. */ >> +} >> + >> +int >> +main (int argc, char *argv[]) >> +{ >> + /* Set vector length to the maximum size allowed. */ >> + max_vl = set_vl (SVE_VL_MAX); >> + >> + printf ("main: max_vl = %d\n", max_vl); >> + >> + /* If maximum VL is less than 32 bytes we can't decrease it in the signal >> + handler. */ >> + if (max_vl < 32) >> + return 1; > > We bail out above if we have a vector length smaller than 32 bytes. But the testcase > puts a breakpoint within sighandler, which isn't registered as a handler until... > >> + >> + unsigned char buf[256]; >> + >> + /* Use an SVE register to make the kernel start saving the SVE bank. */ >> + asm volatile ("mov z0.b, #255\n\t" >> + "str z0, %0" >> + : >> + : "m" (buf) >> + : "z0", "memory"); >> + >> + struct sigaction sigact; >> + sigact.sa_handler = sighandler; >> + sigaction (SIGUSR1, &sigact, NULL); > > ... here. So the testcase exits early and results in a FAIL. > > For SVE/SME tests, we have some helpers that extract the supported vector > lengths. I'm not sure we record the default length though. But maybe we > could use that information to bail out of the test for this 128-bit vector > length corner case. Good idea, I had forgotten about these helpers. In v2 I changed the test to get two valid vector lengths to use instead of setting the maximum and trying to decrease. The test is then able to exit early if only one vector length is supported. I'll post the patch shortly. > I also tried with a setup where we only support a 256-bit vector length, but that > passed just fine by setting vg to 2 (from 4), even though 128-bit isn't a supported > vector length. I'm not sure if that's of concern or not. But I thought I'd mention. That's a kernel bug I think. prctl (PR_SVE_SET_VL) should have reported an error. >> + >> + kill (getpid (), SIGUSR1); >> + >> + return 0; >> +}
diff --git a/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c new file mode 100644 index 000000000000..f2403ae448d1 --- /dev/null +++ b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.c @@ -0,0 +1,87 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2025 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/>. */ + +/* Exercise unwinding AArch64's Scalable Vector Extension VG register. */ + +#include <stdio.h> +#include <stdlib.h> +#include <signal.h> /* For SVE_VL_MAX. */ +#include <sys/prctl.h> +#include <sys/ptrace.h> +#include <unistd.h> + +static int max_vl = 0; + +/* Set new value for the SVE vector length. + Return the value that was set. */ + +static int +set_vl (int vl) +{ + int rc; + + rc = prctl (PR_SVE_SET_VL, vl, 0, 0, 0); + if (rc < 0) + { + perror ("FAILED to PR_SVE_SET_VL"); + exit (EXIT_FAILURE); + } + + return rc & PR_SVE_VL_LEN_MASK; +} + +static void +sighandler (int) +{ + int current_vl; + + /* Decrease vector length by 16 bytes. */ + printf ("sighandler: setting to %d\n", max_vl - 16); + current_vl = set_vl (max_vl - 16); + printf ("sighandler: current_vl = %d\n", current_vl); /* Break here. */ +} + +int +main (int argc, char *argv[]) +{ + /* Set vector length to the maximum size allowed. */ + max_vl = set_vl (SVE_VL_MAX); + + printf ("main: max_vl = %d\n", max_vl); + + /* If maximum VL is less than 32 bytes we can't decrease it in the signal + handler. */ + if (max_vl < 32) + return 1; + + unsigned char buf[256]; + + /* Use an SVE register to make the kernel start saving the SVE bank. */ + asm volatile ("mov z0.b, #255\n\t" + "str z0, %0" + : + : "m" (buf) + : "z0", "memory"); + + struct sigaction sigact; + sigact.sa_handler = sighandler; + sigaction (SIGUSR1, &sigact, NULL); + + kill (getpid (), SIGUSR1); + + return 0; +} diff --git a/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp new file mode 100644 index 000000000000..7fea93ffbd58 --- /dev/null +++ b/gdb/testsuite/gdb.arch/aarch64-sve-vg-sigunwind.exp @@ -0,0 +1,55 @@ +# Copyright 2025 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/>. + +# Exercise unwinding AArch64's Scalable Vector Extension VG register. + +require allow_aarch64_sve_tests + +standard_testfile +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ + [list debug additional_flags=-march=armv9-a]] } { + return +} + +# We want SIGUSR1 to be delivered normally. +gdb_test "handle SIGUSR1 nostop" \ + [multi_line {Signal Stop Print Pass to program Description} \ + {SIGUSR1 No Yes Yes User defined signal 1}] \ + "don't stop for SIGUSR1" + +set linespec ${srcfile}:[gdb_get_line_number "Break here."] + +if ![runto ${linespec}] { + return +} + +set max_vl [get_valueof "" "max_vl" 0] +set max_vg [expr $max_vl/8] + +# If maximum VL is less than 256 bits we can't decrease it in the signal +# handler and the test can't work. +if { $max_vl < 32 } { + untested "Maximum vector length is too small." + return +} + +set reduced_vg [expr "$max_vg - 2"] +gdb_test "print \$vg" ". = ${reduced_vg}" "vg was reduced" + +gdb_test "frame function main" \ + [multi_line "#$decimal $hex in main \[^\r\n\]+" \ + "$decimal\[ \t\]+kill \\(getpid \\(\\), SIGUSR1\\);"] + +gdb_test "print \$vg" ". = $max_vg" "vg was correctly unwound"