| Message ID | 20171102212649.108880-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
El Thu, Nov 02, 2017 at 02:26:48PM -0700 Nick Desaulniers ha dit: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. > > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. > > We didn't notice this problem on Android, because we took the original > LLVMLinux patch into our 4.4 kernels, which did not have this issue. We > ran into this taking the proper upstream patch on newer kernel versions. > The original LLVMLinux patch can be seen at: > > http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 > > It seems that when the patch was re-upstreamed, a V2 was requested that > moved the definition of Clang's target triple to be later in the top > level Makefile than the inclusion of the arch specific Makefile, > breaking macros like ld-option when cross compiling. V2 was requested > at: > > https://lkml.org/lkml/2017/4/21/116 > > Signed-off-by: Chris Fries <cfries@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- > 1 file changed, 32 insertions(+), 32 deletions(-) > > diff --git a/Makefile b/Makefile > index 5f91a28a3cea..72ea86157114 100644 > --- a/Makefile > +++ b/Makefile > @@ -512,6 +512,38 @@ ifneq ($(filter install,$(MAKECMDGOALS)),) > endif > endif > > +ifeq ($(cc-name),clang) > +ifneq ($(CROSS_COMPILE),) > +CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > +endif > +ifneq ($(GCC_TOOLCHAIN),) > +CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > +endif > +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > +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) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +else > + > +# These warnings generated too much noise in a regular build. > +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > +endif > + > ifeq ($(mixed-targets),1) > # =========================================================================== > # We're called with mixed targets (*config and build targets). > @@ -695,38 +727,6 @@ ifdef CONFIG_CC_STACKPROTECTOR > endif > KBUILD_CFLAGS += $(stackp-flag) > > -ifeq ($(cc-name),clang) > -ifneq ($(CROSS_COMPILE),) > -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > -endif > -ifneq ($(GCC_TOOLCHAIN),) > -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > -endif > -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > -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) > -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > -else > - > -# These warnings generated too much noise in a regular build. > -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > -endif > - > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > else FWIW Tested-by: Matthias Kaehlcke <mka@chromium.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.org> On the Chrome OS side we missed this because by default our kernel builds use an architecture aware wrapper instead of bare clang, which masks this issue. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2017-11-03 6:26 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > From: Chris Fries <cfries@google.com> > > Set the clang KBUILD_CFLAGS up before including arch/ Makefiles, > so that ld-options (etc.) can work correctly. ld-option is only used for arch/{arm64,powerpc}/Makefile arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call ld-option,--orphan-handling=warn) I think this patch makes sense when it comes along with https://patchwork.kernel.org/patch/10030581/ but, it is now being blocked by 0-day bot due to a x86 problem. > This fixes errors with clang such as ld-options trying to CC > against your host architecture, but LD trying to link against > your target architecture. > > We didn't notice this problem on Android, because we took the original > LLVMLinux patch into our 4.4 kernels, which did not have this issue. We > ran into this taking the proper upstream patch on newer kernel versions. > The original LLVMLinux patch can be seen at: > > http://git.linuxfoundation.org/?p=llvmlinux/kernel.git;a=blobdiff;f=Makefile;h=389006c4ef494cda3a1ee52bf355618673ab4f31;hp=e41a3356abee83f08288362950bfceebd25ec3c2;hb=ef9126da11b18ff34eb1f01561f53c378860336c;hpb=f800c25b7a762d445ba1439a2428c8362157eba6 > > It seems that when the patch was re-upstreamed, a V2 was requested that > moved the definition of Clang's target triple to be later in the top > level Makefile than the inclusion of the arch specific Makefile, > breaking macros like ld-option when cross compiling. V2 was requested > at: But, ld-option is defines as follows in llvm-linux tree (and mainline too): ld-option = $(call try-run,\ $(CC) -x c /dev/null -c -o "$$TMPO" ; $(LD) $(1) "$$TMPO" -o "$$TMP",$(1),$(2)) ld-option does not depend on any pre-defined flags. The location of CLANG_GCC_TC define only matters after your patch is applied, right? Did my request for v2 break anything? One more thing: this patch does not apply to kbuild tree. > https://lkml.org/lkml/2017/4/21/116 > > Signed-off-by: Chris Fries <cfries@google.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Makefile | 64 ++++++++++++++++++++++++++++++++-------------------------------- > 1 file changed, 32 insertions(+), 32 deletions(-) > > diff --git a/Makefile b/Makefile > index 5f91a28a3cea..72ea86157114 100644 > --- a/Makefile > +++ b/Makefile > @@ -512,6 +512,38 @@ ifneq ($(filter install,$(MAKECMDGOALS)),) > endif > endif > > +ifeq ($(cc-name),clang) > +ifneq ($(CROSS_COMPILE),) > +CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > +endif > +ifneq ($(GCC_TOOLCHAIN),) > +CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > +endif > +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > +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) > +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > +else > + > +# These warnings generated too much noise in a regular build. > +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > +endif > + > ifeq ($(mixed-targets),1) > # =========================================================================== > # We're called with mixed targets (*config and build targets). > @@ -695,38 +727,6 @@ ifdef CONFIG_CC_STACKPROTECTOR > endif > KBUILD_CFLAGS += $(stackp-flag) > > -ifeq ($(cc-name),clang) > -ifneq ($(CROSS_COMPILE),) > -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) > -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) > -endif > -ifneq ($(GCC_TOOLCHAIN),) > -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) > -endif > -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) > -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > -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) > -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) > -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) > -else > - > -# These warnings generated too much noise in a regular build. > -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) > -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) > -endif > - > ifdef CONFIG_FRAME_POINTER > KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > else > -- > 2.15.0.403.gc27cc4dac6-goog > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Nov 4, 2017 at 8:06 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > ld-option is only used for arch/{arm64,powerpc}/Makefile > > arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) > arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call > ld-option,--orphan-handling=warn) > > I think this patch makes sense when it comes along with > https://patchwork.kernel.org/patch/10030581/ Good point. > but, it is now being blocked by 0-day bot > due to a x86 problem. Looks like that is now resolved (unless 0-day bot strikes again). > The location of CLANG_GCC_TC define > only matters after your patch is applied, right? By "your patch" referring to the 0-day bot thread, yes. > Did my request for v2 break anything? Nothing immediately obvious, and no regressions. It just made this patch necessary (along with my previous one) for correctly cross compiling with clang for arm64 and powerpc as you point out. > One more thing: this patch does not apply to kbuild tree. I absolutely will rebase it on your tree and send a v2. Just to help me understand the contribution model better: none of my other patches have yet been requested against any trees other than Linus'. Is this because of where we are in the release cycle, or that a lot of kbuild code has changed, or what? -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2017-11-08 2:37 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: > On Sat, Nov 4, 2017 at 8:06 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> ld-option is only used for arch/{arm64,powerpc}/Makefile >> >> arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) >> arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call >> ld-option,--orphan-handling=warn) >> >> I think this patch makes sense when it comes along with >> https://patchwork.kernel.org/patch/10030581/ > > Good point. > >> but, it is now being blocked by 0-day bot >> due to a x86 problem. > > Looks like that is now resolved (unless 0-day bot strikes again). > >> The location of CLANG_GCC_TC define >> only matters after your patch is applied, right? > > By "your patch" referring to the 0-day bot thread, yes. > >> Did my request for v2 break anything? > > Nothing immediately obvious, and no regressions. It just made this > patch necessary (along with my previous one) for correctly cross > compiling with clang for arm64 and powerpc as you point out. > >> One more thing: this patch does not apply to kbuild tree. > > I absolutely will rebase it on your tree and send a v2. Just to help > me understand the contribution model better: none of my other patches > have yet been requested against any trees other than Linus'. Is this > because of where we are in the release cycle, or that a lot of kbuild > code has changed, or what? Generally speaking, a preferred way is to base patches on the subsystem tree. Kernel developers are supposed to do their development on linux-next, but, in reality, many people work on Linus' tree since it is more stable and git history is fast-forward. In many cases, patches based on Linus' tree can apply to sub-systems as well. I am happy to fix-up a conflict locally as long as it is trivial, and there is no other reason for re-spin. Unfortunately, Kbuild tree changed the top-level Makefile a lot in this development cycle. If your patch does not apply cleanly, I do not know which context you are moving the code to. Also, I found suspicious description in the commit log. That's why.
On Wed, Nov 8, 2017 at 8:15 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2017-11-08 2:37 GMT+09:00 Nick Desaulniers <ndesaulniers@google.com>: >> On Sat, Nov 4, 2017 at 8:06 PM, Masahiro Yamada >> <yamada.masahiro@socionext.com> wrote: >>> ld-option is only used for arch/{arm64,powerpc}/Makefile >>> >>> arch/arm64/Makefile: ifeq ($(call ld-option, --fix-cortex-a53-843419),) >>> arch/powerpc/Makefile:LDFLAGS_vmlinux += $(call >>> ld-option,--orphan-handling=warn) >>> >>> I think this patch makes sense when it comes along with >>> https://patchwork.kernel.org/patch/10030581/ >> >> Good point. >> >>> but, it is now being blocked by 0-day bot >>> due to a x86 problem. >> >> Looks like that is now resolved (unless 0-day bot strikes again). >> >>> The location of CLANG_GCC_TC define >>> only matters after your patch is applied, right? >> >> By "your patch" referring to the 0-day bot thread, yes. >> >>> Did my request for v2 break anything? >> >> Nothing immediately obvious, and no regressions. It just made this >> patch necessary (along with my previous one) for correctly cross >> compiling with clang for arm64 and powerpc as you point out. >> >>> One more thing: this patch does not apply to kbuild tree. >> >> I absolutely will rebase it on your tree and send a v2. Just to help >> me understand the contribution model better: none of my other patches >> have yet been requested against any trees other than Linus'. Is this >> because of where we are in the release cycle, or that a lot of kbuild >> code has changed, or what? > > > Generally speaking, > a preferred way is to base patches on the subsystem tree. > > Kernel developers are supposed to do their development on linux-next, > but, in reality, many people work on Linus' tree since it is more stable and > git history is fast-forward. > > In many cases, patches based on Linus' tree can apply to sub-systems as well. > > I am happy to fix-up a conflict locally > as long as it is trivial, and there is no other reason for re-spin. > > Unfortunately, Kbuild tree changed the top-level Makefile a lot in > this development cycle. > > If your patch does not apply cleanly, I do not know which context you > are moving the code to. > Also, I found suspicious description in the commit log. > > That's why. > > > -- > Best Regards > Masahiro Yamada Great, thanks for taking time to explain that, I appreciate it.
diff --git a/Makefile b/Makefile index 5f91a28a3cea..72ea86157114 100644 --- a/Makefile +++ b/Makefile @@ -512,6 +512,38 @@ ifneq ($(filter install,$(MAKECMDGOALS)),) endif endif +ifeq ($(cc-name),clang) +ifneq ($(CROSS_COMPILE),) +CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) +GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) +endif +ifneq ($(GCC_TOOLCHAIN),) +CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) +endif +KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) +KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) +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) +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) +else + +# These warnings generated too much noise in a regular build. +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) +KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) +endif + ifeq ($(mixed-targets),1) # =========================================================================== # We're called with mixed targets (*config and build targets). @@ -695,38 +727,6 @@ ifdef CONFIG_CC_STACKPROTECTOR endif KBUILD_CFLAGS += $(stackp-flag) -ifeq ($(cc-name),clang) -ifneq ($(CROSS_COMPILE),) -CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) -GCC_TOOLCHAIN := $(realpath $(dir $(shell which $(LD)))/..) -endif -ifneq ($(GCC_TOOLCHAIN),) -CLANG_GCC_TC := --gcc-toolchain=$(GCC_TOOLCHAIN) -endif -KBUILD_CFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_AFLAGS += $(CLANG_TARGET) $(CLANG_GCC_TC) -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) -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) -KBUILD_CFLAGS += $(call cc-option, -no-integrated-as) -KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) -else - -# These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.extrawarn) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) -KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) -endif - ifdef CONFIG_FRAME_POINTER KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls else