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-07T17:24:21Z

From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: Tue, 7 Nov 2023 17:44:00 +0100

> 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:

[...]

> 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.

This one seems to work. At least previously mad GCC 9.3.0 now sits
quietly, as if I added OPTIMIZER_HIDE_VAR() as Yury suggested.

Note that ideally @map should be marked as `POS` in both cases to help
Clang, but `POS` gets undefined at the end of fortify-string.h, so I
decided to not do that within this draft.

Thanks,
Olek
---
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index e8031a157db5..efa0a0287d7c 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -589,12 +589,14 @@ static inline unsigned long bitmap_read(const
unsigned long *map,
size_t index = BIT_WORD(start);
unsigned long offset = start % BITS_PER_LONG;
unsigned long space = BITS_PER_LONG - offset;
+ const size_t map_size = __member_size(map);
unsigned long value_low, value_high;

if (unlikely(!nbits || nbits > BITS_PER_LONG))
return 0;

- if (space >= nbits)
+ if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX &&
+ index + 1 >= map_size / sizeof(long)) || space >= nbits)
return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits);

value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
@@ -620,6 +622,7 @@ static inline unsigned long bitmap_read(const
unsigned long *map,
static inline void bitmap_write(unsigned long *map, unsigned long value,
unsigned long start, unsigned long nbits)
{
+ const size_t map_size = __member_size(map);
size_t index;
unsigned long offset;
unsigned long space;
@@ -638,7 +641,9 @@ static inline void bitmap_write(unsigned long *map,
unsigned long value,

map[index] &= (fit ? (~(mask << offset)) :
~BITMAP_FIRST_WORD_MASK(start));
map[index] |= value << offset;
- if (fit)
+
+ if ((__builtin_constant_p(map_size) && map_size != SIZE_MAX &&
+ index + 1 >= map_size / sizeof(long)) || fit)
return;

map[index + 1] &= BITMAP_FIRST_WORD_MASK(start + nbits);