Messages in this thread | | | From | Nick Desaulniers <> | Date | Wed, 14 Dec 2022 10:34:21 -0800 | Subject | Re: [PATCH v2 4/4] objtool: Alter how HOSTCC is forced |
| |
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
-- Thanks, ~Nick Desaulniers
| |