linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS
@ 2019-06-07 16:12 Nathan Chancellor
  2019-06-07 16:24 ` Dave Martin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Nathan Chancellor @ 2019-06-07 16:12 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Qian Cai,
	Nathan Chancellor, Dave Martin, linux-arm-kernel

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 <cai@lca.pw>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 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
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS
  2019-06-07 16:12 [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS Nathan Chancellor
@ 2019-06-07 16:24 ` Dave Martin
  2019-06-07 16:27 ` Nick Desaulniers
  2019-06-11 17:19 ` [PATCH v2] " Nathan Chancellor
  2 siblings, 0 replies; 6+ messages in thread
From: Dave Martin @ 2019-06-07 16:24 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Catalin Marinas, Nick Desaulniers, linux-kernel, Will Deacon,
	clang-built-linux, Qian Cai, linux-arm-kernel

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 <cai@lca.pw>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

FWIW,
Acked-by: Dave Martin <Dave.Martin@arm.com>

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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS
  2019-06-07 16:12 [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS Nathan Chancellor
  2019-06-07 16:24 ` Dave Martin
@ 2019-06-07 16:27 ` Nick Desaulniers
  2019-06-11 17:19 ` [PATCH v2] " Nathan Chancellor
  2 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2019-06-07 16:27 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Catalin Marinas, Will Deacon, LKML, clang-built-linux, Qian Cai,
	Dave Martin, Linux ARM

On Fri, Jun 7, 2019 at 9:12 AM Nathan Chancellor
<natechancellor@gmail.com> 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 <ndesaulniers@google.com>
-- 
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS
  2019-06-07 16:12 [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS Nathan Chancellor
  2019-06-07 16:24 ` Dave Martin
  2019-06-07 16:27 ` Nick Desaulniers
@ 2019-06-11 17:19 ` Nathan Chancellor
  2019-06-12  9:25   ` Dave Martin
  2 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2019-06-11 17:19 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon
  Cc: Nick Desaulniers, linux-kernel, clang-built-linux, Qian Cai,
	Nathan Chancellor, Dave Martin, linux-arm-kernel

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS
  2019-06-11 17:19 ` [PATCH v2] " Nathan Chancellor
@ 2019-06-12  9:25   ` Dave Martin
  2019-06-12  9:33     ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Dave Martin @ 2019-06-12  9:25 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Catalin Marinas, Nick Desaulniers, linux-kernel, Will Deacon,
	clang-built-linux, Qian Cai, linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS
  2019-06-12  9:25   ` Dave Martin
@ 2019-06-12  9:33     ` Will Deacon
  0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2019-06-12  9:33 UTC (permalink / raw)
  To: Dave Martin
  Cc: Catalin Marinas, Nick Desaulniers, linux-kernel,
	clang-built-linux, Qian Cai, Nathan Chancellor, linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-12  9:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07 16:12 [PATCH] arm64: Don't unconditionally add -Wno-psabi to KBUILD_CFLAGS Nathan Chancellor
2019-06-07 16:24 ` Dave Martin
2019-06-07 16:27 ` Nick Desaulniers
2019-06-11 17:19 ` [PATCH v2] " Nathan Chancellor
2019-06-12  9:25   ` Dave Martin
2019-06-12  9:33     ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).