linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[David Binderman]

Hello there,

I just tried to build linux-6.12-rc1 with clang. It said:

drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]

A grep for the identifier shows the following strange results::

inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
static const unsigned int bmi323_ext_reg_savestate[] = {
	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
linux-6.12-rc1 $ 

I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
cut'n'paste error in one of the two for loops ?

Regards

David Binderman

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Javier Carrasco]

On 30/09/2024 15:49, David Binderman wrote:
> Hello there,
> 
> I just tried to build linux-6.12-rc1 with clang. It said:
> 
> drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
> 
> A grep for the identifier shows the following strange results::
> 
> inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
> static const unsigned int bmi323_ext_reg_savestate[] = {
> 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> linux-6.12-rc1 $ 
> 
> I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
> cut'n'paste error in one of the two for loops ?
> 
> Regards
> 
> David Binderman


I think that is a bug in clang:

https://bugs.llvm.org/show_bug.cgi?id=33068

That happens because clang sees that bmi323_ext_reg_savestate is not
used but to gets its size, and that means for it that the variable is
not needed. That does not happen for example with
bmi323_ext_reg_savestate (right above bmi323_ext_reg_savestate) because
that one is used beyond ARRAY_SIZE.

Safe to ignore?

Best regards,
Javier Carrasco

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Javier Carrasco]

On 30/09/2024 15:58, Javier Carrasco wrote:
> On 30/09/2024 15:49, David Binderman wrote:
>> Hello there,
>>
>> I just tried to build linux-6.12-rc1 with clang. It said:
>>
>> drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
>>
>> A grep for the identifier shows the following strange results::
>>
>> inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
>> static const unsigned int bmi323_ext_reg_savestate[] = {
>> 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
>> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>> linux-6.12-rc1 $ 
>>
>> I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
>> cut'n'paste error in one of the two for loops ?
>>
>> Regards
>>
>> David Binderman
> 
> 
> I think that is a bug in clang:
> 
> https://bugs.llvm.org/show_bug.cgi?id=33068
> 
> That happens because clang sees that bmi323_ext_reg_savestate is not
> used but to gets its size, and that means for it that the variable is
> not needed. That does not happen for example with
> bmi323_ext_reg_savestate (right above bmi323_ext_reg_savestate) because

bmi323_reg_savestate :)

> that one is used beyond ARRAY_SIZE.
> 
> Safe to ignore?
> 
> Best regards,
> Javier Carrasco

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Nathan Chancellor]

On Mon, Sep 30, 2024 at 03:58:06PM +0200, Javier Carrasco wrote:
> On 30/09/2024 15:49, David Binderman wrote:
> > Hello there,
> > 
> > I just tried to build linux-6.12-rc1 with clang. It said:
> > 
> > drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
> > 
> > A grep for the identifier shows the following strange results::
> > 
> > inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
> > static const unsigned int bmi323_ext_reg_savestate[] = {
> > 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
> > 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> > 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> > linux-6.12-rc1 $ 
> > 
> > I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
> > cut'n'paste error in one of the two for loops ?
> > 
> > Regards
> > 
> > David Binderman
> 
> 
> I think that is a bug in clang:
> 
> https://bugs.llvm.org/show_bug.cgi?id=33068
> 
> That happens because clang sees that bmi323_ext_reg_savestate is not
> used but to gets its size, and that means for it that the variable is
> not needed. That does not happen for example with
> bmi323_ext_reg_savestate (right above bmi323_ext_reg_savestate) because
> that one is used beyond ARRAY_SIZE.
> 
> Safe to ignore?

As later comments in this thread point out, this is a bug in the code:

https://git.kernel.org/jic23/iio/c/506a1ac4c4464a61e4336e135841067dbc040aaa

That bug report is pretty misguided, as that is exactly the type of code
that warning tries to catch: the programmer has a static variable that
is only used in sizeof(), a compile time evaluation, so they won't get a
-Wunused-variable since it is used in that expression, but did they
intend to use it elsewhere? If they didn't, they can just slap
'__maybe_unused' / '__attribute__((unused))' on it or use
sizeof(<type>), which removes all ambiguity. Otherwise, the code should
be fixed, like in this case.

Cheers,
Nathan

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Javier Carrasco]

On 30/09/2024 17:15, Nathan Chancellor wrote:
> On Mon, Sep 30, 2024 at 03:58:06PM +0200, Javier Carrasco wrote:
>> On 30/09/2024 15:49, David Binderman wrote:
>>> Hello there,
>>>
>>> I just tried to build linux-6.12-rc1 with clang. It said:
>>>
>>> drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
>>>
>>> A grep for the identifier shows the following strange results::
>>>
>>> inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
>>> static const unsigned int bmi323_ext_reg_savestate[] = {
>>> 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
>>> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>>> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>>> linux-6.12-rc1 $ 
>>>
>>> I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
>>> cut'n'paste error in one of the two for loops ?
>>>
>>> Regards
>>>
>>> David Binderman
>>
>>
>> I think that is a bug in clang:
>>
>> https://bugs.llvm.org/show_bug.cgi?id=33068
>>
>> That happens because clang sees that bmi323_ext_reg_savestate is not
>> used but to gets its size, and that means for it that the variable is
>> not needed. That does not happen for example with
>> bmi323_ext_reg_savestate (right above bmi323_ext_reg_savestate) because
>> that one is used beyond ARRAY_SIZE.
>>
>> Safe to ignore?
> 
> As later comments in this thread point out, this is a bug in the code:
> 
> https://git.kernel.org/jic23/iio/c/506a1ac4c4464a61e4336e135841067dbc040aaa
> 
> That bug report is pretty misguided, as that is exactly the type of code
> that warning tries to catch: the programmer has a static variable that
> is only used in sizeof(), a compile time evaluation, so they won't get a
> -Wunused-variable since it is used in that expression, but did they
> intend to use it elsewhere? If they didn't, they can just slap
> '__maybe_unused' / '__attribute__((unused))' on it or use
> sizeof(<type>), which removes all ambiguity. Otherwise, the code should
> be fixed, like in this case.
> 
> Cheers,
> Nathan


@Nathan thanks for the clarification.

So in this case, the array should have been used not only to get its size.

But if that wasn't the case, and since you can't use sizeof(<type>),
should it be marked with __maybe_unused / __attribute__((unused)) even
though it's known in advance that it won't be used, or at least that its
use will be to get its size? Is it then just to silence the warning, or
does it have other implications? Thanks again!

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Nathan Chancellor]

