2019-06-07 16:14:10

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS

This is a GCC only option, which warns about ABI changes within GCC, so
unconditionally adding 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

I suspect the link time failure comes from some flags not being added
via cc-option, which will always fail when an unknown flag is
unconditionally added to KBUILD_CFLAGS because -Werror is added after
commit c3f0d0bc5b01 ("kbuild, LLVMLinux: Add -Werror to cc-option to
support clang").

$ echo "int main() { return 0; }" | clang -Wno-psabi -o /dev/null -x c -
warning: unknown warning option '-Wno-psabi' [-Wunknown-warning-option]
1 warning generated.

$ echo $?
0

$ echo "int main() { return 0; }" | clang -Werror -Wno-psabi -o /dev/null -x c -
error: unknown warning option '-Wno-psabi' [-Werror,-Wunknown-warning-option]

$ echo $?
1

This side effect is user visible (aside from the inordinate amount of
-Wunknown-warning-option and build failure), as some warnings that are
normally disabled like -Waddress-of-packed-member or
-Wunused-const-variable show up.

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 <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---
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)
--
2.22.0.rc3


2019-06-07 16:25:45

by Dave Martin

[permalink] [raw]
Subject: Re: [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS

On Fri, Jun 07, 2019 at 09:12:01AM -0700, Nathan Chancellor wrote:
> This is a GCC only option, which warns about ABI changes within GCC, so
> unconditionally adding 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
>
> I suspect the link time failure comes from some flags not being added
> via cc-option, which will always fail when an unknown flag is
> unconditionally added to KBUILD_CFLAGS because -Werror is added after
> commit c3f0d0bc5b01 ("kbuild, LLVMLinux: Add -Werror to cc-option to
> support clang").
>
> $ echo "int main() { return 0; }" | clang -Wno-psabi -o /dev/null -x c -
> warning: unknown warning option '-Wno-psabi' [-Wunknown-warning-option]
> 1 warning generated.
>
> $ echo $?
> 0
>
> $ echo "int main() { return 0; }" | clang -Werror -Wno-psabi -o /dev/null -x c -
> error: unknown warning option '-Wno-psabi' [-Werror,-Wunknown-warning-option]
>
> $ echo $?
> 1
>
> This side effect is user visible (aside from the inordinate amount of
> -Wunknown-warning-option and build failure), as some warnings that are
> normally disabled like -Waddress-of-packed-member or
> -Wunused-const-variable show up.
>
> 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 <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>

FWIW,
Acked-by: Dave Martin <[email protected]>

Cheers
---Dave

> ---
> 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)
> --
> 2.22.0.rc3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2019-06-07 16:30:17

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS

On Fri, Jun 7, 2019 at 9:12 AM Nathan Chancellor
<[email protected]> wrote:
> Use cc-disable-warning so that it gets disabled for GCC and does nothing
> for Clang.

Thanks for the quick fix.
Reviewed-by: Nick Desaulniers <[email protected]>
--
Thanks,
~Nick Desaulniers

2019-06-11 17:23:29

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS

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 <[email protected]>
Acked-by: Dave Martin <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Nathan Chancellor <[email protected]>
---

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)
--
2.22.0

2019-06-12 09:33:11

by Dave Martin

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS

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 <[email protected]>
> Acked-by: Dave Martin <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Signed-off-by: Nathan Chancellor <[email protected]>
> ---
>
> 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

2019-06-12 09:37:29

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS

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