diff mbox series

wifi: ath: add struct_group for struct ath_cycle_counters

Message ID TYAP286MB0315EC437BF53C8DB2B5D022BC4EA@TYAP286MB0315.JPNP286.PROD.OUTLOOK.COM (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: ath: add struct_group for struct ath_cycle_counters | expand

Commit Message

Shiji Yang June 2, 2023, 4:36 a.m. UTC
From: Shiji Yang <yangshiji66@qq.com>

Add a struct_group to around all members in struct ath_cycle_counters.
It can help the compiler detect the intended bounds of the memcpy() and
memset().

This patch fixes the following build warning:

In function 'fortify_memset_chk',
    inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning:
detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  314 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Shiji Yang <yangshiji66@qq.com>
---
More discussion on: https://github.com/openwrt/openwrt/pull/12764
---
 drivers/net/wireless/ath/ath.h                | 10 ++++++----
 drivers/net/wireless/ath/ath5k/ani.c          |  2 +-
 drivers/net/wireless/ath/ath5k/base.c         |  4 ++--
 drivers/net/wireless/ath/ath5k/mac80211-ops.c |  2 +-
 drivers/net/wireless/ath/ath9k/link.c         |  2 +-
 drivers/net/wireless/ath/ath9k/main.c         |  4 ++--
 drivers/net/wireless/ath/hw.c                 |  2 +-
 7 files changed, 14 insertions(+), 12 deletions(-)

Comments

Jiri Slaby June 2, 2023, 4:58 a.m. UTC | #1
On 02. 06. 23, 6:36, Shiji Yang wrote:
> From: Shiji Yang <yangshiji66@qq.com>
> 
> Add a struct_group to around all members in struct ath_cycle_counters.
> It can help the compiler detect the intended bounds of the memcpy() and
> memset().
> 
> This patch fixes the following build warning:
> 
> In function 'fortify_memset_chk',
>      inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
> ./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning:
> detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>    314 |                         __write_overflow_field(p_size_field, size);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 

Hi,

what compiler/version is this with?

> Signed-off-by: Shiji Yang <yangshiji66@qq.com>
> ---
> More discussion on: https://github.com/openwrt/openwrt/pull/12764

No "__write_overflow_field" there. Is this the right link?

> ---
>   drivers/net/wireless/ath/ath.h                | 10 ++++++----
>   drivers/net/wireless/ath/ath5k/ani.c          |  2 +-
>   drivers/net/wireless/ath/ath5k/base.c         |  4 ++--
>   drivers/net/wireless/ath/ath5k/mac80211-ops.c |  2 +-
>   drivers/net/wireless/ath/ath9k/link.c         |  2 +-
>   drivers/net/wireless/ath/ath9k/main.c         |  4 ++--
>   drivers/net/wireless/ath/hw.c                 |  2 +-
>   7 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
> index f02a308a9..4b42e401a 100644
> --- a/drivers/net/wireless/ath/ath.h
> +++ b/drivers/net/wireless/ath/ath.h
> @@ -43,10 +43,12 @@ struct ath_ani {
>   };
>   
>   struct ath_cycle_counters {
> -	u32 cycles;
> -	u32 rx_busy;
> -	u32 rx_frame;
> -	u32 tx_frame;
> +	struct_group(cnts,
> +		u32 cycles;
> +		u32 rx_busy;
> +		u32 rx_frame;
> +		u32 tx_frame;
> +	);

This is horrid.

>   };
>   
>   enum ath_device_state {
> diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
> index 850c608b4..fa95f0f0f 100644
> --- a/drivers/net/wireless/ath/ath5k/ani.c
> +++ b/drivers/net/wireless/ath/ath5k/ani.c
> @@ -379,7 +379,7 @@ ath5k_hw_ani_get_listen_time(struct ath5k_hw *ah, struct ath5k_ani_state *as)
>   	spin_lock_bh(&common->cc_lock);
>   
>   	ath_hw_cycle_counters_update(common);
> -	memcpy(&as->last_cc, &common->cc_ani, sizeof(as->last_cc));
> +	memcpy(&as->last_cc.cnts, &common->cc_ani.cnts, sizeof(as->last_cc.cnts));

So is this.

Care to elaborate why this is needed at all, provided we copy/zero a 
whole structure? And describe it in the commit log, not in random 
external sources.

thanks,
Kalle Valo June 2, 2023, 5:19 a.m. UTC | #2
Jiri Slaby <jirislaby@kernel.org> writes:

> On 02. 06. 23, 6:36, Shiji Yang wrote:
>> From: Shiji Yang <yangshiji66@qq.com>
>>
>> Add a struct_group to around all members in struct ath_cycle_counters.
>> It can help the compiler detect the intended bounds of the memcpy() and
>> memset().
>>
>> This patch fixes the following build warning:
>>
>> In function 'fortify_memset_chk',
>>      inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
>> ./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning:
>> detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>>    314 |                         __write_overflow_field(p_size_field, size);
>>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>
> Hi,
>
> what compiler/version is this with?
>
>> Signed-off-by: Shiji Yang <yangshiji66@qq.com>
>> ---
>> More discussion on: https://github.com/openwrt/openwrt/pull/12764
>
> No "__write_overflow_field" there. Is this the right link?

Also it's better to include links like this in the actual commit log so
it's archived to git.
Shiji Yang June 4, 2023, 1:40 a.m. UTC | #3
Thank you for your reply.
>On 02. 06. 23, 6:36, Shiji Yang wrote:
>> From: Shiji Yang <yangshiji66@qq.com>
>> 
>> Add a struct_group to around all members in struct ath_cycle_counters.
>> It can help the compiler detect the intended bounds of the memcpy() and
>> memset().
>> 
>> This patch fixes the following build warning:
>> 
>> In function 'fortify_memset_chk',
>>      inlined from 'ath9k_ps_wakeup' at drivers/net/wireless/ath/ath9k/main.c:140:3:
>> ./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning:
>> detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>>    314 |                         __write_overflow_field(p_size_field, size);
>>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>
>Hi,
>
>what compiler/version is this with?

I'm using the mips gcc 12.3 (for arc ath79). It seems to be related
to the compiler, and arm gcc 12.3 does not show compilation warnings.

>> Signed-off-by: Shiji Yang <yangshiji66@qq.com>
>> ---
>> More discussion on: https://github.com/openwrt/openwrt/pull/12764
>
>No "__write_overflow_field" there. Is this the right link?

Yes. However, only few of the discussion here is related to compilation errors.

The full log:

make[4]: Entering directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24'
make[5]: 'Kconfig.versions' is up to date.
make[7]: 'Kconfig.versions' is up to date.
make[8]: 'conf' is up to date.
boolean symbol CRYPTO_LIB_ARC4 tested for 'm'? test forced to 'n'
#
# configuration written to .config
#
Building backport-include/backport/autoconf.h ... done.
  CC [M]  /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o
In file included from ./include/linux/string.h:253,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/string.h:3,
                 from ./include/linux/bitmap.h:11,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/mips/include/asm/processor.h:15,
                 from ./arch/mips/include/asm/thread_info.h:16,
                 from ./include/linux/thread_info.h:60,
                 from ./include/asm-generic/current.h:5,
                 from ./arch/mips/include/generated/asm/current.h:1,
                 from ./include/linux/sched.h:12,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/sched.h:4,
                 from ./include/linux/delay.h:23,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/delay.h:3,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:18:
In function 'fortify_memset_chk',
    inlined from 'ath9k_ps_wakeup' at /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:140:3:
./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  314 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[11]: *** [scripts/Makefile.build:250: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o] Error 1
make[10]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k] Error 2
make[9]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath] Error 2
make[8]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless] Error 2
make[7]: *** [Makefile:2012: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24] Error 2
make[6]: *** [Makefile.build:13: modules] Error 2
make[5]: *** [Makefile.real:93: modules] Error 2
make[4]: *** [Makefile:121: modules] Error 2
make[4]: Leaving directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24'
make[3]: *** [Makefile:401: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/.built] Error 2
make[3]: Leaving directory '/home/db/openwrt/package/kernel/mac80211'
time: package/kernel/mac80211/regular/compile#4.94#0.95#21.75
    ERROR: package/kernel/mac80211 failed to build (build variant: regular).
