Re: [PATCH 2/2] tests/tcg/s390x: Test SIGILL handling

[David Hildenbrand]

On 21.05.21 05:01, Ilya Leoshkevich wrote:
> Verify that s390x-specific uc_mcontext.psw.addr is reported correctly.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/Makefile.target |  1 +
>   tests/tcg/s390x/sigill.c        | 41 +++++++++++++++++++++++++++++++++
>   2 files changed, 42 insertions(+)
>   create mode 100644 tests/tcg/s390x/sigill.c
>
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 241ef28f61..8699d829a5 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -8,3 +8,4 @@ TESTS+=exrl-trtr
>   TESTS+=pack
>   TESTS+=mvo
>   TESTS+=mvc
> +TESTS+=sigill
> diff --git a/tests/tcg/s390x/sigill.c b/tests/tcg/s390x/sigill.c
> new file mode 100644
> index 0000000000..f8021dc6af
> --- /dev/null
> +++ b/tests/tcg/s390x/sigill.c
> @@ -0,0 +1,41 @@
> +#include <assert.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <ucontext.h>
> +#include <unistd.h>
> +
> +extern char expected_si_addr[];
> +extern char expected_psw_addr[];

Why "extern" ? For the magic inline asm below to work?

> +
> +static void handle_signal(int sig, siginfo_t *info, void *ucontext)
> +{
> +    if (sig != SIGILL) {
> +        _exit(1);
> +    }
> +
> +    if (info->si_addr != expected_si_addr) {
> +        _exit(2);
> +    }
> +
> +    if (((ucontext_t *)ucontext)->uc_mcontext.psw.addr !=
> +            (unsigned long)expected_psw_addr) {
> +        _exit(3);
> +    }
> +}
> +
> +int main(void)
> +{
> +    struct sigaction act;
> +
> +    memset(&act, 0, sizeof(act));
> +    act.sa_sigaction = handle_signal;
> +    act.sa_flags = SA_SIGINFO;
> +
> +    int ret = sigaction(SIGILL, &act, NULL);

Mixing code and declaration.

> +    assert(ret == 0);
> +
> +    asm volatile("expected_si_addr:\t.byte\t0x00,0x00\n"
> +                 "expected_psw_addr:");

At least I am confused how the right values actually end up in 
expected_si_addr and expected_psw_addr.

Can we maybe add a comment? This looks quite hacky ;)

--
Thanks,

David / dhildenb