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 with the exception of
EXTRA_WARNINGS. 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 | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 61a00b7acae9..49956f4f58b9 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
@@ -37,12 +33,19 @@ INCLUDES := -I$(srctree)/tools/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)
+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.39.0.314.g84b9a713c41-goog
On Thu, Jan 05, 2023 at 01:01:55AM -0800, Ian Rogers 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. I'm having trouble parsing this last sentence, can you rephrase or restructure it? > To avoid mixing CFLAGS from different compilers just > the objtool CFLAGS are determined with the exception of > EXTRA_WARNINGS. I'm not really sure what this one means either. > HOSTCFLAGS is added to these so that command line > flags can add to the CFLAGS. Overall this patch description is a big wall of dense text which I found hard to decipher. Please split it up into paragraphs and make it more legible and logical. For example, un-jumble the ordering, with the background first, then the problem(s), then the fix(es). (At least three paragraphs) If possible, the subject should describe the end result for the user, something like objtool: Fix HOSTCFLAGS cmdline support ... unless I'm misunderstanding the point of the patch. HOSTCFLAGS from the cmdline *used* to work, a git bisect shows it stopped working with 96f14fe738b6 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS") ... so please add a "Fixes" tag for that commit. > +MAKE = make -S Why? > LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/ > ifneq ($(OUTPUT),) > LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd > @@ -37,12 +33,19 @@ INCLUDES := -I$(srctree)/tools/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) > +OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS) I *think* I understand the reason for the switch from KBUILD_HOSTCFLAGS to HOSTCFLAGS -- because KBUILD_HOSTCFLAGS is defined in the top-level kernel Makefile which isn't included here so it's undefined? -- but regardless that should be called out more explicitly as a problem being fixed in the commit log. This issue really has me scratching my head, as I could have sworn objtool was being built with -O2. > +OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD) Is there a reason to not include $(HOSTLDFLAGS) here? > # 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)" Maybe it depends on your perspective, but I'm thinking that some of these aren't really overrides, but rather normal expected flags. And the distinction between these two variables is muddy: it's not only Build vs non-Build, but also objtool vs libsubcmd. How about just HOST_OVERRIDES := CC="$(HOSTCC) "LD="$(HOSTLD)" AR="$(HOSTAR)" And then specifying the {EXTRA_}CFLAGS and LDFLAGS below where needed. > 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 $@ Does KBUILD_HOSTLDFLAGS even work here? -- Josh
On Wed, Jan 25, 2023 at 5:49 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > On Thu, Jan 05, 2023 at 01:01:55AM -0800, Ian Rogers 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. > > I'm having trouble parsing this last sentence, can you rephrase or > restructure it? It was restating what was happening, deleted. > > To avoid mixing CFLAGS from different compilers just > > the objtool CFLAGS are determined with the exception of > > EXTRA_WARNINGS. > > I'm not really sure what this one means either. Moved to an inline comment. > > HOSTCFLAGS is added to these so that command line > > flags can add to the CFLAGS. > > Overall this patch description is a big wall of dense text which I found > hard to decipher. Please split it up into paragraphs and make it more > legible and logical. Yes, I've deleted most of the text now. I'd been adding to it as v1 went to v2 and so on. > For example, un-jumble the ordering, with the background first, then the > problem(s), then the fix(es). (At least three paragraphs) > > If possible, the subject should describe the end result for the user, > something like > > objtool: Fix HOSTCFLAGS cmdline support > > ... unless I'm misunderstanding the point of the patch. The patch is trying to fix the Makefile. Setting "CC=$(HOSTCC)" was just wrong as apparent from looking at the behavior of tools/scripts/Makefile.include. Some of that persists after this change with WARNINGS, as now noted in a comment. A side effect of fixing the Makefile is to make HOSTCFLAGS work, but I suspect with the variable renames this may not be working. I'll leave that for yourself and Nick. I told Nick I'd take a look at this as I saw the wrong use of libsubcmd's headers and that was something I wanted to clean up having done similar in tools/perf, along with the whole removal of tools/lib/traceevent from Linux 6.2. > HOSTCFLAGS from the cmdline *used* to work, a git bisect shows it > stopped working with > > 96f14fe738b6 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS") > > ... so please add a "Fixes" tag for that commit. Left off, see later. > > +MAKE = make -S > > Why? Cruft, removed. > > LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/ > > ifneq ($(OUTPUT),) > > LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd > > @@ -37,12 +33,19 @@ INCLUDES := -I$(srctree)/tools/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) > > +OBJTOOL_CFLAGS := -Werror $(WARNINGS) -g $(INCLUDES) $(LIBELF_FLAGS) $(HOSTCFLAGS) > > I *think* I understand the reason for the switch from KBUILD_HOSTCFLAGS > to HOSTCFLAGS -- because KBUILD_HOSTCFLAGS is defined in the top-level > kernel Makefile which isn't included here so it's undefined? -- but > regardless that should be called out more explicitly as a problem being > fixed in the commit log. I was matching tools/perf, I've switched back to KBUILD_HOSTCFLAGS in v4. There's some higher logic at play with these variable names and I'm not aware of it so I'll leave it to be fixed if necessary later. > This issue really has me scratching my head, as I could have sworn > objtool was being built with -O2. > > > +OBJTOOL_LDFLAGS := $(LIBELF_LIBS) $(LIBSUBCMD) > > Is there a reason to not include $(HOSTLDFLAGS) here? Done as KBUILD_HOSTLDFLAGS as I don't see HOSTLDFLAGS set anywhere. > > # 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)" > > Maybe it depends on your perspective, but I'm thinking that some of > these aren't really overrides, but rather normal expected flags. And > the distinction between these two variables is muddy: it's not only > Build vs non-Build, but also objtool vs libsubcmd. > > How about just > > HOST_OVERRIDES := CC="$(HOSTCC) "LD="$(HOSTLD)" AR="$(HOSTAR)" > > And then specifying the {EXTRA_}CFLAGS and LDFLAGS below where needed. Done. > > 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 $@ > > Does KBUILD_HOSTLDFLAGS even work here? Removed, albeit now to be part of OBJTOOL_LDFLAGS as your earlier comment requested. Thanks, Ian > -- > Josh
On Thu, Jan 05, 2023 at 01:01:55AM -0800 Ian Rogers 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 with the exception of > EXTRA_WARNINGS. 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 | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
On Thu, Jan 5, 2023 at 1:02 AM 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 with the exception of > EXTRA_WARNINGS. HOSTCFLAGS is added to these so that command line > flags can add to the CFLAGS. > > Signed-off-by: Ian Rogers <irogers@google.com> Thanks Ian, and happy new year! Sorry for delays reviewing this; I'm just getting back online today. Assuming Peter and Josh may be too, otherwise can Peter or Josh PTAL? Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Nick Desaulniers <ndesaulniers@google.com> (I'm happy that this allows me to now pass HOSTCFLAGS on to objtool successfully) > --- > tools/objtool/Makefile | 25 +++++++++++++++---------- > 1 file changed, 15 insertions(+), 10 deletions(-) > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile > index 61a00b7acae9..49956f4f58b9 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 > @@ -37,12 +33,19 @@ INCLUDES := -I$(srctree)/tools/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) > +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.39.0.314.g84b9a713c41-goog > -- Thanks, ~Nick Desaulniers
© 2016 - 2025 Red Hat, Inc.