| Message ID | 20240303192610.498490-2-gustavo.romero@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] gdbstub: Add Xfer:siginfo:read stub | expand |
On 3/3/24 09:26, Gustavo Romero wrote: > Add multiarch test for testing if Xfer:siginfo:read query is properly > handled by gdbstub. > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> > --- > tests/tcg/multiarch/Makefile.target | 10 ++++++- > .../gdbstub/test-qxfer-siginfo-read.py | 26 +++++++++++++++++++ > tests/tcg/multiarch/segfault.c | 14 ++++++++++ > 3 files changed, 49 insertions(+), 1 deletion(-) > create mode 100644 tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py > create mode 100644 tests/tcg/multiarch/segfault.c > > diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target > index e10951a801..61cda9640e 100644 > --- a/tests/tcg/multiarch/Makefile.target > +++ b/tests/tcg/multiarch/Makefile.target > @@ -80,6 +80,13 @@ run-gdbstub-qxfer-auxv-read: sha1 > --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \ > basic gdbstub qXfer:auxv:read support) > > +run-gdbstub-qxfer-siginfo-read: segfault > + $(call run-test, $@, $(GDB_SCRIPT) \ > + --gdb $(GDB) \ > + --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ > + --bin "$< -s" --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-siginfo-read.py, \ > + basic gdbstub qXfer:siginfo:read support) > + > run-gdbstub-proc-mappings: sha1 > $(call run-test, $@, $(GDB_SCRIPT) \ > --gdb $(GDB) \ > @@ -122,7 +129,8 @@ endif > EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \ > run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \ > run-gdbstub-registers run-gdbstub-prot-none \ > - run-gdbstub-catch-syscalls > + run-gdbstub-catch-syscalls \ > + run-gdbstub-qxfer-siginfo-read > > # ARM Compatible Semi Hosting Tests > # > diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py > new file mode 100644 > index 0000000000..862596b07a > --- /dev/null > +++ b/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py > @@ -0,0 +1,26 @@ > +from __future__ import print_function > +# > +# Test gdbstub Xfer:siginfo:read stub. > +# > +# The test runs a binary that causes a SIGSEGV and then looks for additional > +# info about the signal through printing GDB's '$_siginfo' special variable, > +# which sends a Xfer:siginfo:read query to the gdbstub. > +# > +# The binary causes a SIGSEGV at dereferencing a pointer with value 0xdeadbeef, > +# so the test looks for and checks if this address is correctly reported by the > +# gdbstub. > +# > +# This is launched via tests/guest-debug/run-test.py > +# > + > +import gdb > +from test_gdbstub import main, report > + > +def run_test(): > + "Run through the test" > + > + gdb.execute("continue", False, True) > + resp = gdb.execute("print/x $_siginfo", False, True) > + report(resp.find("si_addr = 0xdeadbeef"), "Found fault address.") > + > +main(run_test) > diff --git a/tests/tcg/multiarch/segfault.c b/tests/tcg/multiarch/segfault.c > new file mode 100644 > index 0000000000..e6c8ff31ca > --- /dev/null > +++ b/tests/tcg/multiarch/segfault.c > @@ -0,0 +1,14 @@ > +#include <stdio.h> > +#include <string.h> > + > +/* Cause a segfault for testing purposes. */ > + > +int main(int argc, char *argv[]) > +{ > + int *ptr = (void *)0xdeadbeef; > + > + if (argc == 2 && strcmp(argv[1], "-s") == 0) { > + /* Cause segfault. */ > + printf("%d\n", *ptr); > + } > +} Any reason SIGSEGV is interesting? Perhaps just abort for SIGABRT instead? A test using setitimer to raise SIGALRM would test the async path. r~
Hi Richard! On 3/4/24 2:21 PM, Richard Henderson wrote: > On 3/3/24 09:26, Gustavo Romero wrote: >> Add multiarch test for testing if Xfer:siginfo:read query is properly >> handled by gdbstub. >> >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >> --- >> tests/tcg/multiarch/Makefile.target | 10 ++++++- >> .../gdbstub/test-qxfer-siginfo-read.py | 26 +++++++++++++++++++ >> tests/tcg/multiarch/segfault.c | 14 ++++++++++ >> 3 files changed, 49 insertions(+), 1 deletion(-) >> create mode 100644 tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py >> create mode 100644 tests/tcg/multiarch/segfault.c >> >> diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target >> index e10951a801..61cda9640e 100644 >> --- a/tests/tcg/multiarch/Makefile.target >> +++ b/tests/tcg/multiarch/Makefile.target >> @@ -80,6 +80,13 @@ run-gdbstub-qxfer-auxv-read: sha1 >> --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \ >> basic gdbstub qXfer:auxv:read support) >> +run-gdbstub-qxfer-siginfo-read: segfault >> + $(call run-test, $@, $(GDB_SCRIPT) \ >> + --gdb $(GDB) \ >> + --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ >> + --bin "$< -s" --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-siginfo-read.py, \ >> + basic gdbstub qXfer:siginfo:read support) >> + >> run-gdbstub-proc-mappings: sha1 >> $(call run-test, $@, $(GDB_SCRIPT) \ >> --gdb $(GDB) \ >> @@ -122,7 +129,8 @@ endif >> EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \ >> run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \ >> run-gdbstub-registers run-gdbstub-prot-none \ >> - run-gdbstub-catch-syscalls >> + run-gdbstub-catch-syscalls \ >> + run-gdbstub-qxfer-siginfo-read >> # ARM Compatible Semi Hosting Tests >> # >> diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py >> new file mode 100644 >> index 0000000000..862596b07a >> --- /dev/null >> +++ b/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py >> @@ -0,0 +1,26 @@ >> +from __future__ import print_function >> +# >> +# Test gdbstub Xfer:siginfo:read stub. >> +# >> +# The test runs a binary that causes a SIGSEGV and then looks for additional >> +# info about the signal through printing GDB's '$_siginfo' special variable, >> +# which sends a Xfer:siginfo:read query to the gdbstub. >> +# >> +# The binary causes a SIGSEGV at dereferencing a pointer with value 0xdeadbeef, >> +# so the test looks for and checks if this address is correctly reported by the >> +# gdbstub. >> +# >> +# This is launched via tests/guest-debug/run-test.py >> +# >> + >> +import gdb >> +from test_gdbstub import main, report >> + >> +def run_test(): >> + "Run through the test" >> + >> + gdb.execute("continue", False, True) >> + resp = gdb.execute("print/x $_siginfo", False, True) >> + report(resp.find("si_addr = 0xdeadbeef"), "Found fault address.") >> + >> +main(run_test) >> diff --git a/tests/tcg/multiarch/segfault.c b/tests/tcg/multiarch/segfault.c >> new file mode 100644 >> index 0000000000..e6c8ff31ca >> --- /dev/null >> +++ b/tests/tcg/multiarch/segfault.c >> @@ -0,0 +1,14 @@ >> +#include <stdio.h> >> +#include <string.h> >> + >> +/* Cause a segfault for testing purposes. */ >> + >> +int main(int argc, char *argv[]) >> +{ >> + int *ptr = (void *)0xdeadbeef; >> + >> + if (argc == 2 && strcmp(argv[1], "-s") == 0) { >> + /* Cause segfault. */ >> + printf("%d\n", *ptr); >> + } >> +} > > Any reason SIGSEGV is interesting? I'm particularly interested in the SIGSEGV because that's the signal generated on a MTE tag mismatch. GDB uses the si_code to show additional info on the fault, for instance: gromero@arm64:~$ gdb -q (gdb) target remote amd:1234 Remote debugging using amd:1234 Reading /home/gromero/git/qemu/build/mte_t from remote target... warning: File transfers from remote targets can be slow. Use "set sysroot" to access files locally instead. Reading /home/gromero/git/qemu/build/mte_t from remote target... Reading symbols from target:/home/gromero/git/qemu/build/mte_t... Failed to read a valid object file image from memory. 0x0000000000400580 in _start () (gdb) c Continuing. Program received signal SIGSEGV, Segmentation fault Memory tag violation <============ (the info I'm keen on) Fault address unavailable. 0x0000000000407290 in puts () (gdb) > Perhaps just abort for SIGABRT instead? Although this can make a simpler test, the test can't control the si_addr value easily, which I think is interesting to be tested. Why do you prefer SIGABRT? > A test using setitimer to raise SIGALRM would test the async path. SIGLARM doesn't generate any interesting siginfo? gromero@arm64:~$ gdb -q ./sigalrm Reading symbols from ./sigalrm... (gdb) run Starting program: /home/gromero/sigalrm Program terminated with signal SIGALRM, Alarm clock. The program no longer exists. (gdb) p $_siginfo $1 = void (gdb) Cheers, Gustavo
On 3/4/24 10:59, Gustavo Romero wrote: >> Perhaps just abort for SIGABRT instead? > > Although this can make a simpler test, the test can't control > the si_addr value easily, which I think is interesting to be tested. > > Why do you prefer SIGABRT? I missed that you were testing si_addr -- in which case SIGSEGV is a good match. >> A test using setitimer to raise SIGALRM would test the async path. > > SIGLARM doesn't generate any interesting siginfo? It should at minimum have si_sig = SIGALRM. > > gromero@arm64:~$ gdb -q ./sigalrm > Reading symbols from ./sigalrm... > (gdb) run > Starting program: /home/gromero/sigalrm > > Program terminated with signal SIGALRM, Alarm clock. > The program no longer exists. > (gdb) p $_siginfo > $1 = void Well that's because the program died. Do you need to have gdb handle the signal? r~
Hi Richard, On 3/4/24 7:51 PM, Richard Henderson wrote: > On 3/4/24 10:59, Gustavo Romero wrote: >>> Perhaps just abort for SIGABRT instead? >> >> Although this can make a simpler test, the test can't control >> the si_addr value easily, which I think is interesting to be tested. >> >> Why do you prefer SIGABRT? > > I missed that you were testing si_addr -- in which case SIGSEGV is a good match. > > >>> A test using setitimer to raise SIGALRM would test the async path. >> >> SIGLARM doesn't generate any interesting siginfo? > > It should at minimum have si_sig = SIGALRM. > >> >> gromero@arm64:~$ gdb -q ./sigalrm >> Reading symbols from ./sigalrm... >> (gdb) run >> Starting program: /home/gromero/sigalrm >> >> Program terminated with signal SIGALRM, Alarm clock. >> The program no longer exists. >> (gdb) p $_siginfo >> $1 = void > > Well that's because the program died. > Do you need to have gdb handle the signal? ouch, right :) However, on a remote target, even if I catch that signal using 'catch signal SIGALRM' the GDBstub only closes the connection when SIGALRM is delivered. That's odd, I don't understand why. I'm using the same binary that pretty much works on GDB locally. [Remote target] gromero@arm64:~$ gdb -q gromero@arm64:~/qemu_tests$ gdb -q ./sigalrm Reading symbols from ./sigalrm... (gdb) catch signal SIGALRM Catchpoint 1 (signal SIGALRM) (gdb) c The program is not being run. (gdb) run Starting program: /home/gromero/qemu_tests/sigalrm [Inferior 1 (process 12732) exited normally] (gdb) quit on the QEMU gdbstub side it reports "Alarm clock": gromero@amd:~/git/qemu/build$ ./qemu-aarch64 -g 1234 ./sigalrm -s Alarm clock gromero@amd:~/git/qemu/build$ [Locally] gromero@arm64:~/qemu_tests$ gdb -q ./sigalrm Reading symbols from ./sigalrm... (gdb) catch signal SIGALRM Catchpoint 1 (signal SIGALRM) (gdb) run -s Starting program: /home/gromero/qemu_tests/sigalrm -s Catchpoint 1 (signal SIGALRM), 0x000000000041a410 in ualarm () (gdb) quit I'd like to add for the async path using SIGALRM but I need more time to understand what's going on regarding SIGLARM. I understand that's nothing wrong with the Xfer:siginfo:read stub itself, and because the main goal of the test is to test the stub, if you don't mind, I'd like to keep only the test with SIGSEGV for v2 and leave the async test as a follow-up. Cheers, Gustavo
On 3/7/24 07:50, Gustavo Romero wrote: > Hi Richard, > > On 3/4/24 7:51 PM, Richard Henderson wrote: >> On 3/4/24 10:59, Gustavo Romero wrote: >>>> Perhaps just abort for SIGABRT instead? >>> >>> Although this can make a simpler test, the test can't control >>> the si_addr value easily, which I think is interesting to be tested. >>> >>> Why do you prefer SIGABRT? >> >> I missed that you were testing si_addr -- in which case SIGSEGV is a good match. >> >> >>>> A test using setitimer to raise SIGALRM would test the async path. >>> >>> SIGLARM doesn't generate any interesting siginfo? >> >> It should at minimum have si_sig = SIGALRM. >> >>> >>> gromero@arm64:~$ gdb -q ./sigalrm >>> Reading symbols from ./sigalrm... >>> (gdb) run >>> Starting program: /home/gromero/sigalrm >>> >>> Program terminated with signal SIGALRM, Alarm clock. >>> The program no longer exists. >>> (gdb) p $_siginfo >>> $1 = void >> >> Well that's because the program died. >> Do you need to have gdb handle the signal? > > ouch, right :) > > However, on a remote target, even if I catch that signal using > 'catch signal SIGALRM' the GDBstub only closes the connection > when SIGALRM is delivered. That's odd, I don't understand why. > > I'm using the same binary that pretty much works on GDB locally. > > > [Remote target] > > gromero@arm64:~$ gdb -q > gromero@arm64:~/qemu_tests$ gdb -q ./sigalrm > Reading symbols from ./sigalrm... > (gdb) catch signal SIGALRM > Catchpoint 1 (signal SIGALRM) > (gdb) c > The program is not being run. > (gdb) run > Starting program: /home/gromero/qemu_tests/sigalrm > [Inferior 1 (process 12732) exited normally] > (gdb) quit > > on the QEMU gdbstub side it reports "Alarm clock": > > gromero@amd:~/git/qemu/build$ ./qemu-aarch64 -g 1234 ./sigalrm -s > Alarm clock > gromero@amd:~/git/qemu/build$ > > > [Locally] > > gromero@arm64:~/qemu_tests$ gdb -q ./sigalrm > Reading symbols from ./sigalrm... > (gdb) catch signal SIGALRM > Catchpoint 1 (signal SIGALRM) > (gdb) run -s > Starting program: /home/gromero/qemu_tests/sigalrm -s > > Catchpoint 1 (signal SIGALRM), 0x000000000041a410 in ualarm () > (gdb) quit > > > I'd like to add for the async path using SIGALRM but I need more > time to understand what's going on regarding SIGLARM. I understand > that's nothing wrong with the Xfer:siginfo:read stub itself, and > because the main goal of the test is to test the stub, if you don't > mind, I'd like to keep only the test with SIGSEGV for v2 and leave > the async test as a follow-up. Well that's certainly surprising. Would you please file a bug report about this? I think I know what the problem is, but let's track it anyway. r~
On 3/7/24 4:31 PM, Richard Henderson wrote: > On 3/7/24 07:50, Gustavo Romero wrote: >> Hi Richard, >> >> On 3/4/24 7:51 PM, Richard Henderson wrote: >>> On 3/4/24 10:59, Gustavo Romero wrote: >>>>> Perhaps just abort for SIGABRT instead? >>>> >>>> Although this can make a simpler test, the test can't control >>>> the si_addr value easily, which I think is interesting to be tested. >>>> >>>> Why do you prefer SIGABRT? >>> >>> I missed that you were testing si_addr -- in which case SIGSEGV is a good match. >>> >>> >>>>> A test using setitimer to raise SIGALRM would test the async path. >>>> >>>> SIGLARM doesn't generate any interesting siginfo? >>> >>> It should at minimum have si_sig = SIGALRM. >>> >>>> >>>> gromero@arm64:~$ gdb -q ./sigalrm >>>> Reading symbols from ./sigalrm... >>>> (gdb) run >>>> Starting program: /home/gromero/sigalrm >>>> >>>> Program terminated with signal SIGALRM, Alarm clock. >>>> The program no longer exists. >>>> (gdb) p $_siginfo >>>> $1 = void >>> >>> Well that's because the program died. >>> Do you need to have gdb handle the signal? >> >> ouch, right :) >> >> However, on a remote target, even if I catch that signal using >> 'catch signal SIGALRM' the GDBstub only closes the connection >> when SIGALRM is delivered. That's odd, I don't understand why. >> >> I'm using the same binary that pretty much works on GDB locally. >> >> >> [Remote target] >> >> gromero@arm64:~$ gdb -q >> gromero@arm64:~/qemu_tests$ gdb -q ./sigalrm >> Reading symbols from ./sigalrm... >> (gdb) catch signal SIGALRM >> Catchpoint 1 (signal SIGALRM) >> (gdb) c >> The program is not being run. >> (gdb) run >> Starting program: /home/gromero/qemu_tests/sigalrm >> [Inferior 1 (process 12732) exited normally] >> (gdb) quit >> >> on the QEMU gdbstub side it reports "Alarm clock": >> >> gromero@amd:~/git/qemu/build$ ./qemu-aarch64 -g 1234 ./sigalrm -s >> Alarm clock >> gromero@amd:~/git/qemu/build$ >> >> >> [Locally] >> >> gromero@arm64:~/qemu_tests$ gdb -q ./sigalrm >> Reading symbols from ./sigalrm... >> (gdb) catch signal SIGALRM >> Catchpoint 1 (signal SIGALRM) >> (gdb) run -s >> Starting program: /home/gromero/qemu_tests/sigalrm -s >> >> Catchpoint 1 (signal SIGALRM), 0x000000000041a410 in ualarm () >> (gdb) quit >> >> >> I'd like to add for the async path using SIGALRM but I need more >> time to understand what's going on regarding SIGLARM. I understand >> that's nothing wrong with the Xfer:siginfo:read stub itself, and >> because the main goal of the test is to test the stub, if you don't >> mind, I'd like to keep only the test with SIGSEGV for v2 and leave >> the async test as a follow-up. > > Well that's certainly surprising. > Would you please file a bug report about this? > I think I know what the problem is, but let's track it anyway. Yeah.. I filed an issue here: https://gitlab.com/qemu-project/qemu/-/issues/2214 Cheers, Gustavo
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target index e10951a801..61cda9640e 100644 --- a/tests/tcg/multiarch/Makefile.target +++ b/tests/tcg/multiarch/Makefile.target @@ -80,6 +80,13 @@ run-gdbstub-qxfer-auxv-read: sha1 --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \ basic gdbstub qXfer:auxv:read support) +run-gdbstub-qxfer-siginfo-read: segfault + $(call run-test, $@, $(GDB_SCRIPT) \ + --gdb $(GDB) \ + --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \ + --bin "$< -s" --test $(MULTIARCH_SRC)/gdbstub/test-qxfer-siginfo-read.py, \ + basic gdbstub qXfer:siginfo:read support) + run-gdbstub-proc-mappings: sha1 $(call run-test, $@, $(GDB_SCRIPT) \ --gdb $(GDB) \ @@ -122,7 +129,8 @@ endif EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \ run-gdbstub-proc-mappings run-gdbstub-thread-breakpoint \ run-gdbstub-registers run-gdbstub-prot-none \ - run-gdbstub-catch-syscalls + run-gdbstub-catch-syscalls \ + run-gdbstub-qxfer-siginfo-read # ARM Compatible Semi Hosting Tests # diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py new file mode 100644 index 0000000000..862596b07a --- /dev/null +++ b/tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py @@ -0,0 +1,26 @@ +from __future__ import print_function +# +# Test gdbstub Xfer:siginfo:read stub. +# +# The test runs a binary that causes a SIGSEGV and then looks for additional +# info about the signal through printing GDB's '$_siginfo' special variable, +# which sends a Xfer:siginfo:read query to the gdbstub. +# +# The binary causes a SIGSEGV at dereferencing a pointer with value 0xdeadbeef, +# so the test looks for and checks if this address is correctly reported by the +# gdbstub. +# +# This is launched via tests/guest-debug/run-test.py +# + +import gdb +from test_gdbstub import main, report + +def run_test(): + "Run through the test" + + gdb.execute("continue", False, True) + resp = gdb.execute("print/x $_siginfo", False, True) + report(resp.find("si_addr = 0xdeadbeef"), "Found fault address.") + +main(run_test) diff --git a/tests/tcg/multiarch/segfault.c b/tests/tcg/multiarch/segfault.c new file mode 100644 index 0000000000..e6c8ff31ca --- /dev/null +++ b/tests/tcg/multiarch/segfault.c @@ -0,0 +1,14 @@ +#include <stdio.h> +#include <string.h> + +/* Cause a segfault for testing purposes. */ + +int main(int argc, char *argv[]) +{ + int *ptr = (void *)0xdeadbeef; + + if (argc == 2 && strcmp(argv[1], "-s") == 0) { + /* Cause segfault. */ + printf("%d\n", *ptr); + } +}
Add multiarch test for testing if Xfer:siginfo:read query is properly handled by gdbstub. Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> --- tests/tcg/multiarch/Makefile.target | 10 ++++++- .../gdbstub/test-qxfer-siginfo-read.py | 26 +++++++++++++++++++ tests/tcg/multiarch/segfault.c | 14 ++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/multiarch/gdbstub/test-qxfer-siginfo-read.py create mode 100644 tests/tcg/multiarch/segfault.c