| Message ID | 20170926022835.30916-1-nick.desaulniers@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Nick, 2017-09-26 11:28 GMT+09:00 Nick Desaulniers <nick.desaulniers@gmail.com>: > When compiling with `make CC=clang HOSTCC=clang`, I was seeing warnings > that clang did not recognize -fno-delete-null-pointer-checks for HOSTCC > targets. These were added in commit 61163efae020 ("kbuild: LLVMLinux: > Add Kbuild support for building kernel with Clang"). That patch wraps > that flag in cc-option for KBUILD_CFLAGS, but not hostcc-option for > HOSTCFLAGS. Either hostcc-option did not exist, or the author was not > setting HOSTCC to clang as well as CC when authored. > > It's not clear why the other warnings were disabled, and just for > HOSTCFLAGS, but I can remove them, add -Werror to HOSTCFLAGS and compile > with clang just fine. > > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > --- > * It may also be worthwhile keep the old flags, and simply wrap > everything in hostcc-option. > > Makefile | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/Makefile b/Makefile > index d1119941261c..2e908969e0d8 100644 > --- a/Makefile > +++ b/Makefile > @@ -301,16 +301,12 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS) > HOSTCC = gcc > HOSTCXX = g++ > HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > + $(call hostcc-option,-fno-delete-null-pointer-checks) \ > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) You call hostcc-option before Kbuild.include is included around line 341. So, $(call hostcc-option, ...) returns always an empty string here whether the compiler supports the option or not. > HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) > HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) > HOST_LOADLIBES := $(HOST_LFS_LIBS) > > -ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1) > -HOSTCFLAGS += -Wno-unused-value -Wno-unused-parameter \ > - -Wno-missing-field-initializers -fno-delete-null-pointer-checks > -endif > - The logic is very strange in the first place. Even very old GCC supports -fno-delete-null-pointer-checks, but clang does not. Here, -fno-delete-null-pointer-checks is added only when we are using clang for HOSTCC. This is opposite. I guess we can remove all of them unless somebody can explain the rationale. > # Decide whether to build built-in, modular, or both. > # Normally, just do built-in. >
On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote: > 2017-09-26 11:28 GMT+09:00 Nick Desaulniers <nick.desaulniers@gmail.com>: > > HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > + $(call hostcc-option,-fno-delete-null-pointer-checks) \ > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) > > You call hostcc-option > before Kbuild.include is included around line 341. > > So, $(call hostcc-option, ...) returns always an empty string here > whether the compiler supports the option or not. So calling a yet-to-be defined variable results in an empty string rather than a loud failure? Chalk that up there with language features no one ever asked for. That kind of implicit conversion gets languages like JavaScript (with its loose type system, not that C is without its own implicit type conversions/promotions) in a lot of hot water. If that's the case, why are includes not at the top of Makefiles, if silent failure is a possibility? Is there a reason the include is so far into the Makefile? Is your sugguestion to raise the include or lower the HOSTCFLAGS definition? > > -ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1) > > -HOSTCFLAGS += -Wno-unused-value -Wno-unused-parameter \ > > - -Wno-missing-field-initializers -fno-delete-null-pointer-checks > > -endif > > The logic is very strange in the first place. > > Even very old GCC supports -fno-delete-null-pointer-checks, > but clang does not. > > Here, -fno-delete-null-pointer-checks is added only when > we are using clang for HOSTCC. This is opposite. > > I guess we can remove all of them > unless somebody can explain the rationale. +llvm-linux I suppose maybe different ARCH's have different host binaries made during the build? I tested x86_64 and arm64. The commit message that added them missed any context or justification. -- 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, Sep 30, 2017 at 04:14:50PM -0700, Nick Desaulniers wrote: > On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote: > > 2017-09-26 11:28 GMT+09:00 Nick Desaulniers <nick.desaulniers@gmail.com>: > > > HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ > > > + $(call hostcc-option,-fno-delete-null-pointer-checks) \ > > > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) > > > > You call hostcc-option > > before Kbuild.include is included around line 341. > > > > So, $(call hostcc-option, ...) returns always an empty string here > > whether the compiler supports the option or not. > > So calling a yet-to-be defined variable results in an empty string > rather than a loud failure? Chalk that up there with language features > no one ever asked for. That kind of implicit conversion gets languages > like JavaScript (with its loose type system, not that C is without its > own implicit type conversions/promotions) in a lot of hot water. make --warn-undefined-variables (and it warns all over the place during a kernel build -- having undefined variables expand to the empty string is a useful feature, too, not just a trap for the unwary). Segher -- 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
Hi Nick. 2017-10-01 8:14 GMT+09:00 Nick Desaulniers <nick.desaulniers@gmail.com>: > On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote: >> 2017-09-26 11:28 GMT+09:00 Nick Desaulniers <nick.desaulniers@gmail.com>: >> > HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ >> > + $(call hostcc-option,-fno-delete-null-pointer-checks) \ >> > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) >> >> You call hostcc-option >> before Kbuild.include is included around line 341. >> >> So, $(call hostcc-option, ...) returns always an empty string here >> whether the compiler supports the option or not. > > So calling a yet-to-be defined variable results in an empty string > rather than a loud failure? Chalk that up there with language features > no one ever asked for. That kind of implicit conversion gets languages > like JavaScript (with its loose type system, not that C is without its > own implicit type conversions/promotions) in a lot of hot water. > > If that's the case, why are includes not at the top of Makefiles, if > silent failure is a possibility? Is there a reason the include is so > far into the Makefile? Kbuild.include depends on some other variables. You can not include it at the top of the Makefile. > Is your sugguestion to raise the include or lower the HOSTCFLAGS > definition? In this case, you do not need to move any of them. -fno-delete-null-pointer-checks has never enabled for host-tools before. Just remove it to keep the current behavior. >> > -ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1) >> > -HOSTCFLAGS += -Wno-unused-value -Wno-unused-parameter \ >> > - -Wno-missing-field-initializers -fno-delete-null-pointer-checks >> > -endif >> >> The logic is very strange in the first place. >> >> Even very old GCC supports -fno-delete-null-pointer-checks, >> but clang does not. >> >> Here, -fno-delete-null-pointer-checks is added only when >> we are using clang for HOSTCC. This is opposite. >> >> I guess we can remove all of them >> unless somebody can explain the rationale. > > +llvm-linux > > I suppose maybe different ARCH's have different host binaries made > during the build? I tested x86_64 and arm64. The commit message that > added them missed any context or justification. According to http://llvm.linuxfoundation.org/index.php/Main_Page llvm-linux was only successful for x86, arm(64) at that time. If you tested x86_64 and arm64, and saw no problem, it is fine.
diff --git a/Makefile b/Makefile index d1119941261c..2e908969e0d8 100644 --- a/Makefile +++ b/Makefile @@ -301,16 +301,12 @@ HOST_LFS_LIBS := $(shell getconf LFS_LIBS) HOSTCC = gcc HOSTCXX = g++ HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 \ + $(call hostcc-option,-fno-delete-null-pointer-checks) \ -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) HOST_LOADLIBES := $(HOST_LFS_LIBS) -ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1) -HOSTCFLAGS += -Wno-unused-value -Wno-unused-parameter \ - -Wno-missing-field-initializers -fno-delete-null-pointer-checks -endif - # Decide whether to build built-in, modular, or both. # Normally, just do built-in.
When compiling with `make CC=clang HOSTCC=clang`, I was seeing warnings that clang did not recognize -fno-delete-null-pointer-checks for HOSTCC targets. These were added in commit 61163efae020 ("kbuild: LLVMLinux: Add Kbuild support for building kernel with Clang"). That patch wraps that flag in cc-option for KBUILD_CFLAGS, but not hostcc-option for HOSTCFLAGS. Either hostcc-option did not exist, or the author was not setting HOSTCC to clang as well as CC when authored. It's not clear why the other warnings were disabled, and just for HOSTCFLAGS, but I can remove them, add -Werror to HOSTCFLAGS and compile with clang just fine. Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- * It may also be worthwhile keep the old flags, and simply wrap everything in hostcc-option. Makefile | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)