diff mbox series

[v2,4/4] objtool: Alter how HOSTCC is forced

Message ID 20221122001125.765003-5-irogers@google.com
State New
Headers
Series objtool build improvements |

Commit Message

Ian Rogers Nov. 22, 2022, 12:11 a.m. UTC 
  HOSTCC is always wanted when building objtool. Setting CC to HOSTCC
happens after tools/scripts/Makefile.include is included, meaning
flags are set assuming say CC is gcc, but then it can be later set to
HOSTCC which may be clang. tools/scripts/Makefile.include is needed
for host set up and common macros in objtool's Makefile. Rather than
override CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the
libsubcmd builds and the linkage step. This means the Makefiles don't
see things like CC changing and tool flag determination, and similar,
work properly. To avoid mixing CFLAGS from different compilers just
the objtool CFLAGS are determined. HOSTCFLAGS is added to these so
that command line flags can add to the CFLAGS.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/objtool/Makefile | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

Comments

Nick Desaulniers Dec. 9, 2022, 6:26 p.m. UTC | #1 
On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <irogers@google.com> wrote:
>
> HOSTCC is always wanted when building objtool. Setting CC to HOSTCC
> happens after tools/scripts/Makefile.include is included, meaning
> flags are set assuming say CC is gcc, but then it can be later set to
> HOSTCC which may be clang. tools/scripts/Makefile.include is needed
> for host set up and common macros in objtool's Makefile. Rather than
> override CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the
> libsubcmd builds and the linkage step. This means the Makefiles don't
> see things like CC changing and tool flag determination, and similar,
> work properly. To avoid mixing CFLAGS from different compilers just
> the objtool CFLAGS are determined. HOSTCFLAGS is added to these so
> that command line flags can add to the CFLAGS.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/objtool/Makefile | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index 61a00b7acae9..e550a98e2dd9 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -2,16 +2,12 @@
>  include ../scripts/Makefile.include
>  include ../scripts/Makefile.arch
>
> -# always use the host compiler
> -AR      = $(HOSTAR)
> -CC      = $(HOSTCC)
> -LD      = $(HOSTLD)
> -
>  ifeq ($(srctree),)
>  srctree := $(patsubst %/,%,$(dir $(CURDIR)))
>  srctree := $(patsubst %/,%,$(dir $(srctree)))
>  endif
>
> +MAKE = make -S
>  LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
>  ifneq ($(OUTPUT),)
>    LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
> @@ -36,13 +32,20 @@ INCLUDES := -I$(srctree)/tools/include \
>             -I$(srctree)/tools/objtool/include \
>             -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include \
>             -I$(LIBSUBCMD_OUTPUT)/include
> -WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs

Looking closer at the V=1 diff in meld, I think this is dropping
EXTRA_WARNINGS.  I think you want to add those back to OBJTOOL_CFLAGS.

