linux-kernel - Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 19 Oct 2018 01:39:21 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Leonardo Brás <leobras.c@...il.com>,
        lkcamp@...ts.libreplanetbr.org,
        Matthew Wilcox <willy6545@...il.com>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH 1/4] Adds -Wshadow=local on KBUILD_HOSTCFLAGS

On Thu, Oct 18, 2018 at 6:18 PM Borislav Petkov <bp@...en8.de> wrote:
>
> On Wed, Oct 17, 2018 at 09:40:53PM -0300, Leonardo Bras wrote:
> > The idea was to put it as default and fix all the shadowing warnings.
> > What do you think?  I am open to suggestions.
>
> That's Masahiro's call. In the rest of the kernel, those warnings are behind
> the W=2 switch - i.e., not enabled by default.


It is not realistic to enable this warning option by default.
Even -Wshadow=local emits tons of warnings.
(More with -Wshadow)

The problem of this flag is,
it is false positive in macro expansions.


For example, I think the following is a legitimate case.



In file included from ./arch/arm64/include/asm/cputype.h:126:0,
                 from ./arch/arm64/include/asm/cache.h:19,
                 from ./include/linux/cache.h:6,
                 from ./include/linux/printk.h:9,
                 from ./include/linux/kernel.h:14,
                 from ./include/linux/bitmap.h:10,
                 from arch/arm64/kernel/fpsimd.c:20:
arch/arm64/kernel/fpsimd.c: In function ‘sve_kernel_enable’:
./arch/arm64/include/asm/sysreg.h:707:6: warning: declaration of
‘__val’ shadows a previous local [-Wshadow=compatible-local]
  u64 __val;      \
      ^
./arch/arm64/include/asm/sysreg.h:717:20: note: in definition of macro
‘write_sysreg’
  u64 __val = (u64)(v);     \
                    ^
arch/arm64/kernel/fpsimd.c:713:15: note: in expansion of macro ‘read_sysreg’
  write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1);
               ^~~~~~~~~~~
./arch/arm64/include/asm/sysreg.h:717:6: note: shadowed declaration is here
  u64 __val = (u64)(v);     \
      ^
arch/arm64/kernel/fpsimd.c:713:2: note: in expansion of macro ‘write_sysreg’
  write_sysreg(read_sysreg(CPACR_EL1) | CPACR_EL1_ZEN_EL1EN, CPACR_EL1);
  ^~~~~~~~~~~~




--
Best Regards
Masahiro Yamada