[PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Nick Desaulniers]

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

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Masahiro Yamada]

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.
>




-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Nick Desaulniers]

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.

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Segher Boessenkool]

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

Re: [PATCH] kbuild: clang: remove crufty HOSTCFLAGS

[Masahiro Yamada]

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.





-- 
Best Regards
Masahiro Yamada

[PATCH v2] kbuild: clang: remove crufty HOSTCFLAGS

[Nick Desaulniers]

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").

Clang does not support -fno-delete-null-pointer-checks, so adding it to
HOSTCFLAGS if HOSTCC is clang does not make sense.

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.

Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
---
 Makefile | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/Makefile b/Makefile
index cf007a31d575..5853d9492590 100644
--- a/Makefile
+++ b/Makefile
@@ -306,11 +306,6 @@ 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

Re: [PATCH v2] kbuild: clang: remove crufty HOSTCFLAGS

[Nick Desaulniers]

Sorry, I forgot to note the changes in v2:

* Dont add back -fno-delete-null-pointer-checks wrapped in
  hostcc-option, just remove all flags.
* Update wording of commit message.

Re: [PATCH v2] kbuild: clang: remove crufty HOSTCFLAGS

[Masahiro Yamada]

2017-10-08 5:23 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").
>
> Clang does not support -fno-delete-null-pointer-checks, so adding it to
> HOSTCFLAGS if HOSTCC is clang does not make sense.
>
> 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.
>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com>
> ---
>  Makefile | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index cf007a31d575..5853d9492590 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -306,11 +306,6 @@ 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

Applied to linux-kbuild/kbuild.  Thanks.



-- 
Best Regards
Masahiro Yamada