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

Janis Schoetterl-Glausch
2022-05-18T11:07:18Z

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.

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)