From: Alexander Potapenko <[email protected]> Date: Tue, 7 Nov 2023 17:33:56 +0100
> On Tue, Nov 7, 2023 at 2:23 PM Alexander Lobakin > <[email protected]> wrote: >> >> From: Andy Shevchenko <[email protected]> >> 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 <[email protected]> >>>>> | Closes: >>>>> https://lore.kernel.org/oe-kbuild-all/[email protected]/ >>> >>>> 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/[email protected]/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 <source>:15:13, > inlined from 'foo' at <source>:47:7: > <source>: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
