-Warray-bounds fun again

Hi,

while compiling the latest upstream kernel on fedora 36 which uses gcc-12 by default, i got a lot of -Warray-bounds warnings:

(Note that this is on s390 arch)

In function ‘preempt_count’,
inlined from ‘do_one_initcall’ at init/main.c:1290:14:
./include/asm-generic/rwonce.h:44:26: warning: array subscript 0 is outside array bounds of ‘const volatile int[0]’ [-Warray-bounds] 44 | #define __READ_ONCE(x) (*(const volatile __unqual_scalar_typeof(x) *)&(x))
| ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/include/asm-generic/rwonce.h:50:9: note: in expansion of macro ‘__READ_ONCE’

50 |         __READ_ONCE(x);
   |         ^~~~~~~~~~~

./arch/s390/include/asm/preempt.h:17:16: note: in expansion of macro ‘READ_ONCE’
17 | return READ_ONCE(S390_lowcore.preempt_count) & ~PREEMPT_NEED_RESCHED;
| ^~~~~~~~~

This is because S390_lowcore is defined as follows:

#define S390_lowcore (*((struct lowcore *) 0))

Lowcore is a 8K cpu-local memory region on s390 at fixed address 0.

The obvious 'fix' is to use absolute_pointer():

#define S390_lowcore (*((struct lowcore *)absolute_pointer(0)))

That makes the warning go away, but unfortunately the compiler no longer knows that the memory access is fitting into a load/store with a 12 bit displacement.

Without absolute_pointer(), reading the preempt count is just a single
instruction: 'l %r11,936'

static inline int preempt_count(void)

{
        return READ_ONCE(S390_lowcore.preempt_count) & ~PREEMPT_NEED_RESCHED;
 8c4:   58 b0 03 a8             l       %r11,936 <--- load preempt count
 8c8:   b9 04 00 92             lgr     %r9,%r2
        int count = preempt_count();

with absolute pointer(), the compiler no longer optimizes the read to one instruction and uses an additional base register:

static inline int preempt_count(void)

{
        return READ_ONCE(S390_lowcore.preempt_count) & ~PREEMPT_NEED_RESCHED;
 8c4:   a7 19 00 00             lghi    %r1,0             <-- use %r1 as base, load with 0
 8c8:   b9 04 00 92             lgr     %r9,%r2
        int count = preempt_count();
        char msgbuf[64];
 8cc:   d7 3f f0 a8 f0 a8       xc      168(64,%r15),168(%r15)
 8d2:   58 b0 13 a8             l       %r11,936(%r1)    <-- and finally add the offset and fetch
        int ret;

The reason for gcc to not optimize that further is likely the asm statement in RELOC_HIDE (located in include/linux/compiler-gcc.h)

#define RELOC_HIDE(ptr, off)                                    \
({                                                              \
        unsigned long __ptr;                                    \
        __asm__ ("" : "=r"(__ptr) : "0"(ptr));                  \
        (typeof(ptr)) (__ptr + (off));                          \

})

For most of the code this wouldn't be a big problem, but we're storing information like preempt_count, current thread info, etc in lowcore because it is the fastest way. I would like to avoid to use additional instructions/registers just to avoid a warning.

Does anyone have an idea about a different way to make this warning go away?

Thanks
Sven

In the gcc bugzilla for us needing to do these games:

    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578

one of the suggestions was "I recommend suppressing the warning either by #pragma GCC diagnostic or by making the pointer volatile".

But READ_ONCE() should already be doing that volatile thing, so that suggestion may not work with gcc-12 any more.

It is *possible* that gcc-12 has now special-cased the very special issue of a cast of the constant zero. That is how NULL was traditionally defined.

So just out of a perverse curiosity, what happens if you do something like this:

   #define S390_lowcore_end ((struct lowcore *)sizeof(struct lowcore))
   #define S390_lowcore (S390_lowcore_end[-1])

instead? It should get the same value in the end, but it doesn't have that special case of "cast an integer constant 0 to a pointer".

I suspect it probably doesn't help, because gcc will still see "oh, you're basing this off address zero".

Another thing to try might be to remove the initial 16 bytes of padding from 'struct lowcore' (it looks like the first 20 bytes are not used - so leave 4 bytes of padding still), and use

   #define S390_lowcore (*((struct lowcore_nopad *)16))

instead. Then gcc will never see that 0, and hopefully the "he's accessing based off a NULL pointer" logic will go away.

Because right now, our absolute_pointer() protection against this horrible gcc mis-feature is literally based on hiding the value from the compiler with an inline asm, and by virtue of hiding the value then yes, gcc will have to go through a register base pointer and cannot see that it fits in 12 bits.

.. or you just need to disable -Warray-bounds on s390.

Linus

From: Linus Torvalds

I think you might be mixing up two problems.

Accessing ((foo *)0)->member is problematic because NULL might not be zero. In which case an unexpected address is generated.
I think this is why clang really doesn't like you doing that. Using ((foo *)(sizeof (foo))[-1].member might get around that.

I suspect the array bounds issue is caused by gcc using a size of 0 for 'I don't know the size' and then assuming it is real size later on. That seems like a real gcc bug.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)

Yes.

Looks like gcc also catches that, i get the same warning.

That's what i'll do for now. I'll complain in the bugtracker entry mentioned above, but for now it is propably easier to just disable the warning until we have a fix.

Thanks
Sven