1. Patchew
  2. QEMU
  3. View series

[PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting

Ilya Leoshkevich posted 2 patches 2 years, 10 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
linux-user/s390x/cpu_loop.c     |  23 ++++-
target/s390x/excp_helper.c      |  71 +++++++-------
target/s390x/internal.h         |   1 +
target/s390x/mem_helper.c       |   2 +-
tests/tcg/s390x/Makefile.target |   1 +
tests/tcg/s390x/signal.c        | 163 ++++++++++++++++++++++++++++++++
6 files changed, 224 insertions(+), 37 deletions(-)
create mode 100644 tests/tcg/s390x/signal.c
[PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by Ilya Leoshkevich 2 years, 10 months ago 
qemu-s390x puts a wrong value into SIGILL's siginfo_t's psw.addr: it
should be a pointer to the instruction following the illegal
instruction, but at the moment it is a pointer to the illegal
instruction itself. This breaks OpenJDK, which relies on this value.

Patch 1 fixes the issue, patch 2 adds a test.

v1: https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
v1 -> v2: Use a better buglink (Cornelia), simplify the inline asm
          magic in the test and add an explanation (David).

v2: https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
v2 -> v3: Fix SIGSEGV handling (found when trying to run valgrind under
          qemu-user).

Ilya Leoshkevich (2):
  target/s390x: Fix SIGILL psw.addr reporting
  tests/tcg/s390x: Test SIGILL and SIGSEGV handling

 linux-user/s390x/cpu_loop.c     |  23 ++++-
 target/s390x/excp_helper.c      |  71 +++++++-------
 target/s390x/internal.h         |   1 +
 target/s390x/mem_helper.c       |   2 +-
 tests/tcg/s390x/Makefile.target |   1 +
 tests/tcg/s390x/signal.c        | 163 ++++++++++++++++++++++++++++++++
 6 files changed, 224 insertions(+), 37 deletions(-)
 create mode 100644 tests/tcg/s390x/signal.c

-- 
2.31.1
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by no-reply@patchew.org 2 years, 10 months ago 
Patchew URL: https://patchew.org/QEMU/20210602002210.3144559-1-iii@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210602002210.3144559-1-iii@linux.ibm.com
Subject: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/1622589584-22571-1-git-send-email-tsimpson@quicinc.com -> patchew/1622589584-22571-1-git-send-email-tsimpson@quicinc.com
 * [new tag]         patchew/20210602002210.3144559-1-iii@linux.ibm.com -> patchew/20210602002210.3144559-1-iii@linux.ibm.com
Switched to a new branch 'test'
495bc5c tests/tcg/s390x: Test SIGILL and SIGSEGV handling
2e28eb9 target/s390x: Fix SIGILL psw.addr reporting

=== OUTPUT BEGIN ===
1/2 Checking commit 2e28eb9526d2 (target/s390x: Fix SIGILL psw.addr reporting)
2/2 Checking commit 495bc5cfd59a (tests/tcg/s390x: Test SIGILL and SIGSEGV handling)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

ERROR: externs should be avoided in .c files
#44: FILE: tests/tcg/s390x/signal.c:14:
+void illegal_op(void);

ERROR: externs should be avoided in .c files
#45: FILE: tests/tcg/s390x/signal.c:15:
+void after_illegal_op(void);

ERROR: externs should be avoided in .c files
#51: FILE: tests/tcg/s390x/signal.c:21:
+void stg(void *dst, unsigned long src);

ERROR: externs should be avoided in .c files
#56: FILE: tests/tcg/s390x/signal.c:26:
+void mvc_8(void *dst, void *src);

total: 4 errors, 1 warnings, 167 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210602002210.3144559-1-iii@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by David Hildenbrand 2 years, 10 months ago 
On 02.06.21 02:22, Ilya Leoshkevich wrote:
> qemu-s390x puts a wrong value into SIGILL's siginfo_t's psw.addr: it
> should be a pointer to the instruction following the illegal
> instruction, but at the moment it is a pointer to the illegal
> instruction itself. This breaks OpenJDK, which relies on this value.
> 
> Patch 1 fixes the issue, patch 2 adds a test.
> 
> v1: https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
> v1 -> v2: Use a better buglink (Cornelia), simplify the inline asm
>            magic in the test and add an explanation (David).
> 
> v2: https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
> v2 -> v3: Fix SIGSEGV handling (found when trying to run valgrind under
>            qemu-user).
> 

There might still be something wrong:

https://gitlab.com/qemu-project/qemu/-/issues/319

At least it smells like some more signal (mis)handling.


-- 
Thanks,

David / dhildenb
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by Christian Borntraeger 2 years, 10 months ago 

On 10.06.21 11:49, David Hildenbrand wrote:
> On 02.06.21 02:22, Ilya Leoshkevich wrote:
>> qemu-s390x puts a wrong value into SIGILL's siginfo_t's psw.addr: it
>> should be a pointer to the instruction following the illegal
>> instruction, but at the moment it is a pointer to the illegal
>> instruction itself. This breaks OpenJDK, which relies on this value.
>>
>> Patch 1 fixes the issue, patch 2 adds a test.
>>
>> v1: https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
>> v1 -> v2: Use a better buglink (Cornelia), simplify the inline asm
>>            magic in the test and add an explanation (David).
>>
>> v2: https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
>> v2 -> v3: Fix SIGSEGV handling (found when trying to run valgrind under
>>            qemu-user).
>>
> 
> There might still be something wrong:
> 
> https://gitlab.com/qemu-project/qemu/-/issues/319
> 
> At least it smells like some more signal (mis)handling.

Yes there is more. Expect a patch (in a different area of signals)
from the ecosystem team soon.
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by Ilya Leoshkevich 2 years, 10 months ago 
On Thu, 2021-06-10 at 11:49 +0200, David Hildenbrand wrote:
> On 02.06.21 02:22, Ilya Leoshkevich wrote:
> > qemu-s390x puts a wrong value into SIGILL's siginfo_t's psw.addr:
> > it
> > should be a pointer to the instruction following the illegal
> > instruction, but at the moment it is a pointer to the illegal
> > instruction itself. This breaks OpenJDK, which relies on this
> > value.
> > 
> > Patch 1 fixes the issue, patch 2 adds a test.
> > 
> > v1:
> > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
> > v1 -> v2: Use a better buglink (Cornelia), simplify the inline asm
> >            magic in the test and add an explanation (David).
> > 
> > v2:
> > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
> > v2 -> v3: Fix SIGSEGV handling (found when trying to run valgrind
> > under
> >            qemu-user).
> > 
> 
> There might still be something wrong:
> 
> https://gitlab.com/qemu-project/qemu/-/issues/319
> 
> At least it smells like some more signal (mis)handling.
> 
> 

I've taken another look, and it must be compare-and-trap SIGFPE/SIGILL
mixup. I think I will just fix it here in v4.

Best regards,
Ilya
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by jonathan.albrecht 2 years, 10 months ago 
On 2021-06-21 8:00 am, Ilya Leoshkevich wrote:
> On Thu, 2021-06-10 at 11:49 +0200, David Hildenbrand wrote:
>> On 02.06.21 02:22, Ilya Leoshkevich wrote:
>> > qemu-s390x puts a wrong value into SIGILL's siginfo_t's psw.addr:
>> > it
>> > should be a pointer to the instruction following the illegal
>> > instruction, but at the moment it is a pointer to the illegal
>> > instruction itself. This breaks OpenJDK, which relies on this
>> > value.
>> >
>> > Patch 1 fixes the issue, patch 2 adds a test.
>> >
>> > v1:
>> > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
>> > v1 -> v2: Use a better buglink (Cornelia), simplify the inline asm
>> >            magic in the test and add an explanation (David).
>> >
>> > v2:
>> > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
>> > v2 -> v3: Fix SIGSEGV handling (found when trying to run valgrind
>> > under
>> >            qemu-user).
>> >
>> 
>> There might still be something wrong:
>> 
>> https://gitlab.com/qemu-project/qemu/-/issues/319
>> 
>> At least it smells like some more signal (mis)handling.
>> 
>> 
> 
> I've taken another look, and it must be compare-and-trap SIGFPE/SIGILL
> mixup. I think I will just fix it here in v4.

Yes, I've been looking at it too and found it is a compare-and-trap
SIGFPE/SIGILL mixup. I was about to send out a patch if you want
to wait. I should be able to send it out in an hour.

Jon

> 
> Best regards,
> Ilya
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by Ilya Leoshkevich 2 years, 10 months ago 
On Mon, 2021-06-21 at 09:12 -0400, jonathan.albrecht wrote:
> On 2021-06-21 8:00 am, Ilya Leoshkevich wrote:
> > On Thu, 2021-06-10 at 11:49 +0200, David Hildenbrand wrote:
> > > On 02.06.21 02:22, Ilya Leoshkevich wrote:
> > > > qemu-s390x puts a wrong value into SIGILL's siginfo_t's
> > > > psw.addr:
> > > > it
> > > > should be a pointer to the instruction following the illegal
> > > > instruction, but at the moment it is a pointer to the illegal
> > > > instruction itself. This breaks OpenJDK, which relies on this
> > > > value.
> > > > 
> > > > Patch 1 fixes the issue, patch 2 adds a test.
> > > > 
> > > > v1:
> > > > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
> > > > v1 -> v2: Use a better buglink (Cornelia), simplify the inline
> > > > asm
> > > >            magic in the test and add an explanation (David).
> > > > 
> > > > v2:
> > > > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
> > > > v2 -> v3: Fix SIGSEGV handling (found when trying to run
> > > > valgrind
> > > > under
> > > >            qemu-user).
> > > > 
> > > 
> > > There might still be something wrong:
> > > 
> > > https://gitlab.com/qemu-project/qemu/-/issues/319
> > > 
> > > At least it smells like some more signal (mis)handling.
> > > 
> > > 
> > 
> > I've taken another look, and it must be compare-and-trap
> > SIGFPE/SIGILL
> > mixup. I think I will just fix it here in v4.
> 
> Yes, I've been looking at it too and found it is a compare-and-trap
> SIGFPE/SIGILL mixup. I was about to send out a patch if you want
> to wait. I should be able to send it out in an hour.
> 
> Jon

Sure, please go ahead. I'll simply rebase my v4 on top of your patch
then.

Best regards,
Ilya
Re: [PATCH v3 0/2] target/s390x: Fix SIGILL psw.addr reporting
Posted by jonathan.albrecht 2 years, 10 months ago 
On 2021-06-21 9:44 am, Ilya Leoshkevich wrote:
> On Mon, 2021-06-21 at 09:12 -0400, jonathan.albrecht wrote:
>> On 2021-06-21 8:00 am, Ilya Leoshkevich wrote:
>> > On Thu, 2021-06-10 at 11:49 +0200, David Hildenbrand wrote:
>> > > On 02.06.21 02:22, Ilya Leoshkevich wrote:
>> > > > qemu-s390x puts a wrong value into SIGILL's siginfo_t's
>> > > > psw.addr:
>> > > > it
>> > > > should be a pointer to the instruction following the illegal
>> > > > instruction, but at the moment it is a pointer to the illegal
>> > > > instruction itself. This breaks OpenJDK, which relies on this
>> > > > value.
>> > > >
>> > > > Patch 1 fixes the issue, patch 2 adds a test.
>> > > >
>> > > > v1:
>> > > > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06592.html
>> > > > v1 -> v2: Use a better buglink (Cornelia), simplify the inline
>> > > > asm
>> > > >            magic in the test and add an explanation (David).
>> > > >
>> > > > v2:
>> > > > https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06649.html
>> > > > v2 -> v3: Fix SIGSEGV handling (found when trying to run
>> > > > valgrind
>> > > > under
>> > > >            qemu-user).
>> > > >
>> > >
>> > > There might still be something wrong:
>> > >
>> > > https://gitlab.com/qemu-project/qemu/-/issues/319
>> > >
>> > > At least it smells like some more signal (mis)handling.
>> > >
>> > >
>> >
>> > I've taken another look, and it must be compare-and-trap
>> > SIGFPE/SIGILL
>> > mixup. I think I will just fix it here in v4.
>> 
>> Yes, I've been looking at it too and found it is a compare-and-trap
>> SIGFPE/SIGILL mixup. I was about to send out a patch if you want
>> to wait. I should be able to send it out in an hour.
>> 
>> Jon
> 
> Sure, please go ahead. I'll simply rebase my v4 on top of your patch
> then.
> 
> Best regards,
> Ilya

Yes, please add it to your v4 if it looks ok.

Jon

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.