Re: [PATCH v8] tests/tcg/s390x: Test SIGILL and SIGSEGV handling

[David Hildenbrand]

On 05.08.21 11:57, Ilya Leoshkevich wrote:
> On Thu, 2021-08-05 at 11:37 +0200, David Hildenbrand wrote:
> > On 05.08.21 00:51, Ilya Leoshkevich wrote:
> > > Verify that s390x-specific uc_mcontext.psw.addr is reported
> > > correctly
> > > and that signal handling interacts properly with debugging.
> > >
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > >
> > > v7:
> > > https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg00463.html
> > > v7 -> v8: Another rebase needed due to the conflict with Jonathan's
> > >             50e36dd61652.
> > >
> > >    tests/tcg/s390x/Makefile.target               |  17 +-
> > >    tests/tcg/s390x/gdbstub/test-signals-s390x.py |  76 ++++++++
> > >    tests/tcg/s390x/signals-s390x.c               | 165
> > > ++++++++++++++++++
> > >    3 files changed, 257 insertions(+), 1 deletion(-)
> > >    create mode 100644 tests/tcg/s390x/gdbstub/test-signals-s390x.py
> > >    create mode 100644 tests/tcg/s390x/signals-s390x.c
> > >
> > > diff --git a/tests/tcg/s390x/Makefile.target
> > > b/tests/tcg/s390x/Makefile.target
> > > index bd084c7840..cc64dd32d2 100644
> > > --- a/tests/tcg/s390x/Makefile.target
> > > +++ b/tests/tcg/s390x/Makefile.target
> > > @@ -1,4 +1,5 @@
> > > -VPATH+=$(SRC_PATH)/tests/tcg/s390x
> > > +S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
> > > +VPATH+=$(S390X_SRC)
> > >    CFLAGS+=-march=zEC12 -m64
> > >    TESTS+=hello-s390x
> > >    TESTS+=csst
> > > @@ -9,3 +10,17 @@ TESTS+=pack
> > >    TESTS+=mvo
> > >    TESTS+=mvc
> > >    TESTS+=trap
> > > +TESTS+=signals-s390x
> > > +
> > > +ifneq ($(HAVE_GDB_BIN),)
> > > +GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
> > > +
> > > +run-gdbstub-signals-s390x: signals-s390x
> > > +       $(call run-test, $@, $(GDB_SCRIPT) \
> > > +               --gdb $(HAVE_GDB_BIN) \
> > > +               --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
> > > +               --bin $< --test $(S390X_SRC)/gdbstub/test-signals-
> > > s390x.py, \
> > > +       "mixing signals and debugging on s390x")
> > > +
> > > +EXTRA_RUNS += run-gdbstub-signals-s390x
> > > +endif
> > > diff --git a/tests/tcg/s390x/gdbstub/test-signals-s390x.py
> > > b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
> > > new file mode 100644
> > > index 0000000000..80a284b475
> > > --- /dev/null
> > > +++ b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
> > > @@ -0,0 +1,76 @@
> > > +from __future__ import print_function
> > > +
> > > +#
> > > +# Test that signals and debugging mix well together on s390x.
> > > +#
> > > +# This is launched via tests/guest-debug/run-test.py
> > > +#
> > > +
> > > +import gdb
> > > +import sys
> > > +
> > > +failcount = 0
> > > +
> > > +
> > > +def report(cond, msg):
> > > +    """Report success/fail of test"""
> > > +    if cond:
> > > +        print("PASS: %s" % (msg))
> > > +    else:
> > > +        print("FAIL: %s" % (msg))
> > > +        global failcount
> > > +        failcount += 1
> > > +
> > > +
> > > +def run_test():
> > > +    """Run through the tests one by one"""
> > > +    illegal_op = gdb.Breakpoint("illegal_op")
> > > +    stg = gdb.Breakpoint("stg")
> > > +    mvc_8 = gdb.Breakpoint("mvc_8")
> > > +
> > > +    # Expect the following events:
> > > +    # 1x illegal_op breakpoint
> > > +    # 2x stg breakpoint, segv, breakpoint
> > > +    # 2x mvc_8 breakpoint, segv, breakpoint
> > > +    for _ in range(14):
> >
> > How do we come up with the value 14?
>
> 1 (initial) + 1 (illegal op) + 2 * 3 (stg) + 2 * 3 (mvc_8).

Oh, that makes sense.

> > > +        gdb.execute("c")
> > > +    report(illegal_op.hit_count == 1, "illegal_op.hit_count == 1")
> > > +    report(stg.hit_count == 4, "stg.hit_count == 4")
> >
> > The doc above says we should see this twice, why do we see it 4
> > times?
>
> With "2x stg breakpoint, segv, breakpoint" I meant: stg break, stg
> segv, stg break, stg break, stg segv, stg break.

Understand it now, wasn't paying attention to the details :)

Thanks!

Acked-by: David Hildenbrand <david@redhat.com>

--
Thanks,

David / dhildenb