diff mbox series

wireless: ath: work around false-positive stringop-overread warning

Message ID 20230417205447.1800912-1-arnd@kernel.org
State New
Headers
Series wireless: ath: work around false-positive stringop-overread warning |

Commit Message

Arnd Bergmann April 17, 2023, 8:54 p.m. UTC 
  From: Arnd Bergmann <arnd@arndb.de>

In a rare arm64 randconfig build, I got multiple warnings for ath11k
and ath12k:

In function 'ath11k_peer_assoc_h_ht',
    inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:2665:2:
drivers/net/wireless/ath/ath11k/mac.c:1709:13: error: 'ath11k_peer_assoc_h_ht_masked' reading 10 bytes from a region of size 0 [-Werror=stringop-overread]
 1709 |         if (ath11k_peer_assoc_h_ht_masked(ht_mcs_mask))
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This happens whenever gcc-13 fails to inline one of the functions
that take a fixed-length array argument but gets passed a pointer.

Change these functions to all take a regular pointer argument
instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/ath/ath11k/mac.c | 12 ++++++------
 drivers/net/wireless/ath/ath12k/mac.c |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

Simon Horman April 20, 2023, 2:04 p.m. UTC | #1 
On Mon, Apr 17, 2023 at 10:54:20PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> In a rare arm64 randconfig build, I got multiple warnings for ath11k
> and ath12k:
> 
> In function 'ath11k_peer_assoc_h_ht',
>     inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:2665:2:
> drivers/net/wireless/ath/ath11k/mac.c:1709:13: error: 'ath11k_peer_assoc_h_ht_masked' reading 10 bytes from a region of size 0 [-Werror=stringop-overread]
>  1709 |         if (ath11k_peer_assoc_h_ht_masked(ht_mcs_mask))
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This happens whenever gcc-13 fails to inline one of the functions
> that take a fixed-length array argument but gets passed a pointer.
> 
> Change these functions to all take a regular pointer argument
> instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Note: I was not able to reproduce the problem described above.
Kalle Valo May 8, 2023, 8:44 a.m. UTC | #2 
Arnd Bergmann <arnd@kernel.org> writes:

> From: Arnd Bergmann <arnd@arndb.de>
>
> In a rare arm64 randconfig build, I got multiple warnings for ath11k
> and ath12k:
>
> In function 'ath11k_peer_assoc_h_ht',
>     inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:2665:2:
> drivers/net/wireless/ath/ath11k/mac.c:1709:13: error: 'ath11k_peer_assoc_h_ht_masked' reading 10 bytes from a region of size 0 [-Werror=stringop-overread]
>  1709 |         if (ath11k_peer_assoc_h_ht_masked(ht_mcs_mask))
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This happens whenever gcc-13 fails to inline one of the functions
> that take a fixed-length array argument but gets passed a pointer.
>
> Change these functions to all take a regular pointer argument
> instead.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

s/wireless:/wifi:/ but I can fix that.

In a awat it's a shame to lose the explicit length but I guess there's
no other way to fix this?

Also I hope you find the time to add GCC 13 to crosstool :) Related to
this, last year we had a similar warning with GCC 11 for which I added this
not-so-pretty workaround:

abf93f369419 wifi: ath11k: mac: fix reading 16 bytes from a region of size 0 warning

https://git.kernel.org/linus/abf93f369419
Arnd Bergmann May 8, 2023, 8:52 a.m. UTC | #3 
On Mon, May 8, 2023, at 10:44, Kalle Valo wrote:
> Arnd Bergmann <arnd@kernel.org> writes:
>
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> In a rare arm64 randconfig build, I got multiple warnings for ath11k
>> and ath12k:
>>
>> In function 'ath11k_peer_assoc_h_ht',
>>     inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:2665:2:
>> drivers/net/wireless/ath/ath11k/mac.c:1709:13: error: 'ath11k_peer_assoc_h_ht_masked' reading 10 bytes from a region of size 0 [-Werror=stringop-overread]
>>  1709 |         if (ath11k_peer_assoc_h_ht_masked(ht_mcs_mask))
>>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This happens whenever gcc-13 fails to inline one of the functions
>> that take a fixed-length array argument but gets passed a pointer.
>>
>> Change these functions to all take a regular pointer argument
>> instead.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> s/wireless:/wifi:/ but I can fix that.

Ok, thanks!

> In a awat it's a shame to lose the explicit length but I guess there's
> no other way to fix this?

There might be, but I couldn't figure out a way that works.

> Also I hope you find the time to add GCC 13 to crosstool :) Related to
> this

I uploaded gcc-13.1.0 binaries last week, but still need to
update the html page, so it's not yet linked. You can navigate
the directories from the gcc-12 builds.

     Arnd