Hi Javier,

On Mon, Sep 30, 2024 at 06:50:14PM +0200, Javier Carrasco wrote:
> But if that wasn't the case, and since you can't use sizeof(<type>),
> should it be marked with __maybe_unused / __attribute__((unused)) even
> though it's known in advance that it won't be used, or at least that its
> use will be to get its size?

Correct.

> Is it then just to silence the warning, or does it have other
> implications? Thanks again!

Yes, the use of the unused attribute would just be to silence the
warning; the variable would still not be emitted in the final binary.
clang's behavior matches GCC's (aside from the special warning):

https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-unused-variable-attribute

If the variable needed to be emitted in the object file,
__attribute__((used)) would need to be used, which explicitly has code
generation implications:

https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-used-variable-attribute

A contrived example:

https://godbolt.org/z/oGGbqK98o

Cheers,
Nathan

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Javier Carrasco]

On 30/09/2024 22:26, Nathan Chancellor wrote:
> Hi Javier,
> 
> On Mon, Sep 30, 2024 at 06:50:14PM +0200, Javier Carrasco wrote:
>> But if that wasn't the case, and since you can't use sizeof(<type>),
>> should it be marked with __maybe_unused / __attribute__((unused)) even
>> though it's known in advance that it won't be used, or at least that its
>> use will be to get its size?
> 
> Correct.
> 
>> Is it then just to silence the warning, or does it have other
>> implications? Thanks again!
> 
> Yes, the use of the unused attribute would just be to silence the
> warning; the variable would still not be emitted in the final binary.
> clang's behavior matches GCC's (aside from the special warning):
> 
> https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-unused-variable-attribute
> 
> If the variable needed to be emitted in the object file,
> __attribute__((used)) would need to be used, which explicitly has code
> generation implications:
> 
> https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-used-variable-attribute
> 
> A contrived example:
> 
> https://godbolt.org/z/oGGbqK98o
> 
> Cheers,
> Nathan

Thanks a lot for spoon-feeding it to me :)

Best regards,
Javier Carrasco

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Lars-Peter Clausen]

On 9/30/24 06:49, David Binderman wrote:
> Hello there,
>
> I just tried to build linux-6.12-rc1 with clang. It said:
>
> drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
>
> A grep for the identifier shows the following strange results::
>
> inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
> static const unsigned int bmi323_ext_reg_savestate[] = {
> 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> linux-6.12-rc1 $
>
> I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
> cut'n'paste error in one of the two for loops ?
Yes. Do you want to send a fix?

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Jonathan Cameron]

On Mon, 30 Sep 2024 07:19:14 -0700
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 9/30/24 06:49, David Binderman wrote:
> > Hello there,
> >
> > I just tried to build linux-6.12-rc1 with clang. It said:
> >
> > drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
> >
> > A grep for the identifier shows the following strange results::
> >
> > inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
> > static const unsigned int bmi323_ext_reg_savestate[] = {
> > 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
> > 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> > 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
> > linux-6.12-rc1 $
> >
> > I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
> > cut'n'paste error in one of the two for loops ?  
> Yes. Do you want to send a fix?
> 

Have a couple of fixes queued for this driver (including I Think this one).

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=fixes-togreg&id=506a1ac4c4464a61e4336e135841067dbc040aaa

I'm just letting the fixes sit in next for a day or two to make sure nothing
else shows up then I'll do a pull request.

Jonathan

Re: linux-6.12-rc1/drivers/iio/imu/bmi323/bmi323_core.c:133: Array contents defined but not used ?

[Denis Benato]

Il 30 settembre 2024 16:19:14 CEST, Lars-Peter Clausen <lars@metafoo.de> ha scritto:
>On 9/30/24 06:49, David Binderman wrote:
>> Hello there,
>> 
>> I just tried to build linux-6.12-rc1 with clang. It said:
>> 
>> drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
>> 
>> A grep for the identifier shows the following strange results::
>> 
>> inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
>> static const unsigned int bmi323_ext_reg_savestate[] = {
>> 	unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
>> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>> 	for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>> linux-6.12-rc1 $
>> 
>> I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
>> cut'n'paste error in one of the two for loops ?
>Yes. Do you want to send a fix?
>
Hello,

I think fixes are pending already in one of Jonathan Cameron's branches.

Best regards,
Denis Benato