All of lore.kernel.org
 help / color / mirror / Atom feed
* + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
@ 2024-05-24  3:00 Andrew Morton
  2024-05-29 14:39 ` Arnd Bergmann
  2024-06-14 17:16 ` Yury Norov
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Morton @ 2024-05-24  3:00 UTC (permalink / raw)
  To: mm-commits, yoann.congal, vincent.guittot, rdunlap, pmladek,
	nphamcs, masahiroy, gustavoars, davem, arnd, aleksander.lobakin,
	yury.norov, akpm


The patch titled
     Subject: gcc: disable '-Warray-bounds' for gcc-9
has been added to the -mm mm-hotfixes-unstable branch.  Its filename is
     gcc-disable-warray-bounds-for-gcc-9.patch

This patch will shortly appear at
     https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/gcc-disable-warray-bounds-for-gcc-9.patch

This patch will later appear in the mm-hotfixes-unstable branch at
    git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days

------------------------------------------------------
From: Yury Norov <yury.norov@gmail.com>
Subject: gcc: disable '-Warray-bounds' for gcc-9
Date: Wed, 22 May 2024 15:58:30 -0700

'-Warray-bounds' is already disabled for gcc-10+.  Now that we've merged
bitmap_{read,write), I see the following error when building the kernel
with gcc-9.4 (Ubuntu 20.04.4 LTS) for x86_64 allmodconfig:

drivers/pinctrl/pinctrl-cy8c95x0.c: In function `cy8c95x0_read_regs_mask.isra.0':
include/linux/bitmap.h:756:18: error: array subscript [1, 288230376151711744] is outside array bounds of `long unsigned int[1]' [-Werror=array-bounds]
  756 |  value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
      |               ~~~^~~~~~~~~~~

The immediate reason is that the commit b44759705f7d ("bitmap: make
bitmap_{get,set}_value8() use bitmap_{read,write}()") switched the
bitmap_get_value8() to an alias of bitmap_read(); the same for 'set'.

Now; the code that triggers Warray-bounds, calls the function like this:

  #define MAX_BANK 8
  #define BANK_SZ 8
  #define MAX_LINE        (MAX_BANK * BANK_SZ)
  DECLARE_BITMAP(tval, MAX_LINE); // 64-bit map: unsigned long tval[1]

  read_val |= bitmap_get_value8(tval, i * BANK_SZ) & ~bits;

bitmap_read() is implemented such that it may conditionally dereference a
pointer beyond the boundary like this:

	unsigned long offset = start % BITS_PER_LONG;
        unsigned long space = BITS_PER_LONG - offset;

        if (space >= nbits)
                return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits);

        value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
        value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
        return (value_low >> offset) | (value_high << space);

In case of bitmap_get_value8(), it's impossible to violate the boundary
because 'space >= nbits' is never the true for byte-aligned 8-bit access. 
So, this is clearly a false-positive.

The same type of false-positives break my allmodconfig build in many
places.  gcc-8, is clear, however.

Link: https://lkml.kernel.org/r/20240522225830.1201778-1-yury.norov@gmail.com
Signed-off-by: Yury Norov <yury.norov@gmail.com>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nhat Pham <nphamcs@gmail.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Yoann Congal <yoann.congal@smile.fr>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 init/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/init/Kconfig~gcc-disable-warray-bounds-for-gcc-9
+++ a/init/Kconfig
@@ -883,7 +883,7 @@ config GCC10_NO_ARRAY_BOUNDS
 
 config CC_NO_ARRAY_BOUNDS
 	bool
-	default y if CC_IS_GCC && GCC_VERSION >= 100000 && GCC10_NO_ARRAY_BOUNDS
+	default y if CC_IS_GCC && GCC_VERSION >= 90000 && GCC10_NO_ARRAY_BOUNDS
 
 # Currently, disable -Wstringop-overflow for GCC globally.
 config GCC_NO_STRINGOP_OVERFLOW
_

Patches currently in -mm which might be from yury.norov@gmail.com are