Kalle Valo May 8, 2023, 2:57 p.m. UTC | #4 
"Arnd Bergmann" <arnd@arndb.de> writes:

> On Mon, May 8, 2023, at 10:44, Kalle Valo wrote:
>
>> Arnd Bergmann <arnd@kernel.org> writes:
>>
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> In a rare arm64 randconfig build, I got multiple warnings for ath11k
>>> and ath12k:
>>>
>>> In function 'ath11k_peer_assoc_h_ht',
>>>     inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:2665:2:
>>> drivers/net/wireless/ath/ath11k/mac.c:1709:13: error: 'ath11k_peer_assoc_h_ht_masked' reading 10 bytes from a region of size 0 [-Werror=stringop-overread]
>>>  1709 |         if (ath11k_peer_assoc_h_ht_masked(ht_mcs_mask))
>>>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> This happens whenever gcc-13 fails to inline one of the functions
>>> that take a fixed-length array argument but gets passed a pointer.
>>>
>>> Change these functions to all take a regular pointer argument
>>> instead.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> s/wireless:/wifi:/ but I can fix that.
>
> Ok, thanks!
>
>> In a awat it's a shame to lose the explicit length but I guess there's
>> no other way to fix this?
>
> There might be, but I couldn't figure out a way that works.

Ok.

>> Also I hope you find the time to add GCC 13 to crosstool :) Related to
>> this
>
> I uploaded gcc-13.1.0 binaries last week, but still need to
> update the html page, so it's not yet linked. You can navigate
> the directories from the gcc-12 builds.

Thanks! I was able to find the build[1] but having an issue:

$ ./x86_64-linux-gcc -v
./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.35' not found (required by ./x86_64-linux-gcc)
./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.32' not found (required by ./x86_64-linux-gcc)
./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.33' not found (required by ./x86_64-linux-gcc)
./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.36' not found (required by ./x86_64-linux-gcc)
./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version `GLIBC_2.34' not found (required by ./x86_64-linux-gcc)

With older GCC versions from your page I don't have this problem. I'm
using Debian 10 still so so is my libc too old?

ii  libc6:amd64          2.28-10+deb10u2:amd6 GNU C Library: Shared libraries

[1] https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.1.0/x86_64-gcc-13.1.0-nolibc-x86_64-linux.tar.gz
Arnd Bergmann May 8, 2023, 3:07 p.m. UTC | #5 
On Mon, May 8, 2023, at 16:57, Kalle Valo wrote:
> "Arnd Bergmann" <arnd@arndb.de> writes:
>>
>> I uploaded gcc-13.1.0 binaries last week, but still need to
>> update the html page, so it's not yet linked. You can navigate
>> the directories from the gcc-12 builds.
>
> Thanks! I was able to find the build[1] but having an issue:
>
> $ ./x86_64-linux-gcc -v
> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
> `GLIBC_2.35' not found (required by ./x86_64-linux-gcc)
> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
> `GLIBC_2.32' not found (required by ./x86_64-linux-gcc)
> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
> `GLIBC_2.33' not found (required by ./x86_64-linux-gcc)
> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
> `GLIBC_2.36' not found (required by ./x86_64-linux-gcc)
> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
> `GLIBC_2.34' not found (required by ./x86_64-linux-gcc)
>
> With older GCC versions from your page I don't have this problem. I'm
> using Debian 10 still so so is my libc too old?

(dropping most Cc)

Indeed, thanks for the report, I forgot about that issue. I used
to build the cross toolchains in an old Ubuntu 16.04 chroot to avoid
that issue, and I linked all other dependencies statically.

The gcc-13.1.0 builds are the first ones I did on an arm64 machine,
so I had to create a new build environment and started out with
just my normal Debian testing rootfs, which caused me enough issues
to figure out first.

I had previously experimented with linking statically against
musl to avoid all other dependencies, but that ended up with
slower binaries because the default memory allocator in musl
doesn't work that well for gcc, and I never quite figured out
how to pick a different memory allocator, or which one to use.

I should probably just pick an older Debian release that is new
enough to contain cross compilers for arm64 and x86 and then
set up the same kind of chroot I had in before.

      Arnd
Kalle Valo May 8, 2023, 7:19 p.m. UTC | #6 
"Arnd Bergmann" <arnd@arndb.de> writes:

