| Message ID | 20190611171931.99705-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Commit | fa63da2ab046b885a7f70291aafc4e8ce015429b |
| Headers | show |
| Series | [v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS | expand |
On Tue, Jun 11, 2019 at 10:19:32AM -0700, Nathan Chancellor wrote: > This is a GCC only option, which warns about ABI changes within GCC, so > unconditionally adding it breaks Clang with tons of: > > warning: unknown warning option '-Wno-psabi' [-Wunknown-warning-option] > > and link time failures: > > ld.lld: error: undefined symbol: __efistub___stack_chk_guard > >>> referenced by arm-stub.c:73 > (/home/nathan/cbl/linux/drivers/firmware/efi/libstub/arm-stub.c:73) > >>> arm-stub.stub.o:(__efistub_install_memreserve_table) > in archive ./drivers/firmware/efi/libstub/lib.a > > These failures come from the lack of -fno-stack-protector, which is > added via cc-option in drivers/firmware/efi/libstub/Makefile. When an > unknown flag is added to KBUILD_CFLAGS, clang will noisily warn that it > is ignoring the option like above, unlike gcc, who will just error. > > $ echo "int main() { return 0; }" > tmp.c > > $ clang -Wno-psabi tmp.c; echo $? > warning: unknown warning option '-Wno-psabi' [-Wunknown-warning-option] > 1 warning generated. > 0 > > $ gcc -Wsometimes-uninitialized tmp.c; echo $? > gcc: error: unrecognized command line option > ‘-Wsometimes-uninitialized’; did you mean ‘-Wmaybe-uninitialized’? > 1 > > For cc-option to work properly with clang and behave like gcc, -Werror > is needed, which was done in commit c3f0d0bc5b01 ("kbuild, LLVMLinux: > Add -Werror to cc-option to support clang"). > > $ clang -Werror -Wno-psabi tmp.c; echo $? > error: unknown warning option '-Wno-psabi' > [-Werror,-Wunknown-warning-option] > 1 > > As a consequence of this, when an unknown flag is unconditionally added > to KBUILD_CFLAGS, it will cause cc-option to always fail and those flags > will never get added: > > $ clang -Werror -Wno-psabi -fno-stack-protector tmp.c; echo $? > error: unknown warning option '-Wno-psabi' > [-Werror,-Wunknown-warning-option] > 1 > > This can be seen when compiling the whole kernel as some warnings that > are normally disabled (see below) show up. The full list of flags > missing from drivers/firmware/efi/libstub are the following (gathered > from diffing .arm64-stub.o.cmd): > > -fno-delete-null-pointer-checks > -Wno-address-of-packed-member > -Wframe-larger-than=2048 > -Wno-unused-const-variable > -fno-strict-overflow > -fno-merge-all-constants > -fno-stack-check > -Werror=date-time > -Werror=incompatible-pointer-types > -ffreestanding > -fno-stack-protector > > Use cc-disable-warning so that it gets disabled for GCC and does nothing > for Clang. > > Fixes: ebcc5928c5d9 ("arm64: Silence gcc warnings about arch ABI drift") > Link: https://github.com/ClangBuiltLinux/linux/issues/511 > Reported-by: Qian Cai <cai@lca.pw> > Acked-by: Dave Martin <Dave.Martin@arm.com> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > * Improve commit message explanation, I wasn't entirely happy with the > first one; let me know if there are any issues/questions. > > * Carry forward Dave's ack and Nick's review (let me know if you > disagree with the commit messasge rewording). > > arch/arm64/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index 8fbd583b18e1..e9d2e578cbe6 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -51,7 +51,7 @@ endif > > KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) > KBUILD_CFLAGS += -fno-asynchronous-unwind-tables > -KBUILD_CFLAGS += -Wno-psabi > +KBUILD_CFLAGS += $(call cc-disable-warning, psabi) > KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) > > KBUILD_CFLAGS += $(call cc-option,-mabi=lp64) Looks OK to me. Thanks for the additional explanation. Cheers ---Dave
On Wed, Jun 12, 2019 at 10:25:20AM +0100, Dave Martin wrote: > On Tue, Jun 11, 2019 at 10:19:32AM -0700, Nathan Chancellor wrote: > > v1 -> v2: > > > > * Improve commit message explanation, I wasn't entirely happy with the > > first one; let me know if there are any issues/questions. > > > > * Carry forward Dave's ack and Nick's review (let me know if you > > disagree with the commit messasge rewording). > > > > arch/arm64/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > index 8fbd583b18e1..e9d2e578cbe6 100644 > > --- a/arch/arm64/Makefile > > +++ b/arch/arm64/Makefile > > @@ -51,7 +51,7 @@ endif > > > > KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) > > KBUILD_CFLAGS += -fno-asynchronous-unwind-tables > > -KBUILD_CFLAGS += -Wno-psabi > > +KBUILD_CFLAGS += $(call cc-disable-warning, psabi) > > KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) > > > > KBUILD_CFLAGS += $(call cc-option,-mabi=lp64) > > Looks OK to me. Thanks for the additional explanation. I'd already queued the previous version, but somehow forgotten to push it out. I'll push this one out instead later today. Cheers, Will
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index 8fbd583b18e1..e9d2e578cbe6 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -51,7 +51,7 @@ endif KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -KBUILD_CFLAGS += -Wno-psabi +KBUILD_CFLAGS += $(call cc-disable-warning, psabi) KBUILD_AFLAGS += $(lseinstr) $(brokengasinst) KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)