gcc-disable-warray-bounds-for-gcc-9.patch


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-05-24  3:00 + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch Andrew Morton
@ 2024-05-29 14:39 ` Arnd Bergmann
  2024-05-31  1:18   ` Yury Norov
  2024-06-14 17:16 ` Yury Norov
  1 sibling, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2024-05-29 14:39 UTC (permalink / raw)
  To: Andrew Morton, mm-commits, yoann.congal, Vincent Guittot,
	Randy Dunlap, Petr Mladek, Nhat Pham, Masahiro Yamada,
	Gustavo A. R. Silva, David S . Miller, Alexander Lobakin,
	Yury Norov

On Fri, May 24, 2024, at 05:00, Andrew Morton wrote:
> ------------------------------------------------------
> From: Yury Norov <yury.norov@gmail.com>
> Subject: gcc: disable '-Warray-bounds' for gcc-9
> Date: Wed, 22 May 2024 15:58:30 -0700
>
> '-Warray-bounds' is already disabled for gcc-10+.  Now that we've merged
> bitmap_{read,write), I see the following error when building the kernel
> with gcc-9.4 (Ubuntu 20.04.4 LTS) for x86_64 allmodconfig:
>
> drivers/pinctrl/pinctrl-cy8c95x0.c: In function 
> `cy8c95x0_read_regs_mask.isra.0':
> include/linux/bitmap.h:756:18: error: array subscript [1, 
> 288230376151711744] is outside array bounds of `long unsigned int[1]' 
> [-Werror=array-bounds]
>   756 |  value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + 
> nbits);
>       |               ~~~^~~~~~~~~~~
>
> The immediate reason is that the commit b44759705f7d ("bitmap: make
> bitmap_{get,set}_value8() use bitmap_{read,write}()") switched the
> bitmap_get_value8() to an alias of bitmap_read(); the same for 'set'.
>
> Now; the code that triggers Warray-bounds, calls the function like this:
>
>   #define MAX_BANK 8
>   #define BANK_SZ 8
>   #define MAX_LINE        (MAX_BANK * BANK_SZ)
>   DECLARE_BITMAP(tval, MAX_LINE); // 64-bit map: unsigned long tval[1]
>
>   read_val |= bitmap_get_value8(tval, i * BANK_SZ) & ~bits;
>
> bitmap_read() is implemented such that it may conditionally dereference a
> pointer beyond the boundary like this:
>
> 	unsigned long offset = start % BITS_PER_LONG;
>         unsigned long space = BITS_PER_LONG - offset;
>
>         if (space >= nbits)
>                 return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits);
>
>         value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
>         value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
>         return (value_low >> offset) | (value_high << space);
>
> In case of bitmap_get_value8(), it's impossible to violate the boundary
> because 'space >= nbits' is never the true for byte-aligned 8-bit access. 
> So, this is clearly a false-positive.
>
> The same type of false-positives break my allmodconfig build in many
> places.  gcc-8, is clear, however.

I'm not too happy about this one, I think this is mixing up
a couple of independent issues, and makes it harder to
ever enable the warning again.

The bitmap.h code looks suspicious to me, and if gcc is
unable to analyze this as a false positive, it probably
also can't optimize it correctly, so it may be better
to either not have this as an inline function at all,
or find an implementation that gcc can optimize better.

In the meantime, I would suggest reverting b44759705f7d
("bitmap: make bitmap_{get,set}_value8() use
bitmap_{read,write}()"), until the implementation is
improved to work without a warning.

The other problem I see is that the warning is
disabled globally even when building with W=123,
and I think we should change it to always warn at
least with W=1 regardless of the compiler version.
It's also likely that the other false-positive
warnings only happen when sanitizers are enabled,
so we could turn it on by default without sanitizers
and move it to W=1 with sanitizers.

       Arnd

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-05-29 14:39 ` Arnd Bergmann
@ 2024-05-31  1:18   ` Yury Norov
  2024-05-31 13:29     ` Arnd Bergmann
  0 siblings, 1 reply; 8+ messages in thread
From: Yury Norov @ 2024-05-31  1:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, mm-commits, yoann.congal, Vincent Guittot,
	Randy Dunlap, Petr Mladek, Nhat Pham, Masahiro Yamada,
	Gustavo A. R. Silva, David S . Miller, Alexander Lobakin

Hi Arnd,

