Re: [kvm-unit-tests PATCH] s390x: Ignore gcc 12 warnings for low addresses

[Janis Schoetterl-Glausch <scgl@linux.ibm.com>]

On 5/19/22 11:46, Thomas Huth wrote:
> On 18/05/2022 13.07, Janis Schoetterl-Glausch wrote:
>> On 5/17/22 18:09, Thomas Huth wrote:
>>> On 17/05/2022 14.02, Claudio Imbrenda wrote:
>>>> On Mon, 16 May 2022 16:43:32 +0200
>>>> Janis Schoetterl-Glausch <scgl@linux.ibm.com> wrote:
>>>>
>>>>> gcc 12 warns if a memory operand to inline asm points to memory in the
>>>>> first 4k bytes. However, in our case, these operands are fine, either
>>>>> because we actually want to use that memory, or expect and handle the
>>>>> resulting exception.
>>>>> Therefore, silence the warning.
>>>>
>>>> I really dislike this
>>>
>>> I agree the pragmas are ugly. But maybe we should mimic what the kernel
>>> is doing here?
>>>
>>> $ git show 8b202ee218395
>>> commit 8b202ee218395319aec1ef44f72043e1fbaccdd6
>>> Author: Sven Schnelle <svens@linux.ibm.com>
>>> Date:   Mon Apr 25 14:17:42 2022 +0200
>>>
>>>      s390: disable -Warray-bounds
>>>          gcc-12 shows a lot of array bound warnings on s390. This is caused
>>>      by the S390_lowcore macro which uses a hardcoded address of 0.
>>>          Wrapping that with absolute_pointer() works, but gcc no longer knows
>>>      that a 12 bit displacement is sufficient to access lowcore. So it
>>>      emits instructions like 'lghi %r1,0; l %rx,xxx(%r1)' instead of a
>>>      single load/store instruction. As s390 stores variables often
>>>      read/written in lowcore, this is considered problematic. Therefore
>>>      disable -Warray-bounds on s390 for gcc-12 for the time being, until
>>>      there is a better solution.
>>>
>>> ... so we should maybe disable it in the Makefile, too, until the
>>> kernel folks found a nicer solution?
>>>
>>>   Thomas
>>>
>>
>> Neat, wasn't aware of that commit.
>>
>> I don't think we need to concern ourselves with performance in this case and can define
>>
>> +#define HIDE_PTR(ptr)                          \
>> +({                                             \
>> +       uint64_t __ptr;                         \
>> +       asm ("" : "=d" (__ptr) : "0" (ptr));    \
>> +       (typeof(ptr))__ptr;                     \
>> +})
>> +
>>
>> in some header (which?).
>>
>> Another alternative would be to define some extern symbols for the addresses we want to use.
>> It might be nice to have a symbol for the lowcore anyway, then we can get rid of
>>
>> static struct lowcore *lc;
>> struct lowcore *lc = (struct lowcore *)0x0;
>> ...
>>
>> in a bunch of tests.
> 
> I like that idea.
> 
>> And use that symbol to derive the addresses we want to use.
>> emulator.c uses -1 to generate an addressing exception, we either need another symbol for
>> that or use another invalid address. (Can't get to -1 from lowcore/0 because the max array
>> size is signed int64 max)
> 
> Maybe use INT64_MAX or something similar? Would that work?

I did it slightly different than in my prototype --- used a pointer instead of an array,
doesn't matter then.
> 
>  Thomas
>