Re: [PATCH] Makefile: Globally enable fall-through warning

Re: [PATCH] Makefile: Globally enable fall-through warning

[Guenter Roeck]

On Thu, Jun 06, 2019 at 07:46:17PM -0500, Gustavo A. R. Silva wrote:
> Now that all the fall-through warnings have been addressed in the
> kernel, enable the fall-through warning globally.
> 

Not really "all".

powerpc:85xx/sbc8548_defconfig:

arch/powerpc/kernel/align.c: In function ‘emulate_spe’:
arch/powerpc/kernel/align.c:178:8: error: this statement may fall through

Plus many more similar errors in the same file.

All sh builds:

arch/sh/kernel/disassemble.c: In function 'print_sh_insn':
arch/sh/kernel/disassemble.c:478:8: error: this statement may fall through

Again, this is seen in several places.

mips:cavium_octeon_defconfig:

arch/mips/cavium-octeon/octeon-usb.c: In function 'dwc3_octeon_clocks_start':
include/linux/device.h:1499:2: error: this statement may fall through

None of those are from recent changes. And this is just from my small
subset of test builds.

Guenter

Re: [PATCH] Makefile: Globally enable fall-through warning

[Gustavo A. R. Silva]

Hi Guenter,

On 7/28/19 8:58 AM, Guenter Roeck wrote:
> On Thu, Jun 06, 2019 at 07:46:17PM -0500, Gustavo A. R. Silva wrote:
>> Now that all the fall-through warnings have been addressed in the
>> kernel, enable the fall-through warning globally.
>>
> 
> Not really "all".
> 
> powerpc:85xx/sbc8548_defconfig:
> 
> arch/powerpc/kernel/align.c: In function ‘emulate_spe’:
> arch/powerpc/kernel/align.c:178:8: error: this statement may fall through
> 
> Plus many more similar errors in the same file.
> 
> All sh builds:
> 
> arch/sh/kernel/disassemble.c: In function 'print_sh_insn':
> arch/sh/kernel/disassemble.c:478:8: error: this statement may fall through
> 
> Again, this is seen in several places.
> 
> mips:cavium_octeon_defconfig:
> 
> arch/mips/cavium-octeon/octeon-usb.c: In function 'dwc3_octeon_clocks_start':
> include/linux/device.h:1499:2: error: this statement may fall through
> 
> None of those are from recent changes. And this is just from my small
> subset of test builds.
> 

Thank you for letting me know about this. I don't have access to build
infrastructure like yours.

My build infrastructure is similar to that of Linus.

But if you send me all of those I can create a patch and send it back
to you to make sure what you see is addressed. If we can coordinate for
this it'd be great for everybody. :)

Thanks
--
Gustavo

Re: [PATCH] Makefile: Globally enable fall-through warning

[Guenter Roeck]

Hi Gustavo,

On Sun, Jul 28, 2019 at 11:42:28AM -0500, Gustavo A. R. Silva wrote:
> Hi Guenter,
> 
> On 7/28/19 8:58 AM, Guenter Roeck wrote:
> > On Thu, Jun 06, 2019 at 07:46:17PM -0500, Gustavo A. R. Silva wrote:
> >> Now that all the fall-through warnings have been addressed in the
> >> kernel, enable the fall-through warning globally.
> >>
> > 
> > Not really "all".
> > 
> > powerpc:85xx/sbc8548_defconfig:
> > 
> > arch/powerpc/kernel/align.c: In function ‘emulate_spe’:
> > arch/powerpc/kernel/align.c:178:8: error: this statement may fall through
> > 
> > Plus many more similar errors in the same file.
> > 
> > All sh builds:
> > 
> > arch/sh/kernel/disassemble.c: In function 'print_sh_insn':
> > arch/sh/kernel/disassemble.c:478:8: error: this statement may fall through
> > 
> > Again, this is seen in several places.
> > 
> > mips:cavium_octeon_defconfig:
> > 
> > arch/mips/cavium-octeon/octeon-usb.c: In function 'dwc3_octeon_clocks_start':
> > include/linux/device.h:1499:2: error: this statement may fall through
> > 
> > None of those are from recent changes. And this is just from my small
> > subset of test builds.
> > 
> 
> Thank you for letting me know about this. I don't have access to build
> infrastructure like yours.
> 

I am always happy to run test builds on my infrastructure.

> My build infrastructure is similar to that of Linus.
> 
> But if you send me all of those I can create a patch and send it back
> to you to make sure what you see is addressed. If we can coordinate for
> this it'd be great for everybody. :)
> 

Just have a look at the output of https://kerneltests.org/builders/,
in the 'master' and/or 'next' column. There are many additional warnings
in 'next'. Only downside is that you won't see the warnings unless there
are also build errors, but -next tends to have lots of those.

Just wondering ... wouldn't it be possible to run a coccinelle script
to identify those problems automatically, without depending on compile
warnings ? Or smatch/sparse, maybe ?

Thanks,
Guenter

Re: [PATCH] Makefile: Globally enable fall-through warning

[Gustavo A. R. Silva]