make[2]: *** [package/Makefile:120: package/kernel/mac80211/compile] Error 1
make[2]: Leaving directory '/home/db/openwrt'
make[1]: *** [package/Makefile:114: /home/db/openwrt/staging_dir/target-mips_24kc_musl/stamp/.package_compile] Error 2
make[1]: Leaving directory '/home/db/openwrt'
make: *** [/home/db/openwrt/include/toplevel.mk:231: world] Error 2

>> ---
>>   drivers/net/wireless/ath/ath.h                | 10 ++++++----
>>   drivers/net/wireless/ath/ath5k/ani.c          |  2 +-
>>   drivers/net/wireless/ath/ath5k/base.c         |  4 ++--
>>   drivers/net/wireless/ath/ath5k/mac80211-ops.c |  2 +-
>>   drivers/net/wireless/ath/ath9k/link.c         |  2 +-
>>   drivers/net/wireless/ath/ath9k/main.c         |  4 ++--
>>   drivers/net/wireless/ath/hw.c                 |  2 +-
>>   7 files changed, 14 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
>> index f02a308a9..4b42e401a 100644
>> --- a/drivers/net/wireless/ath/ath.h
>> +++ b/drivers/net/wireless/ath/ath.h
>> @@ -43,10 +43,12 @@ struct ath_ani {
>>   };
>>   
>>   struct ath_cycle_counters {
>> -	u32 cycles;
>> -	u32 rx_busy;
>> -	u32 rx_frame;
>> -	u32 tx_frame;
>> +	struct_group(cnts,
>> +		u32 cycles;
>> +		u32 rx_busy;
>> +		u32 rx_frame;
>> +		u32 tx_frame;
>> +	);
>
>This is horrid.

Yes, I agree, but this can avoid making more changes.

>>   };
>>   
>>   enum ath_device_state {
>> diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
>> index 850c608b4..fa95f0f0f 100644
>> --- a/drivers/net/wireless/ath/ath5k/ani.c
>> +++ b/drivers/net/wireless/ath/ath5k/ani.c
>> @@ -379,7 +379,7 @@ ath5k_hw_ani_get_listen_time(struct ath5k_hw *ah, struct ath5k_ani_state *as)
>>   	spin_lock_bh(&common->cc_lock);
>>   
>>   	ath_hw_cycle_counters_update(common);
>> -	memcpy(&as->last_cc, &common->cc_ani, sizeof(as->last_cc));
>> +	memcpy(&as->last_cc.cnts, &common->cc_ani.cnts, sizeof(as->last_cc.cnts));
>
>So is this.
>
>Care to elaborate why this is needed at all, provided we copy/zero a 
>whole structure? And describe it in the commit log, not in random 
>external sources.

Thank you for the prompt. I will pay attention to the format of the patch next time.

The compiler did not complain here. But in

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a4197c14f..8608a29a1 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -135,8 +135,8 @@ void ath9k_ps_wakeup(struct ath_softc *sc)
 	if (power_mode != ATH9K_PM_AWAKE) {
 		spin_lock(&common->cc_lock);
 		ath_hw_cycle_counters_update(common);
-		memset(&common->cc_survey, 0, sizeof(common->cc_survey));
-		memset(&common->cc_ani, 0, sizeof(common->cc_ani));
+		memset(&common->cc_survey.cnts, 0, sizeof(common->cc_survey.cnts));
+		memset(&common->cc_ani.cnts, 0, sizeof(common->cc_ani.cnts));
 		spin_unlock(&common->cc_lock);
 	}
 
Here, memset() is used to zero the entire structure. The compiler will only warn
the second memset() `memset(&common->cc_ani, 0, sizeof(common->cc_ani));` However,
`cc_survey` and `cc_survey` are the same structure.

>
>thanks,
>-- 
>js
>suse labs
Shiji Yang June 4, 2023, 1:46 a.m. UTC | #4
>Also it's better to include links like this in the actual commit log so
>it's archived to git.

Thanks for the tips. If necessary, I will update it in v2.
Shiji Yang June 4, 2023, 1:48 a.m. UTC | #5
Thank you all,
This patch may not look so beautiful, but its main purpose is to raise
some awareness about this strange compilation warning.

This problem only occurs in mips gcc 12.3 (maybe only on arc mips/ath79).
In:

>--- a/drivers/net/wireless/ath/ath9k/main.c
>+++ b/drivers/net/wireless/ath/ath9k/main.c
>@@ -135,8 +135,8 @@ void ath9k_ps_wakeup(struct ath_softc *sc)
> 	if (power_mode != ATH9K_PM_AWAKE) {
> 		spin_lock(&common->cc_lock);
> 		ath_hw_cycle_counters_update(common);
>-		memset(&common->cc_survey, 0, sizeof(common->cc_survey));
>-		memset(&common->cc_ani, 0, sizeof(common->cc_ani));
>+		memset(&common->cc_survey.cnts, 0, sizeof(common->cc_survey.cnts));
>+		memset(&common->cc_ani.cnts, 0, sizeof(common->cc_ani.cnts));
> 		spin_unlock(&common->cc_lock);
> 	}

The compiler will only warn the second memset() `memset(&common->cc_ani, 0, sizeof(common->cc_ani));`
detected write beyond size of field. However, `cc_survey` and `cc_survey` are the same structure.
I have no idea about this warning. I will be very grateful if someone can provide some tips or a
better solution.

The full log:
make[4]: Entering directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24'
make[5]: 'Kconfig.versions' is up to date.
make[7]: 'Kconfig.versions' is up to date.
make[8]: 'conf' is up to date.
boolean symbol CRYPTO_LIB_ARC4 tested for 'm'? test forced to 'n'
#
# configuration written to .config
#
Building backport-include/backport/autoconf.h ... done.
  CC [M]  /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o
In file included from ./include/linux/string.h:253,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/string.h:3,
                 from ./include/linux/bitmap.h:11,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/mips/include/asm/processor.h:15,
                 from ./arch/mips/include/asm/thread_info.h:16,
                 from ./include/linux/thread_info.h:60,
                 from ./include/asm-generic/current.h:5,
                 from ./arch/mips/include/generated/asm/current.h:1,
                 from ./include/linux/sched.h:12,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/sched.h:4,
                 from ./include/linux/delay.h:23,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/delay.h:3,
                 from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:18:
In function 'fortify_memset_chk',
    inlined from 'ath9k_ps_wakeup' at /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:140:3:
./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
  314 |                         __write_overflow_field(p_size_field, size);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[11]: *** [scripts/Makefile.build:250: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o] Error 1
make[10]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k] Error 2
make[9]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath] Error 2
make[8]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless] Error 2
make[7]: *** [Makefile:2012: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24] Error 2
make[6]: *** [Makefile.build:13: modules] Error 2
make[5]: *** [Makefile.real:93: modules] Error 2
make[4]: *** [Makefile:121: modules] Error 2
make[4]: Leaving directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24'
make[3]: *** [Makefile:401: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/.built] Error 2
make[3]: Leaving directory '/home/db/openwrt/package/kernel/mac80211'
time: package/kernel/mac80211/regular/compile#4.94#0.95#21.75
    ERROR: package/kernel/mac80211 failed to build (build variant: regular).