> -CFLAGS   := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS)
> -LDFLAGS  += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
> +WARNINGS := -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
> +OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS)
> +OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD)
>
>  # Allow old libelf to be used:
>  elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
> -CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> +OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> +
> +# Always want host compilation.
> +HOST_OVERRIDES := CC="$(HOSTCC)" EXTRA_CFLAGS="$(OBJTOOL_CFLAGS)" \
> +                 LD="$(HOSTLD)" AR="$(HOSTAR)"
> +BUILD_HOST_OVERRIDES := CC="$(HOSTCC)" CFLAGS="$(OBJTOOL_CFLAGS)" \
> +                       LD="$(HOSTLD)" LDFLAGS="$(OBJTOOL_LDFLAGS)" \
> +                       AR="$(HOSTAR)"
>
>  AWK = awk
>  MKDIR = mkdir
> @@ -65,10 +68,11 @@ include $(srctree)/tools/build/Makefile.include
>
>  $(OBJTOOL_IN): fixdep FORCE
>         $(Q)$(CONFIG_SHELL) ./sync-check.sh
> -       $(Q)$(MAKE) $(build)=objtool
> +       $(Q)$(MAKE) $(build)=objtool $(BUILD_HOST_OVERRIDES)
> +
>
>  $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
> -       $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
> +       $(QUIET_LINK)$(HOSTCC) $(OBJTOOL_IN) $(KBUILD_HOSTLDFLAGS) $(OBJTOOL_LDFLAGS) -o $@
>
>
>  $(LIBSUBCMD_OUTPUT):
> @@ -77,6 +81,7 @@ $(LIBSUBCMD_OUTPUT):
>  $(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
>         $(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
>                 DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
> +               $(HOST_OVERRIDES) \
>                 $@ install_headers
>
>  $(LIBSUBCMD)-clean:
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
Ian Rogers Dec. 14, 2022, 6:25 p.m. UTC | #2 
On Fri, Dec 9, 2022 at 10:26 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <irogers@google.com> wrote:
> >
> > HOSTCC is always wanted when building objtool. Setting CC to HOSTCC
> > happens after tools/scripts/Makefile.include is included, meaning
> > flags are set assuming say CC is gcc, but then it can be later set to
> > HOSTCC which may be clang. tools/scripts/Makefile.include is needed
> > for host set up and common macros in objtool's Makefile. Rather than
> > override CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the
> > libsubcmd builds and the linkage step. This means the Makefiles don't
> > see things like CC changing and tool flag determination, and similar,
> > work properly. To avoid mixing CFLAGS from different compilers just
> > the objtool CFLAGS are determined. HOSTCFLAGS is added to these so
> > that command line flags can add to the CFLAGS.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/objtool/Makefile | 27 ++++++++++++++++-----------
> >  1 file changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > index 61a00b7acae9..e550a98e2dd9 100644
> > --- a/tools/objtool/Makefile
> > +++ b/tools/objtool/Makefile
> > @@ -2,16 +2,12 @@
> >  include ../scripts/Makefile.include
> >  include ../scripts/Makefile.arch
> >
> > -# always use the host compiler
> > -AR      = $(HOSTAR)
> > -CC      = $(HOSTCC)
> > -LD      = $(HOSTLD)
> > -
> >  ifeq ($(srctree),)
> >  srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> >  srctree := $(patsubst %/,%,$(dir $(srctree)))
> >  endif
> >
> > +MAKE = make -S
> >  LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
> >  ifneq ($(OUTPUT),)
> >    LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
> > @@ -36,13 +32,20 @@ INCLUDES := -I$(srctree)/tools/include \
> >             -I$(srctree)/tools/objtool/include \
> >             -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include \
> >             -I$(LIBSUBCMD_OUTPUT)/include
> > -WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
>
> Looking closer at the V=1 diff in meld, I think this is dropping
> EXTRA_WARNINGS.  I think you want to add those back to OBJTOOL_CFLAGS.

Thanks Nick! When I try this it causes new build failures.
Specifically EXTRA_WARNINGS includes things like -Wswitch-enum that
trigger:

```
check.c:1312:10: error: 14 enumeration values not explicitly handled
in switch: 'INSN_JUMP_DYNAMIC',
'INSN_JUMP_DYNAMIC_CONDITIONAL', 'INSN_CALL_DYNAMIC'... [-Werror,-Wswitch-enum]
       switch (insn->type) {
               ^~~~~~~~~~
check.c:2620:11: error: enumeration value 'OP_SRC_CONST' not
explicitly handled in switch [-Werror,-
Wswitch-enum]
               switch (op->src.type) {
                       ^~~~~~~~~~~~
check.c:3447:11: error: 5 enumeration values not explicitly handled in
switch: 'INSN_BUG', 'INSN_NOP
', 'INSN_TRAP'... [-Werror,-Wswitch-enum]
               switch (insn->type) {
                       ^~~~~~~~~~
check.c:3647:11: error: 9 enumeration values not explicitly handled in
switch: 'INSN_CONTEXT_SWITCH'
, 'INSN_BUG', 'INSN_STAC'... [-Werror,-Wswitch-enum]
               switch (insn->type) {
                       ^~~~~~~~~~
check.c:4008:10: error: 9 enumeration values not explicitly handled in
switch: 'INSN_CONTEXT_SWITCH'
, 'INSN_BUG', 'INSN_STAC'... [-Werror,-Wswitch-enum]
       switch (insn->type) {
               ^~~~~~~~~~
check.c:4173:11: error: 15 enumeration values not explicitly handled
in switch: 'INSN_JUMP_CONDITION
AL', 'INSN_JUMP_UNCONDITIONAL', 'INSN_JUMP_DYNAMIC_CONDITIONAL'...
[-Werror,-Wswitch-enum]
               switch (insn->type) {
                       ^~~~~~~~~~
6 errors generated.
```

What should the next step be? Land this or add EXTRA_WARNINGS and fix
all the issues?

Thanks,
Ian

> > -CFLAGS   := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS)
> > -LDFLAGS  += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
> > +WARNINGS := -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
> > +OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS)
> > +OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD)
> >
> >  # Allow old libelf to be used:
> >  elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
> > -CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > +OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > +
> > +# Always want host compilation.
> > +HOST_OVERRIDES := CC="$(HOSTCC)" EXTRA_CFLAGS="$(OBJTOOL_CFLAGS)" \
> > +                 LD="$(HOSTLD)" AR="$(HOSTAR)"
> > +BUILD_HOST_OVERRIDES := CC="$(HOSTCC)" CFLAGS="$(OBJTOOL_CFLAGS)" \
> > +                       LD="$(HOSTLD)" LDFLAGS="$(OBJTOOL_LDFLAGS)" \
> > +                       AR="$(HOSTAR)"
> >
> >  AWK = awk
> >  MKDIR = mkdir
> > @@ -65,10 +68,11 @@ include $(srctree)/tools/build/Makefile.include
> >
> >  $(OBJTOOL_IN): fixdep FORCE
> >         $(Q)$(CONFIG_SHELL) ./sync-check.sh
> > -       $(Q)$(MAKE) $(build)=objtool
> > +       $(Q)$(MAKE) $(build)=objtool $(BUILD_HOST_OVERRIDES)
> > +
> >
> >  $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
> > -       $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
> > +       $(QUIET_LINK)$(HOSTCC) $(OBJTOOL_IN) $(KBUILD_HOSTLDFLAGS) $(OBJTOOL_LDFLAGS) -o $@
> >
> >
> >  $(LIBSUBCMD_OUTPUT):
> > @@ -77,6 +81,7 @@ $(LIBSUBCMD_OUTPUT):
> >  $(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
> >         $(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
> >                 DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
> > +               $(HOST_OVERRIDES) \
> >                 $@ install_headers
> >
> >  $(LIBSUBCMD)-clean:
> > --
> > 2.38.1.584.g0f3c55d4c2-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
Nick Desaulniers Dec. 14, 2022, 6:34 p.m. UTC | #3 
On Wed, Dec 14, 2022 at 10:25 AM Ian Rogers <irogers@google.com> wrote:
>
> On Fri, Dec 9, 2022 at 10:26 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > HOSTCC is always wanted when building objtool. Setting CC to HOSTCC
> > > happens after tools/scripts/Makefile.include is included, meaning
> > > flags are set assuming say CC is gcc, but then it can be later set to
> > > HOSTCC which may be clang. tools/scripts/Makefile.include is needed
> > > for host set up and common macros in objtool's Makefile. Rather than
> > > override CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the
> > > libsubcmd builds and the linkage step. This means the Makefiles don't
> > > see things like CC changing and tool flag determination, and similar,
> > > work properly. To avoid mixing CFLAGS from different compilers just
> > > the objtool CFLAGS are determined. HOSTCFLAGS is added to these so
> > > that command line flags can add to the CFLAGS.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/objtool/Makefile | 27 ++++++++++++++++-----------
> > >  1 file changed, 16 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > > index 61a00b7acae9..e550a98e2dd9 100644
> > > --- a/tools/objtool/Makefile
> > > +++ b/tools/objtool/Makefile
> > > @@ -2,16 +2,12 @@
> > >  include ../scripts/Makefile.include
> > >  include ../scripts/Makefile.arch
> > >
> > > -# always use the host compiler
> > > -AR      = $(HOSTAR)
> > > -CC      = $(HOSTCC)
> > > -LD      = $(HOSTLD)
> > > -
> > >  ifeq ($(srctree),)
> > >  srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> > >  srctree := $(patsubst %/,%,$(dir $(srctree)))
> > >  endif
> > >
> > > +MAKE = make -S
> > >  LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
> > >  ifneq ($(OUTPUT),)
> > >    LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
> > > @@ -36,13 +32,20 @@ INCLUDES := -I$(srctree)/tools/include \
> > >             -I$(srctree)/tools/objtool/include \
> > >             -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include \
> > >             -I$(LIBSUBCMD_OUTPUT)/include
> > > -WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
> >
> > Looking closer at the V=1 diff in meld, I think this is dropping
> > EXTRA_WARNINGS.  I think you want to add those back to OBJTOOL_CFLAGS.
>
> Thanks Nick! When I try this it causes new build failures.
> Specifically EXTRA_WARNINGS includes things like -Wswitch-enum that
> trigger:

That's...unexpected. These warnings didn't exist when EXTRA_WARNINGS
was being passed to the compiler for these...unless that wasn't the
case?

>
> ```
> check.c:1312:10: error: 14 enumeration values not explicitly handled
> in switch: 'INSN_JUMP_DYNAMIC',
> 'INSN_JUMP_DYNAMIC_CONDITIONAL', 'INSN_CALL_DYNAMIC'... [-Werror,-Wswitch-enum]
>        switch (insn->type) {
>                ^~~~~~~~~~
> check.c:2620:11: error: enumeration value 'OP_SRC_CONST' not
> explicitly handled in switch [-Werror,-
> Wswitch-enum]
>                switch (op->src.type) {
>                        ^~~~~~~~~~~~
> check.c:3447:11: error: 5 enumeration values not explicitly handled in
> switch: 'INSN_BUG', 'INSN_NOP
> ', 'INSN_TRAP'... [-Werror,-Wswitch-enum]
>                switch (insn->type) {
>                        ^~~~~~~~~~
> check.c:3647:11: error: 9 enumeration values not explicitly handled in
> switch: 'INSN_CONTEXT_SWITCH'
> , 'INSN_BUG', 'INSN_STAC'... [-Werror,-Wswitch-enum]
>                switch (insn->type) {
>                        ^~~~~~~~~~
> check.c:4008:10: error: 9 enumeration values not explicitly handled in
> switch: 'INSN_CONTEXT_SWITCH'
> , 'INSN_BUG', 'INSN_STAC'... [-Werror,-Wswitch-enum]
>        switch (insn->type) {
>                ^~~~~~~~~~
> check.c:4173:11: error: 15 enumeration values not explicitly handled
> in switch: 'INSN_JUMP_CONDITION
> AL', 'INSN_JUMP_UNCONDITIONAL', 'INSN_JUMP_DYNAMIC_CONDITIONAL'...
> [-Werror,-Wswitch-enum]
>                switch (insn->type) {
>                        ^~~~~~~~~~
> 6 errors generated.
> ```
>
> What should the next step be? Land this or add EXTRA_WARNINGS and fix
> all the issues?

Up to Josh. I think the first 3 patches are ready to go.

>
> Thanks,
> Ian
>
> > > -CFLAGS   := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS)
> > > -LDFLAGS  += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
> > > +WARNINGS := -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
> > > +OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS)
> > > +OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD)
> > >
> > >  # Allow old libelf to be used:
> > >  elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
> > > -CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > > +OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > > +
> > > +# Always want host compilation.
> > > +HOST_OVERRIDES := CC="$(HOSTCC)" EXTRA_CFLAGS="$(OBJTOOL_CFLAGS)" \
> > > +                 LD="$(HOSTLD)" AR="$(HOSTAR)"
> > > +BUILD_HOST_OVERRIDES := CC="$(HOSTCC)" CFLAGS="$(OBJTOOL_CFLAGS)" \
> > > +                       LD="$(HOSTLD)" LDFLAGS="$(OBJTOOL_LDFLAGS)" \
> > > +                       AR="$(HOSTAR)"
> > >
> > >  AWK = awk
> > >  MKDIR = mkdir
> > > @@ -65,10 +68,11 @@ include $(srctree)/tools/build/Makefile.include
> > >
> > >  $(OBJTOOL_IN): fixdep FORCE
> > >         $(Q)$(CONFIG_SHELL) ./sync-check.sh
> > > -       $(Q)$(MAKE) $(build)=objtool
> > > +       $(Q)$(MAKE) $(build)=objtool $(BUILD_HOST_OVERRIDES)
> > > +
> > >
> > >  $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
> > > -       $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
> > > +       $(QUIET_LINK)$(HOSTCC) $(OBJTOOL_IN) $(KBUILD_HOSTLDFLAGS) $(OBJTOOL_LDFLAGS) -o $@
> > >
> > >
> > >  $(LIBSUBCMD_OUTPUT):
> > > @@ -77,6 +81,7 @@ $(LIBSUBCMD_OUTPUT):
> > >  $(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
> > >         $(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
> > >                 DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
> > > +               $(HOST_OVERRIDES) \
> > >                 $@ install_headers
> > >
> > >  $(LIBSUBCMD)-clean:
> > > --
> > > 2.38.1.584.g0f3c55d4c2-goog
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
Ian Rogers Dec. 14, 2022, 7:14 p.m. UTC | #4 
On Wed, Dec 14, 2022 at 10:34 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Dec 14, 2022 at 10:25 AM Ian Rogers <irogers@google.com> wrote:
> >
> > On Fri, Dec 9, 2022 at 10:26 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > HOSTCC is always wanted when building objtool. Setting CC to HOSTCC
> > > > happens after tools/scripts/Makefile.include is included, meaning
> > > > flags are set assuming say CC is gcc, but then it can be later set to
> > > > HOSTCC which may be clang. tools/scripts/Makefile.include is needed
> > > > for host set up and common macros in objtool's Makefile. Rather than
> > > > override CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the
> > > > libsubcmd builds and the linkage step. This means the Makefiles don't
> > > > see things like CC changing and tool flag determination, and similar,
> > > > work properly. To avoid mixing CFLAGS from different compilers just
> > > > the objtool CFLAGS are determined. HOSTCFLAGS is added to these so
> > > > that command line flags can add to the CFLAGS.
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > >  tools/objtool/Makefile | 27 ++++++++++++++++-----------
> > > >  1 file changed, 16 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > > > index 61a00b7acae9..e550a98e2dd9 100644
> > > > --- a/tools/objtool/Makefile
> > > > +++ b/tools/objtool/Makefile
> > > > @@ -2,16 +2,12 @@
> > > >  include ../scripts/Makefile.include
> > > >  include ../scripts/Makefile.arch
> > > >
> > > > -# always use the host compiler
> > > > -AR      = $(HOSTAR)
> > > > -CC      = $(HOSTCC)
> > > > -LD      = $(HOSTLD)
> > > > -
> > > >  ifeq ($(srctree),)
> > > >  srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> > > >  srctree := $(patsubst %/,%,$(dir $(srctree)))
> > > >  endif
> > > >
> > > > +MAKE = make -S
> > > >  LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
> > > >  ifneq ($(OUTPUT),)
> > > >    LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
> > > > @@ -36,13 +32,20 @@ INCLUDES := -I$(srctree)/tools/include \
> > > >             -I$(srctree)/tools/objtool/include \
> > > >             -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include \
> > > >             -I$(LIBSUBCMD_OUTPUT)/include
> > > > -WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
> > >
> > > Looking closer at the V=1 diff in meld, I think this is dropping
> > > EXTRA_WARNINGS.  I think you want to add those back to OBJTOOL_CFLAGS.
> >
> > Thanks Nick! When I try this it causes new build failures.
> > Specifically EXTRA_WARNINGS includes things like -Wswitch-enum that
> > trigger:
>
> That's...unexpected. These warnings didn't exist when EXTRA_WARNINGS
> was being passed to the compiler for these...unless that wasn't the
> case?

I wasn't adding EXTRA_WARNINGS but they are set up here:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/scripts/Makefile.include?id=630ae80ea1dd253609cb50cff87f3248f901aca3#n23

I've avoided in these changes passing through values from
Makefile.include to the subprocesses, like Makefile.build, as
Makefile.include makes choices based on the current flags like clang
and llvm, not those we override.

> >
> > ```
> > check.c:1312:10: error: 14 enumeration values not explicitly handled
> > in switch: 'INSN_JUMP_DYNAMIC',
> > 'INSN_JUMP_DYNAMIC_CONDITIONAL', 'INSN_CALL_DYNAMIC'... [-Werror,-Wswitch-enum]
> >        switch (insn->type) {
> >                ^~~~~~~~~~
> > check.c:2620:11: error: enumeration value 'OP_SRC_CONST' not
> > explicitly handled in switch [-Werror,-
> > Wswitch-enum]
> >                switch (op->src.type) {
> >                        ^~~~~~~~~~~~
> > check.c:3447:11: error: 5 enumeration values not explicitly handled in
> > switch: 'INSN_BUG', 'INSN_NOP
> > ', 'INSN_TRAP'... [-Werror,-Wswitch-enum]
> >                switch (insn->type) {
> >                        ^~~~~~~~~~
> > check.c:3647:11: error: 9 enumeration values not explicitly handled in
> > switch: 'INSN_CONTEXT_SWITCH'
> > , 'INSN_BUG', 'INSN_STAC'... [-Werror,-Wswitch-enum]
> >                switch (insn->type) {
> >                        ^~~~~~~~~~
> > check.c:4008:10: error: 9 enumeration values not explicitly handled in
> > switch: 'INSN_CONTEXT_SWITCH'
> > , 'INSN_BUG', 'INSN_STAC'... [-Werror,-Wswitch-enum]
> >        switch (insn->type) {
> >                ^~~~~~~~~~
> > check.c:4173:11: error: 15 enumeration values not explicitly handled
> > in switch: 'INSN_JUMP_CONDITION
> > AL', 'INSN_JUMP_UNCONDITIONAL', 'INSN_JUMP_DYNAMIC_CONDITIONAL'...
> > [-Werror,-Wswitch-enum]
> >                switch (insn->type) {
> >                        ^~~~~~~~~~
> > 6 errors generated.
> > ```
> >
> > What should the next step be? Land this or add EXTRA_WARNINGS and fix
> > all the issues?
>
> Up to Josh. I think the first 3 patches are ready to go.

Note, the first patch is already in Arnaldo's tree:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=630ae80ea1dd253609cb50cff87f3248f901aca3
and shows up in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/tools/lib/subcmd/Makefile?id=630ae80ea1dd253609cb50cff87f3248f901aca3

Thanks,
Ian



> >
> > Thanks,
> > Ian
> >
> > > > -CFLAGS   := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS)
> > > > -LDFLAGS  += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
> > > > +WARNINGS := -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
> > > > +OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS)
> > > > +OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD)
> > > >
> > > >  # Allow old libelf to be used:
> > > >  elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
> > > > -CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > > > +OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > > > +
> > > > +# Always want host compilation.
> > > > +HOST_OVERRIDES := CC="$(HOSTCC)" EXTRA_CFLAGS="$(OBJTOOL_CFLAGS)" \
> > > > +                 LD="$(HOSTLD)" AR="$(HOSTAR)"
> > > > +BUILD_HOST_OVERRIDES := CC="$(HOSTCC)" CFLAGS="$(OBJTOOL_CFLAGS)" \
> > > > +                       LD="$(HOSTLD)" LDFLAGS="$(OBJTOOL_LDFLAGS)" \
> > > > +                       AR="$(HOSTAR)"
> > > >
> > > >  AWK = awk
> > > >  MKDIR = mkdir
> > > > @@ -65,10 +68,11 @@ include $(srctree)/tools/build/Makefile.include
> > > >
> > > >  $(OBJTOOL_IN): fixdep FORCE
> > > >         $(Q)$(CONFIG_SHELL) ./sync-check.sh
> > > > -       $(Q)$(MAKE) $(build)=objtool
> > > > +       $(Q)$(MAKE) $(build)=objtool $(BUILD_HOST_OVERRIDES)
> > > > +
> > > >
> > > >  $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
> > > > -       $(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
> > > > +       $(QUIET_LINK)$(HOSTCC) $(OBJTOOL_IN) $(KBUILD_HOSTLDFLAGS) $(OBJTOOL_LDFLAGS) -o $@
> > > >
> > > >
> > > >  $(LIBSUBCMD_OUTPUT):
> > > > @@ -77,6 +81,7 @@ $(LIBSUBCMD_OUTPUT):
> > > >  $(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
> > > >         $(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
> > > >                 DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
> > > > +               $(HOST_OVERRIDES) \
> > > >                 $@ install_headers
> > > >
> > > >  $(LIBSUBCMD)-clean:
> > > > --
> > > > 2.38.1.584.g0f3c55d4c2-goog
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > > ~Nick Desaulniers
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
diff mbox series

Patch

diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 61a00b7acae9..e550a98e2dd9 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -2,16 +2,12 @@ 
 include ../scripts/Makefile.include
 include ../scripts/Makefile.arch
 
-# always use the host compiler
-AR	 = $(HOSTAR)
-CC	 = $(HOSTCC)
-LD	 = $(HOSTLD)
-
 ifeq ($(srctree),)
 srctree := $(patsubst %/,%,$(dir $(CURDIR)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
 endif
 
+MAKE = make -S
 LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
 ifneq ($(OUTPUT),)
   LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
@@ -36,13 +32,20 @@  INCLUDES := -I$(srctree)/tools/include \
 	    -I$(srctree)/tools/objtool/include \
 	    -I$(srctree)/tools/objtool/arch/$(SRCARCH)/include \
 	    -I$(LIBSUBCMD_OUTPUT)/include
-WARNINGS := $(EXTRA_WARNINGS) -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
-CFLAGS   := -Werror $(WARNINGS) $(KBUILD_HOSTCFLAGS) -g $(INCLUDES) $(LIBELF_FLAGS)
-LDFLAGS  += $(LIBELF_LIBS) $(LIBSUBCMD) $(KBUILD_HOSTLDFLAGS)
+WARNINGS := -Wno-switch-default -Wno-switch-enum -Wno-packed -Wno-nested-externs
+OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS)
+OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD)
 
 # Allow old libelf to be used:
 elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E - | grep elf_getshdr)
-CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
+OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
+
+# Always want host compilation.
+HOST_OVERRIDES := CC="$(HOSTCC)" EXTRA_CFLAGS="$(OBJTOOL_CFLAGS)" \
+		  LD="$(HOSTLD)" AR="$(HOSTAR)"
+BUILD_HOST_OVERRIDES := CC="$(HOSTCC)" CFLAGS="$(OBJTOOL_CFLAGS)" \
+			LD="$(HOSTLD)" LDFLAGS="$(OBJTOOL_LDFLAGS)" \
+			AR="$(HOSTAR)"
 
 AWK = awk
 MKDIR = mkdir
@@ -65,10 +68,11 @@  include $(srctree)/tools/build/Makefile.include
 
 $(OBJTOOL_IN): fixdep FORCE
 	$(Q)$(CONFIG_SHELL) ./sync-check.sh
-	$(Q)$(MAKE) $(build)=objtool
+	$(Q)$(MAKE) $(build)=objtool $(BUILD_HOST_OVERRIDES)
+
 
 $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
-	$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
+	$(QUIET_LINK)$(HOSTCC) $(OBJTOOL_IN) $(KBUILD_HOSTLDFLAGS) $(OBJTOOL_LDFLAGS) -o $@
 
 
 $(LIBSUBCMD_OUTPUT):
@@ -77,6 +81,7 @@  $(LIBSUBCMD_OUTPUT):
 $(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
 	$(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
 		DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
+		$(HOST_OVERRIDES) \
 		$@ install_headers
 
 $(LIBSUBCMD)-clean: