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

[David Hildenbrand <david@redhat.com>]

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