make[2]: *** [package/Makefile:120: package/kernel/mac80211/compile] Error 1
make[2]: Leaving directory '/home/db/openwrt'
make[1]: *** [package/Makefile:114: /home/db/openwrt/staging_dir/target-mips_24kc_musl/stamp/.package_compile] Error 2
make[1]: Leaving directory '/home/db/openwrt'
make: *** [/home/db/openwrt/include/toplevel.mk:231: world] Error 2

Best Regards,
Shiji Yang
Christian Lamparter June 4, 2023, 11:11 a.m. UTC | #6
On 6/4/23 03:48, Shiji Yang wrote:
> Thank you all,
> This patch may not look so beautiful, but its main purpose is to raise
> some awareness about this strange compilation warning.
> 
> This problem only occurs in mips gcc 12.3 (maybe only on arc mips/ath79).

It also occures with PPC464/APM82181. Could it be that that it occurs on
"big-endian" archs?


> In:
> 
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -135,8 +135,8 @@ void ath9k_ps_wakeup(struct ath_softc *sc)
>> 	if (power_mode != ATH9K_PM_AWAKE) {
>> 		spin_lock(&common->cc_lock);
>> 		ath_hw_cycle_counters_update(common);
>> -		memset(&common->cc_survey, 0, sizeof(common->cc_survey));
>> -		memset(&common->cc_ani, 0, sizeof(common->cc_ani));
>> +		memset(&common->cc_survey.cnts, 0, sizeof(common->cc_survey.cnts));
>> +		memset(&common->cc_ani.cnts, 0, sizeof(common->cc_ani.cnts));
>> 		spin_unlock(&common->cc_lock);
>> 	}
> 
> The compiler will only warn the second memset() `memset(&common->cc_ani, 0, sizeof(common->cc_ani));`
> detected write beyond size of field. However, `cc_survey` and `cc_survey` are the same structure.
> I have no idea about this warning. I will be very grateful if someone can provide some tips or a
> better solution.
> 
> The full log:
> make[4]: Entering directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24'
> make[5]: 'Kconfig.versions' is up to date.
> make[7]: 'Kconfig.versions' is up to date.
> make[8]: 'conf' is up to date.
> boolean symbol CRYPTO_LIB_ARC4 tested for 'm'? test forced to 'n'
> #
> # configuration written to .config
> #
> Building backport-include/backport/autoconf.h ... done.
>    CC [M]  /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o
> In file included from ./include/linux/string.h:253,
>                   from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/string.h:3,
>                   from ./include/linux/bitmap.h:11,
>                   from ./include/linux/cpumask.h:12,
>                   from ./arch/mips/include/asm/processor.h:15,
>                   from ./arch/mips/include/asm/thread_info.h:16,
>                   from ./include/linux/thread_info.h:60,
>                   from ./include/asm-generic/current.h:5,
>                   from ./arch/mips/include/generated/asm/current.h:1,
>                   from ./include/linux/sched.h:12,
>                   from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/sched.h:4,
>                   from ./include/linux/delay.h:23,
>                   from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/backport-include/linux/delay.h:3,
>                   from /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:18:
> In function 'fortify_memset_chk',
>      inlined from 'ath9k_ps_wakeup' at /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.c:140:3:
> ./include/linux/fortify-string.h:314:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
>    314 |                         __write_overflow_field(p_size_field, size);
>        |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> make[11]: *** [scripts/Makefile.build:250: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k/main.o] Error 1
> make[10]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath/ath9k] Error 2
> make[9]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless/ath] Error 2
> make[8]: *** [scripts/Makefile.build:500: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/drivers/net/wireless] Error 2
> make[7]: *** [Makefile:2012: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24] Error 2
> make[6]: *** [Makefile.build:13: modules] Error 2
> make[5]: *** [Makefile.real:93: modules] Error 2
> make[4]: *** [Makefile:121: modules] Error 2
> make[4]: Leaving directory '/home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24'
> make[3]: *** [Makefile:401: /home/db/openwrt/build_dir/target-mips_24kc_musl/linux-ath79_nand/backports-6.1.24/.built] Error 2
> make[3]: Leaving directory '/home/db/openwrt/package/kernel/mac80211'
> time: package/kernel/mac80211/regular/compile#4.94#0.95#21.75
>      ERROR: package/kernel/mac80211 failed to build (build variant: regular).
> make[2]: *** [package/Makefile:120: package/kernel/mac80211/compile] Error 1
> make[2]: Leaving directory '/home/db/openwrt'
> make[1]: *** [package/Makefile:114: /home/db/openwrt/staging_dir/target-mips_24kc_musl/stamp/.package_compile] Error 2
> make[1]: Leaving directory '/home/db/openwrt'
> make: *** [/home/db/openwrt/include/toplevel.mk:231: world] Error 2
> 
> Best Regards,
> Shiji Yang
Jiri Slaby June 5, 2023, 6 a.m. UTC | #7
On 04. 06. 23, 13:11, Christian Lamparter wrote:
> On 6/4/23 03:48, Shiji Yang wrote:
>> Thank you all,
>> This patch may not look so beautiful, but its main purpose is to raise
>> some awareness about this strange compilation warning.
>>
>> This problem only occurs in mips gcc 12.3 (maybe only on arc mips/ath79).
> 
> It also occures with PPC464/APM82181. Could it be that that it occurs on
> "big-endian" archs?