On Wed, May 29, 2024 at 04:39:45PM +0200, Arnd Bergmann wrote:
> On Fri, May 24, 2024, at 05:00, Andrew Morton wrote:
> > ------------------------------------------------------
> > From: Yury Norov <yury.norov@gmail.com>
> > Subject: gcc: disable '-Warray-bounds' for gcc-9
> > Date: Wed, 22 May 2024 15:58:30 -0700
> >
> > '-Warray-bounds' is already disabled for gcc-10+.  Now that we've merged
> > bitmap_{read,write), I see the following error when building the kernel
> > with gcc-9.4 (Ubuntu 20.04.4 LTS) for x86_64 allmodconfig:
> >
> > drivers/pinctrl/pinctrl-cy8c95x0.c: In function 
> > `cy8c95x0_read_regs_mask.isra.0':
> > include/linux/bitmap.h:756:18: error: array subscript [1, 
> > 288230376151711744] is outside array bounds of `long unsigned int[1]' 
> > [-Werror=array-bounds]
> >   756 |  value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + 
> > nbits);
> >       |               ~~~^~~~~~~~~~~
> >
> > The immediate reason is that the commit b44759705f7d ("bitmap: make
> > bitmap_{get,set}_value8() use bitmap_{read,write}()") switched the
> > bitmap_get_value8() to an alias of bitmap_read(); the same for 'set'.
> >
> > Now; the code that triggers Warray-bounds, calls the function like this:
> >
> >   #define MAX_BANK 8
> >   #define BANK_SZ 8
> >   #define MAX_LINE        (MAX_BANK * BANK_SZ)
> >   DECLARE_BITMAP(tval, MAX_LINE); // 64-bit map: unsigned long tval[1]
> >
> >   read_val |= bitmap_get_value8(tval, i * BANK_SZ) & ~bits;
> >
> > bitmap_read() is implemented such that it may conditionally dereference a
> > pointer beyond the boundary like this:
> >
> > 	unsigned long offset = start % BITS_PER_LONG;
> >         unsigned long space = BITS_PER_LONG - offset;
> >
> >         if (space >= nbits)
> >                 return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits);
> >
> >         value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
> >         value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
> >         return (value_low >> offset) | (value_high << space);
> >
> > In case of bitmap_get_value8(), it's impossible to violate the boundary
> > because 'space >= nbits' is never the true for byte-aligned 8-bit access. 
> > So, this is clearly a false-positive.
> >
> > The same type of false-positives break my allmodconfig build in many
> > places.  gcc-8, is clear, however.
> 
> I'm not too happy about this one,

Neither me

> I think this is mixing up
> a couple of independent issues, and makes it harder to
> ever enable the warning again.
> 
> The bitmap.h code looks suspicious to me, and if gcc is
> unable to analyze this as a false positive, it probably
> also can't optimize it correctly,

In the b44759705f7d Alexander says:

  bloat-o-meter shows no difference in vmlinux and -2 bytes for
  gpio-pca953x.ko, which says the optimization didn't suffer due to
  that change. The converted helpers have the value width embedded
  and always compile-time constant and that helps a lot.

Bloat-o-meter itself is not a measure of how effective the code is,
but it's a good hit that code generation before/after is at least
on par. Have you an evidence that the patch makes code generation
worse?

> so it may be better
> to either not have this as an inline function at all,
> or find an implementation that gcc can optimize better.

The functions look bulky but it boild to one or at max two words fetch
plus shifts. Inlining helps to generate better code, particularly in
the bitmap_get/set_value8 case because masks and offsets generation is
done at compile time.

We had quite a few cycles back then... Alexander, can you please
share on code generation, particularly inline vs outline versions?