On 7/28/19 12:14 PM, Guenter Roeck wrote:
> Hi Gustavo,
> 
> On Sun, Jul 28, 2019 at 11:42:28AM -0500, Gustavo A. R. Silva wrote:
>> Hi Guenter,
>>
>> On 7/28/19 8:58 AM, Guenter Roeck wrote:
>>> On Thu, Jun 06, 2019 at 07:46:17PM -0500, Gustavo A. R. Silva wrote:
>>>> Now that all the fall-through warnings have been addressed in the
>>>> kernel, enable the fall-through warning globally.
>>>>
>>>
>>> Not really "all".
>>>
>>> powerpc:85xx/sbc8548_defconfig:
>>>
>>> arch/powerpc/kernel/align.c: In function ‘emulate_spe’:
>>> arch/powerpc/kernel/align.c:178:8: error: this statement may fall through
>>>
>>> Plus many more similar errors in the same file.
>>>
>>> All sh builds:
>>>
>>> arch/sh/kernel/disassemble.c: In function 'print_sh_insn':
>>> arch/sh/kernel/disassemble.c:478:8: error: this statement may fall through
>>>
>>> Again, this is seen in several places.
>>>
>>> mips:cavium_octeon_defconfig:
>>>
>>> arch/mips/cavium-octeon/octeon-usb.c: In function 'dwc3_octeon_clocks_start':
>>> include/linux/device.h:1499:2: error: this statement may fall through
>>>
>>> None of those are from recent changes. And this is just from my small
>>> subset of test builds.
>>>
>>
>> Thank you for letting me know about this. I don't have access to build
>> infrastructure like yours.
>>
> 
> I am always happy to run test builds on my infrastructure.
> 

Thank you!

>> My build infrastructure is similar to that of Linus.
>>
>> But if you send me all of those I can create a patch and send it back
>> to you to make sure what you see is addressed. If we can coordinate for
>> this it'd be great for everybody. :)
>>
> 
> Just have a look at the output of https://kerneltests.org/builders/,
> in the 'master' and/or 'next' column. There are many additional warnings
> in 'next'. Only downside is that you won't see the warnings unless there
> are also build errors, but -next tends to have lots of those.
> 

I see.

mm... for some reason I'm not able to establish connection with that site...

> Just wondering ... wouldn't it be possible to run a coccinelle script
> to identify those problems automatically, without depending on compile
> warnings ? Or smatch/sparse, maybe ?
> 

That was a common question from people along the whole process. The short
answer is: no. The reason for that is that Coccinelle is not a sophisticated
enough tool to determine if we are dealing with a false positive or an actual
bug.

That's why a code audit was needed.

Thanks
--
Gustavo

Re: [PATCH] Makefile: Globally enable fall-through warning

[Guenter Roeck]

On 7/28/19 10:31 AM, Gustavo A. R. Silva wrote:
> 
> 
> On 7/28/19 12:14 PM, Guenter Roeck wrote:
>> Hi Gustavo,
>>
>> On Sun, Jul 28, 2019 at 11:42:28AM -0500, Gustavo A. R. Silva wrote:
>>> Hi Guenter,
>>>
>>> On 7/28/19 8:58 AM, Guenter Roeck wrote:
>>>> On Thu, Jun 06, 2019 at 07:46:17PM -0500, Gustavo A. R. Silva wrote:
>>>>> Now that all the fall-through warnings have been addressed in the
>>>>> kernel, enable the fall-through warning globally.
>>>>>
>>>>
>>>> Not really "all".
>>>>
>>>> powerpc:85xx/sbc8548_defconfig:
>>>>
>>>> arch/powerpc/kernel/align.c: In function ‘emulate_spe’:
>>>> arch/powerpc/kernel/align.c:178:8: error: this statement may fall through
>>>>
>>>> Plus many more similar errors in the same file.
>>>>
>>>> All sh builds:
>>>>
>>>> arch/sh/kernel/disassemble.c: In function 'print_sh_insn':
>>>> arch/sh/kernel/disassemble.c:478:8: error: this statement may fall through
>>>>
>>>> Again, this is seen in several places.
>>>>
>>>> mips:cavium_octeon_defconfig:
>>>>
>>>> arch/mips/cavium-octeon/octeon-usb.c: In function 'dwc3_octeon_clocks_start':
>>>> include/linux/device.h:1499:2: error: this statement may fall through
>>>>
>>>> None of those are from recent changes. And this is just from my small
>>>> subset of test builds.
>>>>
>>>
>>> Thank you for letting me know about this. I don't have access to build
>>> infrastructure like yours.
>>>
>>
>> I am always happy to run test builds on my infrastructure.
>>
> 
> Thank you!
> 
>>> My build infrastructure is similar to that of Linus.
>>>
>>> But if you send me all of those I can create a patch and send it back
>>> to you to make sure what you see is addressed. If we can coordinate for
>>> this it'd be great for everybody. :)
>>>
>>
>> Just have a look at the output of https://kerneltests.org/builders/,
>> in the 'master' and/or 'next' column. There are many additional warnings
>> in 'next'. Only downside is that you won't see the warnings unless there
>> are also build errors, but -next tends to have lots of those.
>>
> 
> I see.
> 
> mm... for some reason I'm not able to establish connection with that site...
> 

