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:   Mon, 1 Oct 2018 15:36:41 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Kees Cook <keescook@...omium.org>,
        Michal Marek <michal.lkml@...kovi.net>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] kbuild: add -Wno-unused-but-set-variable compiler
 flag unconditionally

On Mon, Oct 1, 2018 at 3:24 PM Masahiro Yamada
<yamada.masahiro@...ionext.com> wrote:
>
> 2018年10月2日(火) 4:58 Nick Desaulniers <ndesaulniers@...gle.com>:
> >
> > On Mon, Oct 1, 2018 at 12:02 PM Masahiro Yamada
> > <yamada.masahiro@...ionext.com> wrote:
> > >
> > > Hi Nick,
> > >
> > > 2018年10月2日(火) 2:18 Nick Desaulniers <ndesaulniers@...gle.com>:
> > > >
> > > > On Mon, Oct 1, 2018 at 2:45 AM Masahiro Yamada
> > > > <yamada.masahiro@...ionext.com> wrote:
> > > > >
> > > > > We have raised the compiler requirement from time to time.
> > > > > With commit cafa0010cd51 ("Raise the minimum required gcc version
> > > > > to 4.6"), the minimum for GCC is 4.6 now.
> > > > >
> > > > > This flag was added by GCC 4.6, and it is recognized by Clang and
> > > > > ICC as well.
> > > >
> > > > This doesn't seem to be the case for Clang:
> > > > https://godbolt.org/z/qesF5o
> > > >
> > > > Nacked-by: Nick Desaulniers <ndesaulniers@...gle.com>
> > >
> > > Hmm, I tested this patch with pre-built Clang 6.0.1 / 7.0.0
> > > downloaded from http://releases.llvm.org/download.html
> > >
> > > Was this option dropped by clang 8 ?
> >
> > From the Godbolt link above, it seems all versions of Clang do not
> > recognize this flag.  It doesn't look like the kernel sets
> > -Wno-unknown-warning-option to prevent this warning.  Can you please
> > triple check that compiling with Clang and this patch causes no
> > warnings?  I suspect something might be amiss then if this patch
> > doesn't produce warnings with Clang, since on the smaller Godbolt
> > example it does.
>
>
> I got it.
>
> Clang does NOT recognize -Wno-unused-but-set-variable.
>
> But, the code I changed is not parsed for clang.
> That is why I did not see any problem with this patch.
>
> See the following code.
>
>
>
> ifeq ($(cc-name),clang)
> KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,)
> KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier)
> KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
> KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> # Quiet clang warning: comparison of unsigned expression < 0 is always false
> KBUILD_CFLAGS += $(call cc-disable-warning, tautological-compare)
> # CLANG uses a _MergedGlobals as optimization, but this breaks modpost, as the
> # source of a reference will be _MergedGlobals and not on of the
> whitelisted names.
> # See modpost pattern 2
> KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
> KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> else
>
> # These warnings generated too much noise in a regular build.
> # Use make W=1 to enable them (see scripts/Makefile.extrawarn)
> KBUILD_CFLAGS += -Wno-unused-but-set-variable

Ah! I see, it's in the else part of an if clang check.  Ok, in that
case I think the commit message can be cleaned up with this detail,
then I'd be happy to sign off on it.  Sorry I didn't notice that
before.

> endif
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Thanks,
~Nick Desaulniers

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