> In the meantime, I would suggest reverting b44759705f7d
> ("bitmap: make bitmap_{get,set}_value8() use
> bitmap_{read,write}()"), until the implementation is
> improved to work without a warning.

I think there's nothing to improve. This is clearly a false-positive
GCC warning, and it should be fixed on GCC side.

> The other problem I see is that the warning is
> disabled globally even when building with W=123,
> and I think we should change it to always warn at
> least with W=1 regardless of the compiler version.
> It's also likely that the other false-positive
> warnings only happen when sanitizers are enabled,
> so we could turn it on by default without sanitizers
> and move it to W=1 with sanitizers.

Interesting. If sanitizers are involved, we should do like you said.
But this is a matter of a different patch, I think.

Thanks,
Yury

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-05-31  1:18   ` Yury Norov
@ 2024-05-31 13:29     ` Arnd Bergmann
  2024-05-31 19:48       ` Yury Norov
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2024-05-31 13:29 UTC (permalink / raw)
  To: Yury Norov
  Cc: Andrew Morton, mm-commits, yoann.congal, Vincent Guittot,
	Randy Dunlap, Petr Mladek, Nhat Pham, Masahiro Yamada,
	Gustavo A. R. Silva, David S . Miller, Alexander Lobakin

On Fri, May 31, 2024, at 03:18, Yury Norov wrote:
> On Wed, May 29, 2024 at 04:39:45PM +0200, Arnd Bergmann wrote:
>> On Fri, May 24, 2024, at 05:00, Andrew Morton wrote:
>
>> I think this is mixing up
>> a couple of independent issues, and makes it harder to
>> ever enable the warning again.
>> 
>> The bitmap.h code looks suspicious to me, and if gcc is
>> unable to analyze this as a false positive, it probably
>> also can't optimize it correctly,
>
> In the b44759705f7d Alexander says:
>
>   bloat-o-meter shows no difference in vmlinux and -2 bytes for
>   gpio-pca953x.ko, which says the optimization didn't suffer due to
>   that change. The converted helpers have the value width embedded
>   and always compile-time constant and that helps a lot.
>
> Bloat-o-meter itself is not a measure of how effective the code is,
> but it's a good hit that code generation before/after is at least
> on par. Have you an evidence that the patch makes code generation
> worse?

So far, it's only a suspicion based on the gcc warning:
this is in the same category as some other warnings that
depend on gcc analyzing code across function boundaries.
Another example is -Wmaybe-uninitialized.

It's usually good at this, but if the code gets too
complex it fails to track the state correctly, resulting
both in missed optimizations and false-postiive warnings.

This often happens in combination with sanitizers, as they
add more complexity and turn off some optimization steps
that are required here.

>> so it may be better
>> to either not have this as an inline function at all,
>> or find an implementation that gcc can optimize better.
>
> The functions look bulky but it boild to one or at max two words fetch
> plus shifts. Inlining helps to generate better code, particularly in
> the bitmap_get/set_value8 case because masks and offsets generation is
> done at compile time.
>
> We had quite a few cycles back then... Alexander, can you please
> share on code generation, particularly inline vs outline versions?

Right, maybe all we need here is marking it __always_inline
then. Something I've seen before is functions that
are meant to become trivial after inlining, but that
(before inlining) appear to have too many statements for
gcc to consider them candidates. It may then try to generate
a specialized version of the function that optimizes for
certain constant arguments, but is then confused about which
arguments are constant or not.

Do you still see gcc-9 false-positive warnings with the
trivial patch below?

     Arnd

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 8c4768c44a01..08df806df3d2 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -737,7 +737,7 @@ static inline void bitmap_from_u64(unsigned long *dst, u64 mask)
  * @map memory region. For @nbits = 0 and @nbits > BITS_PER_LONG the return
  * value is undefined.
  */
-static inline unsigned long bitmap_read(const unsigned long *map,
+static __always_inline unsigned long bitmap_read(const unsigned long *map,
                                        unsigned long start,
                                        unsigned long nbits)
 {
@@ -772,7 +772,7 @@ static inline unsigned long bitmap_read(const unsigned long *map,
  *
  * For @nbits == 0 and @nbits > BITS_PER_LONG no writes are performed.
  */
-static inline void bitmap_write(unsigned long *map, unsigned long value,
+static __always_inline void bitmap_write(unsigned long *map, unsigned long value,
                                unsigned long start, unsigned long nbits)
 {
        size_t index;

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-05-31 13:29     ` Arnd Bergmann
@ 2024-05-31 19:48       ` Yury Norov
  0 siblings, 0 replies; 8+ messages in thread
From: Yury Norov @ 2024-05-31 19:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, mm-commits, yoann.congal, Vincent Guittot,
	Randy Dunlap, Petr Mladek, Nhat Pham, Masahiro Yamada,
	Gustavo A. R. Silva, David S . Miller, Alexander Lobakin

On Fri, May 31, 2024 at 03:29:28PM +0200, Arnd Bergmann wrote:
>  "Alexander Lobakin" <aleksander.lobakin@intel.com>
> Subject: Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable
>  branch
> Content-Type: text/plain
> Status: RO
> Content-Length: 3582
> Lines: 85
> 
> On Fri, May 31, 2024, at 03:18, Yury Norov wrote:
> > On Wed, May 29, 2024 at 04:39:45PM +0200, Arnd Bergmann wrote:
> >> On Fri, May 24, 2024, at 05:00, Andrew Morton wrote:
> >
> >> I think this is mixing up
> >> a couple of independent issues, and makes it harder to
> >> ever enable the warning again.
> >> 
> >> The bitmap.h code looks suspicious to me, and if gcc is
> >> unable to analyze this as a false positive, it probably
> >> also can't optimize it correctly,
> >
> > In the b44759705f7d Alexander says:
> >
> >   bloat-o-meter shows no difference in vmlinux and -2 bytes for
> >   gpio-pca953x.ko, which says the optimization didn't suffer due to
> >   that change. The converted helpers have the value width embedded
> >   and always compile-time constant and that helps a lot.
> >
> > Bloat-o-meter itself is not a measure of how effective the code is,
> > but it's a good hit that code generation before/after is at least
> > on par. Have you an evidence that the patch makes code generation
> > worse?
> 
> So far, it's only a suspicion based on the gcc warning:
> this is in the same category as some other warnings that
> depend on gcc analyzing code across function boundaries.
> Another example is -Wmaybe-uninitialized.
> 
> It's usually good at this, but if the code gets too
> complex it fails to track the state correctly, resulting
> both in missed optimizations and false-postiive warnings.
> 
> This often happens in combination with sanitizers, as they
> add more complexity and turn off some optimization steps
> that are required here.
>
> >> so it may be better
> >> to either not have this as an inline function at all,
> >> or find an implementation that gcc can optimize better.
> >
> > The functions look bulky but it boild to one or at max two words fetch
> > plus shifts. Inlining helps to generate better code, particularly in
> > the bitmap_get/set_value8 case because masks and offsets generation is
> > done at compile time.
> >
> > We had quite a few cycles back then... Alexander, can you please
> > share on code generation, particularly inline vs outline versions?
> 
> Right, maybe all we need here is marking it __always_inline
> then. Something I've seen before is functions that
> are meant to become trivial after inlining, but that
> (before inlining) appear to have too many statements for
> gcc to consider them candidates. It may then try to generate
> a specialized version of the function that optimizes for
> certain constant arguments, but is then confused about which
> arguments are constant or not.
> 
> Do you still see gcc-9 false-positive warnings with the
> trivial patch below?

KASAN was not enabled. I disabled all CONFIG_*SAN and CONFIG_KFENCE,
but the warning is still there. __always_inline doesn't improve as well.

Thanks,
Yury
 
> 
>      Arnd
> 
> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
> index 8c4768c44a01..08df806df3d2 100644
> --- a/include/linux/bitmap.h
> +++ b/include/linux/bitmap.h
> @@ -737,7 +737,7 @@ static inline void bitmap_from_u64(unsigned long *dst, u64 mask)
>   * @map memory region. For @nbits = 0 and @nbits > BITS_PER_LONG the return
>   * value is undefined.
>   */
> -static inline unsigned long bitmap_read(const unsigned long *map,
> +static __always_inline unsigned long bitmap_read(const unsigned long *map,
>                                         unsigned long start,
>                                         unsigned long nbits)
>  {
> @@ -772,7 +772,7 @@ static inline unsigned long bitmap_read(const unsigned long *map,
>   *
>   * For @nbits == 0 and @nbits > BITS_PER_LONG no writes are performed.
>   */
> -static inline void bitmap_write(unsigned

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-05-24  3:00 + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch Andrew Morton
  2024-05-29 14:39 ` Arnd Bergmann
@ 2024-06-14 17:16 ` Yury Norov
  2024-06-14 17:40   ` Andrew Morton
  1 sibling, 1 reply; 8+ messages in thread
From: Yury Norov @ 2024-06-14 17:16 UTC (permalink / raw)
  To: Andrew Morton
  Cc: mm-commits, yoann.congal, vincent.guittot, rdunlap, pmladek,
	nphamcs, masahiroy, gustavoars, davem, arnd, aleksander.lobakin

On Thu, May 23, 2024 at 08:00:07PM -0700, Andrew Morton wrote:
> 
> The patch titled
>      Subject: gcc: disable '-Warray-bounds' for gcc-9
> has been added to the -mm mm-hotfixes-unstable branch.  Its filename is
>      gcc-disable-warray-bounds-for-gcc-9.patch

Hi Andrew,

The script said you've applied it to mm-hotfixes, but the patch is
still not in mainline. Are you going to move it in this cycle, or
what are your plans on it?

Thanks,
Yury
 
> This patch will shortly appear at
>      https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/gcc-disable-warray-bounds-for-gcc-9.patch
> 
> This patch will later appear in the mm-hotfixes-unstable branch at
>     git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> 
> Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
> 
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
> 
> The -mm tree is included into linux-next via the mm-everything
> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there every 2-3 working days
> 
> ------------------------------------------------------
> From: Yury Norov <yury.norov@gmail.com>
> Subject: gcc: disable '-Warray-bounds' for gcc-9
> Date: Wed, 22 May 2024 15:58:30 -0700
> 
> '-Warray-bounds' is already disabled for gcc-10+.  Now that we've merged
> bitmap_{read,write), I see the following error when building the kernel
> with gcc-9.4 (Ubuntu 20.04.4 LTS) for x86_64 allmodconfig:
> 
> drivers/pinctrl/pinctrl-cy8c95x0.c: In function `cy8c95x0_read_regs_mask.isra.0':
> include/linux/bitmap.h:756:18: error: array subscript [1, 288230376151711744] is outside array bounds of `long unsigned int[1]' [-Werror=array-bounds]
>   756 |  value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
>       |               ~~~^~~~~~~~~~~
> 
> The immediate reason is that the commit b44759705f7d ("bitmap: make
> bitmap_{get,set}_value8() use bitmap_{read,write}()") switched the
> bitmap_get_value8() to an alias of bitmap_read(); the same for 'set'.
> 
> Now; the code that triggers Warray-bounds, calls the function like this:
> 
>   #define MAX_BANK 8
>   #define BANK_SZ 8
>   #define MAX_LINE        (MAX_BANK * BANK_SZ)
>   DECLARE_BITMAP(tval, MAX_LINE); // 64-bit map: unsigned long tval[1]
> 
>   read_val |= bitmap_get_value8(tval, i * BANK_SZ) & ~bits;
> 
> bitmap_read() is implemented such that it may conditionally dereference a
> pointer beyond the boundary like this:
> 
> 	unsigned long offset = start % BITS_PER_LONG;
>         unsigned long space = BITS_PER_LONG - offset;
> 
>         if (space >= nbits)
>                 return (map[index] >> offset) & BITMAP_LAST_WORD_MASK(nbits);
> 
>         value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
>         value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
>         return (value_low >> offset) | (value_high << space);
> 
> In case of bitmap_get_value8(), it's impossible to violate the boundary
> because 'space >= nbits' is never the true for byte-aligned 8-bit access. 
> So, this is clearly a false-positive.
> 
> The same type of false-positives break my allmodconfig build in many
> places.  gcc-8, is clear, however.
> 
> Link: https://lkml.kernel.org/r/20240522225830.1201778-1-yury.norov@gmail.com
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> Cc: Alexander Lobakin <aleksander.lobakin@intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Nhat Pham <nphamcs@gmail.com>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Yoann Congal <yoann.congal@smile.fr>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  init/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/init/Kconfig~gcc-disable-warray-bounds-for-gcc-9
> +++ a/init/Kconfig
> @@ -883,7 +883,7 @@ config GCC10_NO_ARRAY_BOUNDS
>  
>  config CC_NO_ARRAY_BOUNDS
>  	bool
> -	default y if CC_IS_GCC && GCC_VERSION >= 100000 && GCC10_NO_ARRAY_BOUNDS
> +	default y if CC_IS_GCC && GCC_VERSION >= 90000 && GCC10_NO_ARRAY_BOUNDS
>  
>  # Currently, disable -Wstringop-overflow for GCC globally.
>  config GCC_NO_STRINGOP_OVERFLOW
> _
> 
> Patches currently in -mm which might be from yury.norov@gmail.com are
> 
> gcc-disable-warray-bounds-for-gcc-9.patch

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-06-14 17:16 ` Yury Norov
@ 2024-06-14 17:40   ` Andrew Morton
  2024-06-14 17:55     ` Yury Norov
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2024-06-14 17:40 UTC (permalink / raw)
  To: Yury Norov
  Cc: mm-commits, yoann.congal, vincent.guittot, rdunlap, pmladek,
	nphamcs, masahiroy, gustavoars, davem, arnd, aleksander.lobakin

On Fri, 14 Jun 2024 10:16:02 -0700 Yury Norov <yury.norov@gmail.com> wrote:

> On Thu, May 23, 2024 at 08:00:07PM -0700, Andrew Morton wrote:
> > 
> > The patch titled
> >      Subject: gcc: disable '-Warray-bounds' for gcc-9
> > has been added to the -mm mm-hotfixes-unstable branch.  Its filename is
> >      gcc-disable-warray-bounds-for-gcc-9.patch
> 
> Hi Andrew,
> 
> The script said you've applied it to mm-hotfixes, but the patch is
> still not in mainline. Are you going to move it in this cycle, or
> what are your plans on it?

I was thinking it would go via the net tree but it seems that
b44759705f7d is in mainline so I can send this into Linus soon.

Did we agree that

	Fixes: b44759705f7d ("bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}()")

is the appropriate target?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch
  2024-06-14 17:40   ` Andrew Morton
@ 2024-06-14 17:55     ` Yury Norov
  0 siblings, 0 replies; 8+ messages in thread
From: Yury Norov @ 2024-06-14 17:55 UTC (permalink / raw)
  To: Andrew Morton
  Cc: mm-commits, yoann.congal, vincent.guittot, rdunlap, pmladek,
	nphamcs, masahiroy, gustavoars, davem, arnd, aleksander.lobakin

On Fri, Jun 14, 2024 at 10:40:52AM -0700, Andrew Morton wrote:
> On Fri, 14 Jun 2024 10:16:02 -0700 Yury Norov <yury.norov@gmail.com> wrote:
> 
> > On Thu, May 23, 2024 at 08:00:07PM -0700, Andrew Morton wrote:
> > > 
> > > The patch titled
> > >      Subject: gcc: disable '-Warray-bounds' for gcc-9
> > > has been added to the -mm mm-hotfixes-unstable branch.  Its filename is
> > >      gcc-disable-warray-bounds-for-gcc-9.patch
> > 
> > Hi Andrew,
> > 
> > The script said you've applied it to mm-hotfixes, but the patch is
> > still not in mainline. Are you going to move it in this cycle, or
> > what are your plans on it?
> 
> I was thinking it would go via the net tree but it seems that
> b44759705f7d is in mainline so I can send this into Linus soon.
> 
> Did we agree that
> 
> 	Fixes: b44759705f7d ("bitmap: make bitmap_{get,set}_value8() use bitmap_{read,write}()")
> 
> is the appropriate target?

Not that b44759705f7d introduced a bug, it reveals GCC bug.
But I think it's worth to mention the b44759705f7d somehow.

Please put it at your discretion.

Thanks,
Yury

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-06-14 17:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-24  3:00 + gcc-disable-warray-bounds-for-gcc-9.patch added to mm-hotfixes-unstable branch Andrew Morton
2024-05-29 14:39 ` Arnd Bergmann
2024-05-31  1:18   ` Yury Norov
2024-05-31 13:29     ` Arnd Bergmann
2024-05-31 19:48       ` Yury Norov
2024-06-14 17:16 ` Yury Norov
2024-06-14 17:40   ` Andrew Morton
2024-06-14 17:55     ` Yury Norov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.