Re: [alobakin:pfcp 11/19] include/linux/bitmap.h:642:17: warning: array subscript [1, 1024] is outside array bounds of 'long unsigned int[1]'

Alexander Lobakin
2023-11-07T16:44:15Z

From: Alexander Potapenko <glider@google.com>
Date: Tue, 7 Nov 2023 17:33:56 +0100

> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin
> <aleksander.lobakin@intel.com> wrote:
>>
>> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Date: Mon, 6 Nov 2023 20:23:52 +0200
>>
>>> On Mon, Nov 06, 2023 at 05:31:34PM +0100, Alexander Lobakin wrote:
>>>
>>>>> | Reported-by: kernel test robot <lkp@intel.com>
>>>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202310170708.fJzLlgDM-lkp@intel.com/
>>>
>>>> Not sure how to approach this :z It was also captured on the version you
>>>> sent 2 weeks ago, so this could've been resolved already.
>>>
>>> Is it in the repository already? if so, we should revert it.
>>> Otherwise you have time to think and fix.
>>
>> Nah, neither Alex' series nor mine. And I'd say this should rather be
>> resolved in the functions Alex introduce.
>>
>> Thanks,
>> Olek
>
> Sorry, I couldn't reproduce the problem using the instructions at
> https://download.01.org/0day-ci/archive/20231017/202310170708.fJzLlgDM-lkp@intel.com/reproduce
> locally, maybe that's because I only have gcc-11 and higher.
>
> But if I'm understanding correctly what's going on, then GCC will be
> reporting the same issue in the following code:
>
> =======================================================
> #include <stddef.h>
> #include <stdbool.h>
>
> #define BITS_PER_LONG 64
> #define unlikely(x) x
> #define UL(x) (x##UL)
> #define GENMASK(h, l) \
> (((~UL(0)) - (UL(1) << (l)) + 1) & \
> (~UL(0) >> (BITS_PER_LONG - 1 - (h))))
>
> #define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
> #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
> #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
>
> inline void bitmap_write(unsigned long *map,
> unsigned long value,
> unsigned long start, unsigned long nbits)
> {
> size_t index;
> unsigned long offset;
> unsigned long space;
> unsigned long mask;
> bool fit;
>
> if (unlikely(!nbits))
> return;
>
> mask = BITMAP_LAST_WORD_MASK(nbits);
> value &= mask;
> offset = start % BITS_PER_LONG;
> space = BITS_PER_LONG - offset;
> fit = space >= nbits;
> index = BIT_WORD(start);
>
> map[index] &= (fit ? (~(mask << offset)) :
> ~BITMAP_FIRST_WORD_MASK(start));
> map[index] |= value << offset;
> if (fit)
> return;
>
> map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
> map[index + 1] |= (value >> space);
> }
>
> unsigned long foo(unsigned int n) {
> unsigned long bm[1] = {0};
> bitmap_write(bm, 1, n, 2);
> return bm[0];
> }
> =======================================================
> (see also https://godbolt.org/z/GfGfYje53)
>
> If so, the problem is not specific to GCC 9, trunk GCC also barks on this code:
>
> =======================================================
> In function 'bitmap_write',
> inlined from 'bitmap_write' at :15:13,
> inlined from 'foo' at :47:7:
> :40:12: warning: array subscript 1 is outside array bounds of
> 'long unsigned int[1]' [-Warray-bounds=]
> 40 | map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);
> | ~~~^~~~~~~~~~~
> =======================================================
>
> If this is true for the code in drivers/gpio/gpio-pca953x.c,
> suppressing the report for GCC 9 won't help for other versions.
> Given that this report is isolated in a single file, we probably need

I tested it on GCC 9 using modified make.cross from lkp and it triggers
on one more file:

drivers/thermal/intel/intel_soc_dts_iosf.c: In function 'sys_get_curr_temp':
./include/linux/bitmap.h:601:18: error: array subscript [1,
288230376151711744] is outside array bounds of 'long unsigned int[1]'
[-Werror=array-bounds]

> to give the compiler some hints about the range of values passed to
> bitmap_write() rather than suppressing the optimizations.

OPTIMIZER_HIDE_VAR() doesn't disable optimizations if I get it
correctly, rather shuts up the compiler in cases like this one.

I've been thinking of using __member_size() from fortify-string.h, we
could probably optimize the object code even a bit more while silencing
this warning.
Adding Kees, maybe he'd like to participate in sorting this out as well.

Thanks,
Olek