2017-09-26 02:28:55

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

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 <[email protected]>
---
* 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)
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.

--
2.11.0


2017-09-28 10:53:32

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

Hi Nick,

2017-09-26 11:28 GMT+09:00 Nick Desaulniers <[email protected]>:
> 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 <[email protected]>
> ---
> * 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.
>




--
Best Regards
Masahiro Yamada

2017-09-30 23:14:56

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

On Thu, Sep 28, 2017 at 07:52:35PM +0900, Masahiro Yamada wrote:
> 2017-09-26 11:28 GMT+09:00 Nick Desaulniers <[email protected]>:
> > 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.

2017-10-01 00:39:28

by Segher Boessenkool

[permalink] [raw]
Subject: Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

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 <[email protected]>:
> > > 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