> On Mon, May 8, 2023, at 16:57, Kalle Valo wrote:
>> "Arnd Bergmann" <arnd@arndb.de> writes:
>>>
>>> I uploaded gcc-13.1.0 binaries last week, but still need to
>>> update the html page, so it's not yet linked. You can navigate
>>> the directories from the gcc-12 builds.
>>
>> Thanks! I was able to find the build[1] but having an issue:
>>
>> $ ./x86_64-linux-gcc -v
>> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
>> `GLIBC_2.35' not found (required by ./x86_64-linux-gcc)
>> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
>> `GLIBC_2.32' not found (required by ./x86_64-linux-gcc)
>> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
>> `GLIBC_2.33' not found (required by ./x86_64-linux-gcc)
>> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
>> `GLIBC_2.36' not found (required by ./x86_64-linux-gcc)
>> ./x86_64-linux-gcc: /lib/x86_64-linux-gnu/libc.so.6: version 
>> `GLIBC_2.34' not found (required by ./x86_64-linux-gcc)
>>
>> With older GCC versions from your page I don't have this problem. I'm
>> using Debian 10 still so so is my libc too old?
>
> (dropping most Cc)
>
> Indeed, thanks for the report, I forgot about that issue. I used
> to build the cross toolchains in an old Ubuntu 16.04 chroot to avoid
> that issue, and I linked all other dependencies statically.
>
> The gcc-13.1.0 builds are the first ones I did on an arm64 machine,
> so I had to create a new build environment and started out with
> just my normal Debian testing rootfs, which caused me enough issues
> to figure out first.
>
> I had previously experimented with linking statically against
> musl to avoid all other dependencies, but that ended up with
> slower binaries because the default memory allocator in musl
> doesn't work that well for gcc, and I never quite figured out
> how to pick a different memory allocator, or which one to use.
>
> I should probably just pick an older Debian release that is new
> enough to contain cross compilers for arm64 and x86 and then
> set up the same kind of chroot I had in before.

Thanks! I really should update to Debian 11 but I have been lazy :) But
I doubt that would have helped either as it looks like it has libc6
v2.31:

https://packages.debian.org/bullseye/libc6

I'm disappointed glibc creates so uncompatible binaries, feels like they
don't create about backwards compatibility :/
Kalle Valo May 9, 2023, 4:48 p.m. UTC | #7 
Arnd Bergmann <arnd@kernel.org> wrote:

> In a rare arm64 randconfig build, I got multiple warnings for ath11k
> and ath12k:
> 
> In function 'ath11k_peer_assoc_h_ht',
>     inlined from 'ath11k_peer_assoc_prepare' at drivers/net/wireless/ath/ath11k/mac.c:2665:2:
> drivers/net/wireless/ath/ath11k/mac.c:1709:13: error: 'ath11k_peer_assoc_h_ht_masked' reading 10 bytes from a region of size 0 [-Werror=stringop-overread]
>  1709 |         if (ath11k_peer_assoc_h_ht_masked(ht_mcs_mask))
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This happens whenever gcc-13 fails to inline one of the functions
> that take a fixed-length array argument but gets passed a pointer.
> 
> Change these functions to all take a regular pointer argument
> instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

Patch applied to ath-next branch of ath.git, thanks.

695df2f417d2 wifi: ath: work around false-positive stringop-overread warning
Arnd Bergmann June 3, 2023, 7:43 a.m. UTC | #8 
On Mon, May 8, 2023, at 17:07, Arnd Bergmann wrote:
> On Mon, May 8, 2023, at 16:57, Kalle Valo wrote:
>> With older GCC versions from your page I don't have this problem. I'm
>> using Debian 10 still so so is my libc too old?
>
> (dropping most Cc)
>
> Indeed, thanks for the report, I forgot about that issue. I used
> to build the cross toolchains in an old Ubuntu 16.04 chroot to avoid
> that issue, and I linked all other dependencies statically.
>
> The gcc-13.1.0 builds are the first ones I did on an arm64 machine,
> so I had to create a new build environment and started out with
> just my normal Debian testing rootfs, which caused me enough issues
> to figure out first.
>
> I had previously experimented with linking statically against
> musl to avoid all other dependencies, but that ended up with
> slower binaries because the default memory allocator in musl
> doesn't work that well for gcc, and I never quite figured out
> how to pick a different memory allocator, or which one to use.
>
> I should probably just pick an older Debian release that is new
> enough to contain cross compilers for arm64 and x86 and then
> set up the same kind of chroot I had in before.

It took me a while, but now I have a working build setup
in a Debian Buster schroot with gcc-13 as the main compiler,
and I updated the gcc-13.1 binaries with those, as well as
uploading gcc-11.4 and gcc-12.3 build the same way.

I have only tested the binaries on arm64 Debian testing,
could you see if the new x86 builds work for you?

       Arnd
Kalle Valo June 5, 2023, 1:06 p.m. UTC | #9 
"Arnd Bergmann" <arnd@kernel.org> writes:

> On Mon, May 8, 2023, at 17:07, Arnd Bergmann wrote:
>
>> On Mon, May 8, 2023, at 16:57, Kalle Valo wrote:
>>> With older GCC versions from your page I don't have this problem. I'm
>>> using Debian 10 still so so is my libc too old?
>>
>> (dropping most Cc)
>>
>> Indeed, thanks for the report, I forgot about that issue. I used
>> to build the cross toolchains in an old Ubuntu 16.04 chroot to avoid
>> that issue, and I linked all other dependencies statically.
>>
>> The gcc-13.1.0 builds are the first ones I did on an arm64 machine,
>> so I had to create a new build environment and started out with
>> just my normal Debian testing rootfs, which caused me enough issues
>> to figure out first.
>>
>> I had previously experimented with linking statically against
>> musl to avoid all other dependencies, but that ended up with
>> slower binaries because the default memory allocator in musl
>> doesn't work that well for gcc, and I never quite figured out
>> how to pick a different memory allocator, or which one to use.
>>
>> I should probably just pick an older Debian release that is new
>> enough to contain cross compilers for arm64 and x86 and then
>> set up the same kind of chroot I had in before.
>
> It took me a while, but now I have a working build setup
> in a Debian Buster schroot with gcc-13 as the main compiler,
> and I updated the gcc-13.1 binaries with those, as well as
> uploading gcc-11.4 and gcc-12.3 build the same way.
>
> I have only tested the binaries on arm64 Debian testing,
> could you see if the new x86 builds work for you?

I tested GCC 12.3 and 13.1 on x86 Debian 10, they both worked perfectly.
Thank you!
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index cad832e0e6b8..393669be7847 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -433,7 +433,7 @@  u8 ath11k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
 }
 
 static u32
-ath11k_mac_max_ht_nss(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
+ath11k_mac_max_ht_nss(const u8 *ht_mcs_mask)
 {
 	int nss;
 
@@ -445,7 +445,7 @@  ath11k_mac_max_ht_nss(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
 }
 
 static u32
-ath11k_mac_max_vht_nss(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
+ath11k_mac_max_vht_nss(const u16 *vht_mcs_mask)
 {
 	int nss;
 
@@ -457,7 +457,7 @@  ath11k_mac_max_vht_nss(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
 }
 
 static u32
-ath11k_mac_max_he_nss(const u16 he_mcs_mask[NL80211_HE_NSS_MAX])
+ath11k_mac_max_he_nss(const u16 *he_mcs_mask)
 {
 	int nss;
 
@@ -1658,7 +1658,7 @@  static void ath11k_peer_assoc_h_rates(struct ath11k *ar,
 }
 
 static bool
-ath11k_peer_assoc_h_ht_masked(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
+ath11k_peer_assoc_h_ht_masked(const u8 *ht_mcs_mask)
 {
 	int nss;
 
@@ -1670,7 +1670,7 @@  ath11k_peer_assoc_h_ht_masked(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
 }
 
 static bool
-ath11k_peer_assoc_h_vht_masked(const u16 vht_mcs_mask[])
+ath11k_peer_assoc_h_vht_masked(const u16 *vht_mcs_mask)
 {
 	int nss;
 
@@ -2065,7 +2065,7 @@  static u16 ath11k_peer_assoc_h_he_limit(u16 tx_mcs_set,
 }
 
 static bool
-ath11k_peer_assoc_h_he_masked(const u16 he_mcs_mask[NL80211_HE_NSS_MAX])
+ath11k_peer_assoc_h_he_masked(const u16 *he_mcs_mask)
 {
 	int nss;
 
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index d683a0668989..ea2313b6b759 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -381,7 +381,7 @@  u8 ath12k_mac_bitrate_to_idx(const struct ieee80211_supported_band *sband,
 }
 
 static u32
-ath12k_mac_max_ht_nss(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
+ath12k_mac_max_ht_nss(const u8 *ht_mcs_mask)
 {
 	int nss;
 
@@ -393,7 +393,7 @@  ath12k_mac_max_ht_nss(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
 }
 
 static u32
-ath12k_mac_max_vht_nss(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
+ath12k_mac_max_vht_nss(const u16 *vht_mcs_mask)
 {
 	int nss;
 
@@ -1303,7 +1303,7 @@  static void ath12k_peer_assoc_h_rates(struct ath12k *ar,
 }
 
 static bool
-ath12k_peer_assoc_h_ht_masked(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
+ath12k_peer_assoc_h_ht_masked(const u8 *ht_mcs_mask)
 {
 	int nss;
 
@@ -1315,7 +1315,7 @@  ath12k_peer_assoc_h_ht_masked(const u8 ht_mcs_mask[IEEE80211_HT_MCS_MASK_LEN])
 }
 
 static bool
-ath12k_peer_assoc_h_vht_masked(const u16 vht_mcs_mask[NL80211_VHT_NSS_MAX])
+ath12k_peer_assoc_h_vht_masked(const u16 *vht_mcs_mask)
 {
 	int nss;