1. Patchew
  2. QEMU
  3. View series

[PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c

Ilya Leoshkevich posted 1 patch 3 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220502164830.1622191-1-iii@linux.ibm.com
Maintainers: Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>
tests/tcg/s390x/branch-relative-long.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Posted by Ilya Leoshkevich 3 years, 9 months ago 
Binutils >=2.37 and Clang do not accept (. - 0x100000000) PCRel32
constants. While this looks like a bug that needs fixing, use a
different notation (-0x100000000) as a workaround.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/branch-relative-long.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/s390x/branch-relative-long.c b/tests/tcg/s390x/branch-relative-long.c
index 94219afcad..8ce9f1c2e5 100644
--- a/tests/tcg/s390x/branch-relative-long.c
+++ b/tests/tcg/s390x/branch-relative-long.c
@@ -13,8 +13,8 @@
         #_name "_end:\n");
 
 DEFINE_ASM(br_r14, "br %r14");
-DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
-DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
+DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
+DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
 
 struct test {
     const char *code;
-- 
2.35.1
Re: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Posted by Thomas Huth 3 years, 9 months ago 
On 02/05/2022 18.48, Ilya Leoshkevich wrote:
> Binutils >=2.37 and Clang do not accept (. - 0x100000000) PCRel32
> constants. While this looks like a bug that needs fixing, use a
> different notation (-0x100000000) as a workaround.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/branch-relative-long.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/tcg/s390x/branch-relative-long.c b/tests/tcg/s390x/branch-relative-long.c
> index 94219afcad..8ce9f1c2e5 100644
> --- a/tests/tcg/s390x/branch-relative-long.c
> +++ b/tests/tcg/s390x/branch-relative-long.c
> @@ -13,8 +13,8 @@
>           #_name "_end:\n");
>   
>   DEFINE_ASM(br_r14, "br %r14");
> -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
> -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
> +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
> +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");

Works for me, thanks!

Tested-by: Thomas Huth <thuth@redhat.com>

and queued to my s390x-next branch:

  https://gitlab.com/thuth/qemu/-/commits/s390x-next/

  Thomas
Re: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Posted by Thomas Huth 3 years, 9 months ago 
On 03/05/2022 11.02, Thomas Huth wrote:
> On 02/05/2022 18.48, Ilya Leoshkevich wrote:
>> Binutils >=2.37 and Clang do not accept (. - 0x100000000) PCRel32
>> constants. While this looks like a bug that needs fixing, use a
>> different notation (-0x100000000) as a workaround.
>>
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>> ---
>>   tests/tcg/s390x/branch-relative-long.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/tcg/s390x/branch-relative-long.c 
>> b/tests/tcg/s390x/branch-relative-long.c
>> index 94219afcad..8ce9f1c2e5 100644
>> --- a/tests/tcg/s390x/branch-relative-long.c
>> +++ b/tests/tcg/s390x/branch-relative-long.c
>> @@ -13,8 +13,8 @@
>>           #_name "_end:\n");
>>   DEFINE_ASM(br_r14, "br %r14");
>> -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
>> -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
>> +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
>> +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
> 
> Works for me, thanks!

Sorry, I spoke too soon - it compiles fine, and also runs fine when I run it 
natively, but when I run it through "qemu-s390x", it crashes... does that 
work for you?

  Thomas
Re: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Posted by Ilya Leoshkevich 3 years, 9 months ago 
On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
> On 03/05/2022 11.02, Thomas Huth wrote:
> > On 02/05/2022 18.48, Ilya Leoshkevich wrote:
> > > Binutils >=2.37 and Clang do not accept (. - 0x100000000) PCRel32
> > > constants. While this looks like a bug that needs fixing, use a
> > > different notation (-0x100000000) as a workaround.
> > > 
> > > Reported-by: Thomas Huth <thuth@redhat.com>
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > >   tests/tcg/s390x/branch-relative-long.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/tcg/s390x/branch-relative-long.c 
> > > b/tests/tcg/s390x/branch-relative-long.c
> > > index 94219afcad..8ce9f1c2e5 100644
> > > --- a/tests/tcg/s390x/branch-relative-long.c
> > > +++ b/tests/tcg/s390x/branch-relative-long.c
> > > @@ -13,8 +13,8 @@
> > >           #_name "_end:\n");
> > >   DEFINE_ASM(br_r14, "br %r14");
> > > -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
> > > -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
> > > +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
> > > +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
> > 
> > Works for me, thanks!
> 
> Sorry, I spoke too soon - it compiles fine, and also runs fine when I
> run it 
> natively, but when I run it through "qemu-s390x", it crashes... does
> that 
> work for you?

Hi, yes, I just double-checked - it works fine for me.
Could you please share the resulting test binary?
Re: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Posted by Thomas Huth 3 years, 9 months ago 
On 04/05/2022 00.46, Ilya Leoshkevich wrote:
> On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
>> On 03/05/2022 11.02, Thomas Huth wrote:
>>> On 02/05/2022 18.48, Ilya Leoshkevich wrote:
>>>> Binutils >=2.37 and Clang do not accept (. - 0x100000000) PCRel32
>>>> constants. While this looks like a bug that needs fixing, use a
>>>> different notation (-0x100000000) as a workaround.
>>>>
>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>>> ---
>>>>    tests/tcg/s390x/branch-relative-long.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tests/tcg/s390x/branch-relative-long.c
>>>> b/tests/tcg/s390x/branch-relative-long.c
>>>> index 94219afcad..8ce9f1c2e5 100644
>>>> --- a/tests/tcg/s390x/branch-relative-long.c
>>>> +++ b/tests/tcg/s390x/branch-relative-long.c
>>>> @@ -13,8 +13,8 @@
>>>>            #_name "_end:\n");
>>>>    DEFINE_ASM(br_r14, "br %r14");
>>>> -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
>>>> -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
>>>> +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
>>>> +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
>>>
>>> Works for me, thanks!
>>
>> Sorry, I spoke too soon - it compiles fine, and also runs fine when I
>> run it
>> natively, but when I run it through "qemu-s390x", it crashes... does
>> that
>> work for you?
> 
> Hi, yes, I just double-checked - it works fine for me.
> Could you please share the resulting test binary?


Sure, here it is:

https://people.redhat.com/~thuth/data/branch-relative-long

  Thomas
Re: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Posted by Ilya Leoshkevich 3 years, 9 months ago 
On Wed, 2022-05-04 at 09:01 +0200, Thomas Huth wrote:
> On 04/05/2022 00.46, Ilya Leoshkevich wrote:
> > On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
> > > On 03/05/2022 11.02, Thomas Huth wrote:
> > > > On 02/05/2022 18.48, Ilya Leoshkevich wrote:
> > > > > Binutils >=2.37 and Clang do not accept (. - 0x100000000)
> > > > > PCRel32
> > > > > constants. While this looks like a bug that needs fixing, use
> > > > > a
> > > > > different notation (-0x100000000) as a workaround.
> > > > > 
> > > > > Reported-by: Thomas Huth <thuth@redhat.com>
> > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > > ---
> > > > >    tests/tcg/s390x/branch-relative-long.c | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/tests/tcg/s390x/branch-relative-long.c
> > > > > b/tests/tcg/s390x/branch-relative-long.c
> > > > > index 94219afcad..8ce9f1c2e5 100644
> > > > > --- a/tests/tcg/s390x/branch-relative-long.c
> > > > > +++ b/tests/tcg/s390x/branch-relative-long.c
> > > > > @@ -13,8 +13,8 @@
> > > > >            #_name "_end:\n");
> > > > >    DEFINE_ASM(br_r14, "br %r14");
> > > > > -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
> > > > > -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
> > > > > +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
> > > > > +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
> > > > 
> > > > Works for me, thanks!
> > > 
> > > Sorry, I spoke too soon - it compiles fine, and also runs fine
> > > when I
> > > run it
> > > natively, but when I run it through "qemu-s390x", it crashes...
> > > does
> > > that
> > > work for you?
> > 
> > Hi, yes, I just double-checked - it works fine for me.
> > Could you please share the resulting test binary?
> 
> 
> Sure, here it is:
> 
> https://people.redhat.com/~thuth/data/branch-relative-long
> 
>   Thomas

Your binary worked fine for me.

QEMU commit 2e3408b3cc7de4e87a9adafc8c19bfce3abec947,
x86_64 host,
configured with --target-list=s390x-linux-user.
branch-relative-long fails on s390x host (was: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c)
Posted by Thomas Huth 3 years, 9 months ago 
On 04/05/2022 11.07, Ilya Leoshkevich wrote:
> On Wed, 2022-05-04 at 09:01 +0200, Thomas Huth wrote:
>> On 04/05/2022 00.46, Ilya Leoshkevich wrote:
>>> On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
>>>> On 03/05/2022 11.02, Thomas Huth wrote:
>>>>> On 02/05/2022 18.48, Ilya Leoshkevich wrote:
>>>>>> Binutils >=2.37 and Clang do not accept (. - 0x100000000)
>>>>>> PCRel32
>>>>>> constants. While this looks like a bug that needs fixing, use
>>>>>> a
>>>>>> different notation (-0x100000000) as a workaround.
>>>>>>
>>>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>>>>> ---
>>>>>>     tests/tcg/s390x/branch-relative-long.c | 4 ++--
>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/tests/tcg/s390x/branch-relative-long.c
>>>>>> b/tests/tcg/s390x/branch-relative-long.c
>>>>>> index 94219afcad..8ce9f1c2e5 100644
>>>>>> --- a/tests/tcg/s390x/branch-relative-long.c
>>>>>> +++ b/tests/tcg/s390x/branch-relative-long.c
>>>>>> @@ -13,8 +13,8 @@
>>>>>>             #_name "_end:\n");
>>>>>>     DEFINE_ASM(br_r14, "br %r14");
>>>>>> -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
>>>>>> -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
>>>>>> +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
>>>>>> +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
>>>>>
>>>>> Works for me, thanks!
>>>>
>>>> Sorry, I spoke too soon - it compiles fine, and also runs fine
>>>> when I
>>>> run it
>>>> natively, but when I run it through "qemu-s390x", it crashes...
>>>> does
>>>> that
>>>> work for you?
>>>
>>> Hi, yes, I just double-checked - it works fine for me.
>>> Could you please share the resulting test binary?
>>
>>
>> Sure, here it is:
>>
>> https://people.redhat.com/~thuth/data/branch-relative-long
>>
>>    Thomas
> 
> Your binary worked fine for me.
> 
> QEMU commit 2e3408b3cc7de4e87a9adafc8c19bfce3abec947,
> x86_64 host,

Oh, well, now that you've mentioned it: I was running "make check-tcg" on a 
s390x host. It works fine on a x86, indeed. So the new problem is likely in 
the s390x TCG host backend... Richard, could you maybe have a look?

  Thomas
Re: branch-relative-long fails on s390x host (was: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c)
Posted by Ilya Leoshkevich 3 years, 9 months ago 
On Wed, 2022-05-04 at 11:14 +0200, Thomas Huth wrote:
> On 04/05/2022 11.07, Ilya Leoshkevich wrote:
> > On Wed, 2022-05-04 at 09:01 +0200, Thomas Huth wrote:
> > > On 04/05/2022 00.46, Ilya Leoshkevich wrote:
> > > > On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
> > > > > On 03/05/2022 11.02, Thomas Huth wrote:
> > > > > > On 02/05/2022 18.48, Ilya Leoshkevich wrote:
> > > > > > > Binutils >=2.37 and Clang do not accept (. - 0x100000000)
> > > > > > > PCRel32
> > > > > > > constants. While this looks like a bug that needs fixing,
> > > > > > > use
> > > > > > > a
> > > > > > > different notation (-0x100000000) as a workaround.
> > > > > > > 
> > > > > > > Reported-by: Thomas Huth <thuth@redhat.com>
> > > > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > > > > ---
> > > > > > >     tests/tcg/s390x/branch-relative-long.c | 4 ++--
> > > > > > >     1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/tests/tcg/s390x/branch-relative-long.c
> > > > > > > b/tests/tcg/s390x/branch-relative-long.c
> > > > > > > index 94219afcad..8ce9f1c2e5 100644
> > > > > > > --- a/tests/tcg/s390x/branch-relative-long.c
> > > > > > > +++ b/tests/tcg/s390x/branch-relative-long.c
> > > > > > > @@ -13,8 +13,8 @@
> > > > > > >             #_name "_end:\n");
> > > > > > >     DEFINE_ASM(br_r14, "br %r14");
> > > > > > > -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
> > > > > > > -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
> > > > > > > +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
> > > > > > > +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
> > > > > > 
> > > > > > Works for me, thanks!
> > > > > 
> > > > > Sorry, I spoke too soon - it compiles fine, and also runs
> > > > > fine
> > > > > when I
> > > > > run it
> > > > > natively, but when I run it through "qemu-s390x", it
> > > > > crashes...
> > > > > does
> > > > > that
> > > > > work for you?
> > > > 
> > > > Hi, yes, I just double-checked - it works fine for me.
> > > > Could you please share the resulting test binary?
> > > 
> > > 
> > > Sure, here it is:
> > > 
> > > https://people.redhat.com/~thuth/data/branch-relative-long
> > > 
> > >    Thomas
> > 
> > Your binary worked fine for me.
> > 
> > QEMU commit 2e3408b3cc7de4e87a9adafc8c19bfce3abec947,
> > x86_64 host,
> 
> Oh, well, now that you've mentioned it: I was running "make check-
> tcg" on a 
> s390x host. It works fine on a x86, indeed. So the new problem is
> likely in 
> the s390x TCG host backend... Richard, could you maybe have a look?
> 
>   Thomas

It worked fine on a s390x host for me as well.
Can this be related to the large mmap() that the test performs?
Re: branch-relative-long fails on s390x host (was: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c)
Posted by Thomas Huth 3 years, 9 months ago 
On 04/05/2022 11.37, Ilya Leoshkevich wrote:
> On Wed, 2022-05-04 at 11:14 +0200, Thomas Huth wrote:
>> On 04/05/2022 11.07, Ilya Leoshkevich wrote:
>>> On Wed, 2022-05-04 at 09:01 +0200, Thomas Huth wrote:
>>>> On 04/05/2022 00.46, Ilya Leoshkevich wrote:
>>>>> On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
>>>>>> On 03/05/2022 11.02, Thomas Huth wrote:
>>>>>>> On 02/05/2022 18.48, Ilya Leoshkevich wrote:
>>>>>>>> Binutils >=2.37 and Clang do not accept (. - 0x100000000)
>>>>>>>> PCRel32
>>>>>>>> constants. While this looks like a bug that needs fixing,
>>>>>>>> use
>>>>>>>> a
>>>>>>>> different notation (-0x100000000) as a workaround.
>>>>>>>>
>>>>>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>>>>>> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
>>>>>>>> ---
>>>>>>>>      tests/tcg/s390x/branch-relative-long.c | 4 ++--
>>>>>>>>      1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/tests/tcg/s390x/branch-relative-long.c
>>>>>>>> b/tests/tcg/s390x/branch-relative-long.c
>>>>>>>> index 94219afcad..8ce9f1c2e5 100644
>>>>>>>> --- a/tests/tcg/s390x/branch-relative-long.c
>>>>>>>> +++ b/tests/tcg/s390x/branch-relative-long.c
>>>>>>>> @@ -13,8 +13,8 @@
>>>>>>>>              #_name "_end:\n");
>>>>>>>>      DEFINE_ASM(br_r14, "br %r14");
>>>>>>>> -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
>>>>>>>> -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
>>>>>>>> +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
>>>>>>>> +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
>>>>>>>
>>>>>>> Works for me, thanks!
>>>>>>
>>>>>> Sorry, I spoke too soon - it compiles fine, and also runs
>>>>>> fine
>>>>>> when I
>>>>>> run it
>>>>>> natively, but when I run it through "qemu-s390x", it
>>>>>> crashes...
>>>>>> does
>>>>>> that
>>>>>> work for you?
>>>>>
>>>>> Hi, yes, I just double-checked - it works fine for me.
>>>>> Could you please share the resulting test binary?
>>>>
>>>>
>>>> Sure, here it is:
>>>>
>>>> https://people.redhat.com/~thuth/data/branch-relative-long
>>>>
>>>>     Thomas
>>>
>>> Your binary worked fine for me.
>>>
>>> QEMU commit 2e3408b3cc7de4e87a9adafc8c19bfce3abec947,
>>> x86_64 host,
>>
>> Oh, well, now that you've mentioned it: I was running "make check-
>> tcg" on a
>> s390x host. It works fine on a x86, indeed. So the new problem is
>> likely in
>> the s390x TCG host backend... Richard, could you maybe have a look?
>>
>>    Thomas
> 
> It worked fine on a s390x host for me as well.

Weird ... Did you compile qemu-s390x itself with Clang or with GCC? I just 
discovered that the crash also only happens if I compile qemu-s390x with 
Clang - there is no crash when I compile it with GCC.

> Can this be related to the large mmap() that the test performs?

It works when I compile the test with GCC instead of Clang - so I assume 
that the problem is somewhere else...

  Thomas
Re: branch-relative-long fails on s390x host (was: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c)
Posted by Ilya Leoshkevich 3 years, 9 months ago 
On Wed, 2022-05-04 at 12:46 +0200, Thomas Huth wrote:
> On 04/05/2022 11.37, Ilya Leoshkevich wrote:
> > On Wed, 2022-05-04 at 11:14 +0200, Thomas Huth wrote:
> > > On 04/05/2022 11.07, Ilya Leoshkevich wrote:
> > > > On Wed, 2022-05-04 at 09:01 +0200, Thomas Huth wrote:
> > > > > On 04/05/2022 00.46, Ilya Leoshkevich wrote:
> > > > > > On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
> > > > > > > On 03/05/2022 11.02, Thomas Huth wrote:
> > > > > > > > On 02/05/2022 18.48, Ilya Leoshkevich wrote:
> > > > > > > > > Binutils >=2.37 and Clang do not accept (. -
> > > > > > > > > 0x100000000)
> > > > > > > > > PCRel32
> > > > > > > > > constants. While this looks like a bug that needs
> > > > > > > > > fixing,
> > > > > > > > > use
> > > > > > > > > a
> > > > > > > > > different notation (-0x100000000) as a workaround.
> > > > > > > > > 
> > > > > > > > > Reported-by: Thomas Huth <thuth@redhat.com>
> > > > > > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > > > > > > ---
> > > > > > > > >      tests/tcg/s390x/branch-relative-long.c | 4 ++--
> > > > > > > > >      1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > b/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > index 94219afcad..8ce9f1c2e5 100644
> > > > > > > > > --- a/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > +++ b/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > @@ -13,8 +13,8 @@
> > > > > > > > >              #_name "_end:\n");
> > > > > > > > >      DEFINE_ASM(br_r14, "br %r14");
> > > > > > > > > -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
> > > > > > > > > -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
> > > > > > > > > +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
> > > > > > > > > +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
> > > > > > > > 
> > > > > > > > Works for me, thanks!
> > > > > > > 
> > > > > > > Sorry, I spoke too soon - it compiles fine, and also runs
> > > > > > > fine
> > > > > > > when I
> > > > > > > run it
> > > > > > > natively, but when I run it through "qemu-s390x", it
> > > > > > > crashes...
> > > > > > > does
> > > > > > > that
> > > > > > > work for you?
> > > > > > 
> > > > > > Hi, yes, I just double-checked - it works fine for me.
> > > > > > Could you please share the resulting test binary?
> > > > > 
> > > > > 
> > > > > Sure, here it is:
> > > > > 
> > > > > https://people.redhat.com/~thuth/data/branch-relative-long
> > > > > 
> > > > >     Thomas
> > > > 
> > > > Your binary worked fine for me.
> > > > 
> > > > QEMU commit 2e3408b3cc7de4e87a9adafc8c19bfce3abec947,
> > > > x86_64 host,
> > > 
> > > Oh, well, now that you've mentioned it: I was running "make
> > > check-
> > > tcg" on a
> > > s390x host. It works fine on a x86, indeed. So the new problem is
> > > likely in
> > > the s390x TCG host backend... Richard, could you maybe have a
> > > look?
> > > 
> > >    Thomas
> > 
> > It worked fine on a s390x host for me as well.
> 
> Weird ... Did you compile qemu-s390x itself with Clang or with GCC? I
> just 
> discovered that the crash also only happens if I compile qemu-s390x
> with 
> Clang - there is no crash when I compile it with GCC.
> 
> > Can this be related to the large mmap() that the test performs?
> 
> It works when I compile the test with GCC instead of Clang - so I
> assume 
> that the problem is somewhere else...
> 
>   Thomas
> 

I see, I just used your test with the gcc-built QEMU.
With clang-built QEMU it hangs for me on the s390x host.
Re: branch-relative-long fails on s390x host (was: [PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c)
Posted by Ilya Leoshkevich 3 years, 9 months ago 
On Wed, 2022-05-04 at 12:51 +0200, Ilya Leoshkevich wrote:
> On Wed, 2022-05-04 at 12:46 +0200, Thomas Huth wrote:
> > On 04/05/2022 11.37, Ilya Leoshkevich wrote:
> > > On Wed, 2022-05-04 at 11:14 +0200, Thomas Huth wrote:
> > > > On 04/05/2022 11.07, Ilya Leoshkevich wrote:
> > > > > On Wed, 2022-05-04 at 09:01 +0200, Thomas Huth wrote:
> > > > > > On 04/05/2022 00.46, Ilya Leoshkevich wrote:
> > > > > > > On Tue, 2022-05-03 at 21:26 +0200, Thomas Huth wrote:
> > > > > > > > On 03/05/2022 11.02, Thomas Huth wrote:
> > > > > > > > > On 02/05/2022 18.48, Ilya Leoshkevich wrote:
> > > > > > > > > > Binutils >=2.37 and Clang do not accept (. -
> > > > > > > > > > 0x100000000)
> > > > > > > > > > PCRel32
> > > > > > > > > > constants. While this looks like a bug that needs
> > > > > > > > > > fixing,
> > > > > > > > > > use
> > > > > > > > > > a
> > > > > > > > > > different notation (-0x100000000) as a workaround.
> > > > > > > > > > 
> > > > > > > > > > Reported-by: Thomas Huth <thuth@redhat.com>
> > > > > > > > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > > > > > > > ---
> > > > > > > > > >      tests/tcg/s390x/branch-relative-long.c | 4 ++-
> > > > > > > > > > -
> > > > > > > > > >      1 file changed, 2 insertions(+), 2 deletions(-
> > > > > > > > > > )
> > > > > > > > > > 
> > > > > > > > > > diff --git a/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > > b/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > > index 94219afcad..8ce9f1c2e5 100644
> > > > > > > > > > --- a/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > > +++ b/tests/tcg/s390x/branch-relative-long.c
> > > > > > > > > > @@ -13,8 +13,8 @@
> > > > > > > > > >              #_name "_end:\n");
> > > > > > > > > >      DEFINE_ASM(br_r14, "br %r14");
> > > > > > > > > > -DEFINE_ASM(brasl_r0, "brasl %r0,.-0x100000000");
> > > > > > > > > > -DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x100000000");
> > > > > > > > > > +DEFINE_ASM(brasl_r0, "brasl %r0,-0x100000000");
> > > > > > > > > > +DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x100000000");
> > > > > > > > > 
> > > > > > > > > Works for me, thanks!
> > > > > > > > 
> > > > > > > > Sorry, I spoke too soon - it compiles fine, and also
> > > > > > > > runs
> > > > > > > > fine
> > > > > > > > when I
> > > > > > > > run it
> > > > > > > > natively, but when I run it through "qemu-s390x", it
> > > > > > > > crashes...
> > > > > > > > does
> > > > > > > > that
> > > > > > > > work for you?
> > > > > > > 
> > > > > > > Hi, yes, I just double-checked - it works fine for me.
> > > > > > > Could you please share the resulting test binary?
> > > > > > 
> > > > > > 
> > > > > > Sure, here it is:
> > > > > > 
> > > > > > https://people.redhat.com/~thuth/data/branch-relative-long
> > > > > > 
> > > > > >     Thomas
> > > > > 
> > > > > Your binary worked fine for me.
> > > > > 
> > > > > QEMU commit 2e3408b3cc7de4e87a9adafc8c19bfce3abec947,
> > > > > x86_64 host,
> > > > 
> > > > Oh, well, now that you've mentioned it: I was running "make
> > > > check-
> > > > tcg" on a
> > > > s390x host. It works fine on a x86, indeed. So the new problem
> > > > is
> > > > likely in
> > > > the s390x TCG host backend... Richard, could you maybe have a
> > > > look?
> > > > 
> > > >    Thomas
> > > 
> > > It worked fine on a s390x host for me as well.
> > 
> > Weird ... Did you compile qemu-s390x itself with Clang or with GCC?
> > I
> > just 
> > discovered that the crash also only happens if I compile qemu-s390x
> > with 
> > Clang - there is no crash when I compile it with GCC.
> > 
> > > Can this be related to the large mmap() that the test performs?
> > 
> > It works when I compile the test with GCC instead of Clang - so I
> > assume 
> > that the problem is somewhere else...
> > 
> >   Thomas
> > 
> 
> I see, I just used your test with the gcc-built QEMU.
> With clang-built QEMU it hangs for me on the s390x host.

Actually I've been somewhat impatient, it's not a hang, but rather
quite a long wait followed by a SEGV. So I debugged this a bit, and
apparently what happens is:

- The test zeroes out a code page with exrl+xc.

- do_helper_xc() is called. Clang generates exrl+xc combination there
  as well.

- Since there already exists a TB for the code in question, its page is
  read-only. SIGSEGV happens.

- host_signal_handler() calls host_signal_write() and it doesn't
  recognize exrl as a write. Therefore page_unprotect() is not called
  and the signal is forwarded to the test.

The following does indeed help:

--- a/linux-user/include/host/s390/host-signal.h
+++ b/linux-user/include/host/s390/host-signal.h
@@ -61,6 +61,12 @@ static inline bool host_signal_write(siginfo_t
*info, host_sigcontext *uc)
             return true;
         }
         break;
+    case 0xc6: /* RIL-b format insns */
+        switch (pinsn[0] & 0xf) {
+        case 0x0: /* EXRL */
+            return true;
+        }
+        break;
     case 0xc8: /* SSF format insns */
         switch (pinsn[0] & 0xf) {
         case 0x2: /* CSST */

While there can be false positives here, it shouldn't hurt:
for those, page_unprotect() will recognize that the page in question
does not have a corresponding TB and nothing will happen.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.