Whatever, to me this very looks like a compiler bug.

So NACK to this one for now.

If this still happens on the latest tree (e.g. 6.4-rc) and with gcc 13, 
we might need to fix fortify or report to the gcc bugzilla [1] instead 
-- attach a preprocessed source there (the output of your make 
CROSSblabla=xyz drivers/net/wireless/ath/ath9k/main.i).

[1] https://gcc.gnu.org/bugzilla/enter_bug.cgi

thanks,
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index f02a308a9..4b42e401a 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -43,10 +43,12 @@  struct ath_ani {
 };
 
 struct ath_cycle_counters {
-	u32 cycles;
-	u32 rx_busy;
-	u32 rx_frame;
-	u32 tx_frame;
+	struct_group(cnts,
+		u32 cycles;
+		u32 rx_busy;
+		u32 rx_frame;
+		u32 tx_frame;
+	);
 };
 
 enum ath_device_state {
diff --git a/drivers/net/wireless/ath/ath5k/ani.c b/drivers/net/wireless/ath/ath5k/ani.c
index 850c608b4..fa95f0f0f 100644
--- a/drivers/net/wireless/ath/ath5k/ani.c
+++ b/drivers/net/wireless/ath/ath5k/ani.c
@@ -379,7 +379,7 @@  ath5k_hw_ani_get_listen_time(struct ath5k_hw *ah, struct ath5k_ani_state *as)
 	spin_lock_bh(&common->cc_lock);
 
 	ath_hw_cycle_counters_update(common);
-	memcpy(&as->last_cc, &common->cc_ani, sizeof(as->last_cc));
+	memcpy(&as->last_cc.cnts, &common->cc_ani.cnts, sizeof(as->last_cc.cnts));
 
 	/* clears common->cc_ani */
 	listen = ath_hw_get_listen_time(common);
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index c59c14483..efe072e7a 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2985,8 +2985,8 @@  ath5k_reset(struct ath5k_hw *ah, struct ieee80211_channel *chan,
 	memset(&ah->survey, 0, sizeof(ah->survey));
 	spin_lock_bh(&common->cc_lock);
 	ath_hw_cycle_counters_update(common);
-	memset(&common->cc_survey, 0, sizeof(common->cc_survey));
-	memset(&common->cc_ani, 0, sizeof(common->cc_ani));
+	memset(&common->cc_survey.cnts, 0, sizeof(common->cc_survey.cnts));
+	memset(&common->cc_ani.cnts, 0, sizeof(common->cc_ani.cnts));
 	spin_unlock_bh(&common->cc_lock);
 
 	/*
diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
index 11ed30d6b..eb62115b1 100644
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
@@ -665,7 +665,7 @@  ath5k_get_survey(struct ieee80211_hw *hw, int idx, struct survey_info *survey)
 		ah->survey.time_rx += cc->rx_frame / div;
 		ah->survey.time_tx += cc->tx_frame / div;
 	}
-	memset(cc, 0, sizeof(*cc));
+	memset(&cc->cnts, 0, sizeof(cc->cnts));
 	spin_unlock_bh(&common->cc_lock);
 
 	memcpy(survey, &ah->survey, sizeof(*survey));
diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c
index 9d84003db..0d557e6b6 100644
--- a/drivers/net/wireless/ath/ath9k/link.c
+++ b/drivers/net/wireless/ath/ath9k/link.c
@@ -536,7 +536,7 @@  int ath_update_survey_stats(struct ath_softc *sc)
 	if (cc->cycles > 0)
 		ret = cc->rx_busy * 100 / cc->cycles;
 
-	memset(cc, 0, sizeof(*cc));
+	memset(&cc->cnts, 0, sizeof(cc->cnts));
 
 	ath_update_survey_nf(sc, pos);
 
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a4197c14f..8608a29a1 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -135,8 +135,8 @@  void ath9k_ps_wakeup(struct ath_softc *sc)
 	if (power_mode != ATH9K_PM_AWAKE) {
 		spin_lock(&common->cc_lock);
 		ath_hw_cycle_counters_update(common);
-		memset(&common->cc_survey, 0, sizeof(common->cc_survey));
-		memset(&common->cc_ani, 0, sizeof(common->cc_ani));
+		memset(&common->cc_survey.cnts, 0, sizeof(common->cc_survey.cnts));
+		memset(&common->cc_ani.cnts, 0, sizeof(common->cc_ani.cnts));
 		spin_unlock(&common->cc_lock);
 	}
 
diff --git a/drivers/net/wireless/ath/hw.c b/drivers/net/wireless/ath/hw.c
index 85955572a..be8bfed9d 100644
--- a/drivers/net/wireless/ath/hw.c
+++ b/drivers/net/wireless/ath/hw.c
@@ -183,7 +183,7 @@  int32_t ath_hw_get_listen_time(struct ath_common *common)
 	listen_time = (cc->cycles - cc->rx_frame - cc->tx_frame) /
 		      (common->clockrate * 1000);
 
-	memset(cc, 0, sizeof(*cc));
+	memset(&cc->cnts, 0, sizeof(cc->cnts));
 
 	return listen_time;
 }