What is your host's IP address ? It might be auto-filtered; the site is hosted
on my own server and is under constant attack. Any subnet originating a sequence
of attacks will be blocked automatically.

Thanks,
Guenter

>> Just wondering ... wouldn't it be possible to run a coccinelle script
>> to identify those problems automatically, without depending on compile
>> warnings ? Or smatch/sparse, maybe ?
>>
> 
> That was a common question from people along the whole process. The short
> answer is: no. The reason for that is that Coccinelle is not a sophisticated
> enough tool to determine if we are dealing with a false positive or an actual
> bug.
> 
> That's why a code audit was needed.
> 
> Thanks
> --
> Gustavo
> 
> 
> 
> 
> 
> 

Re: [PATCH] Makefile: Globally enable fall-through warning

[Geert Uytterhoeven]

Hi Gustavo,

On Sun, Jul 28, 2019 at 6:44 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> On 7/28/19 8:58 AM, Guenter Roeck wrote:
> > On Thu, Jun 06, 2019 at 07:46:17PM -0500, Gustavo A. R. Silva wrote:
> >> Now that all the fall-through warnings have been addressed in the
> >> kernel, enable the fall-through warning globally.
> >>
> >
> > Not really "all".
> >
> > powerpc:85xx/sbc8548_defconfig:
> >
> > arch/powerpc/kernel/align.c: In function ‘emulate_spe’:
> > arch/powerpc/kernel/align.c:178:8: error: this statement may fall through
> >
> > Plus many more similar errors in the same file.
> >
> > All sh builds:
> >
> > arch/sh/kernel/disassemble.c: In function 'print_sh_insn':
> > arch/sh/kernel/disassemble.c:478:8: error: this statement may fall through
> >
> > Again, this is seen in several places.
> >
> > mips:cavium_octeon_defconfig:
> >
> > arch/mips/cavium-octeon/octeon-usb.c: In function 'dwc3_octeon_clocks_start':
> > include/linux/device.h:1499:2: error: this statement may fall through
> >
> > None of those are from recent changes. And this is just from my small
> > subset of test builds.
> >
>
> Thank you for letting me know about this. I don't have access to build
> infrastructure like yours.
>
> My build infrastructure is similar to that of Linus.
>
> But if you send me all of those I can create a patch and send it back
> to you to make sure what you see is addressed. If we can coordinate for
> this it'd be great for everybody. :)
>

More to be found in
https://lore.kernel.org/lkml/20190729081727.6094-1-geert@linux-m68k.org/

and I saw the following in my local builds (not detected above due to
kisskb using an older compiler for m68k builds):

arch/m68k/include/asm/amigahw.h: warning: this statement may fall
through [-Wimplicit-fallthrough=]:  => 42:50
drivers/block/ataflop.c: warning: this statement may fall through
[-Wimplicit-fallthrough=]:  => 1728:3
drivers/net/arcnet/com20020-isa.c: warning: this statement may fall
through [-Wimplicit-fallthrough=]:  => 205:13, 203:10, 209:7, 201:11,
207:8
drivers/scsi/sun3_scsi.c: warning: this statement may fall through
[-Wimplicit-fallthrough=]:  => 399:9, 403:9
sound/oss/dmasound/dmasound_atari.c: warning: this statement may fall
through [-Wimplicit-fallthrough=]:  => 1449:24

Thanks for fixing ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Makefile: Globally enable fall-through warning

[Gustavo A. R. Silva]

Hi Geert,

On 7/29/19 3:22 AM, Geert Uytterhoeven wrote:

> 
> More to be found in
> https://lore.kernel.org/lkml/20190729081727.6094-1-geert@linux-m68k.org/
> 

I already sent out patches for many of those. :)

Thanks a lot!

> and I saw the following in my local builds (not detected above due to
> kisskb using an older compiler for m68k builds):
> 
> arch/m68k/include/asm/amigahw.h: warning: this statement may fall
> through [-Wimplicit-fallthrough=]:  => 42:50
> drivers/block/ataflop.c: warning: this statement may fall through
> [-Wimplicit-fallthrough=]:  => 1728:3
> drivers/net/arcnet/com20020-isa.c: warning: this statement may fall
> through [-Wimplicit-fallthrough=]:  => 205:13, 203:10, 209:7, 201:11,
> 207:8
> drivers/scsi/sun3_scsi.c: warning: this statement may fall through
> [-Wimplicit-fallthrough=]:  => 399:9, 403:9
> sound/oss/dmasound/dmasound_atari.c: warning: this statement may fall
> through [-Wimplicit-fallthrough=]:  => 1449:24
> 

I already sent patches for some of the ones above, too. Although, I have
some doubts with the one at arch/m68k/amiga/config.c:223

We can talk about it in a separate thread.

> Thanks for fixing ;-)
> 

Glad to help. :)

Thanks
--
Gustavo