Install libsubcmd and then get headers from there, this avoids
inadvertent dependencies on things in tools/lib. Fix V=1
support. Clean up how HOSTCC is used to override CC to avoid CFLAGS
being set for say gcc, and then CC being overridden to clang. Support
HOSTCFLAGS as a make option.
v2. Include required "tools lib subcmd: Add install target" that is
already in Arnaldo's tree:
https://lore.kernel.org/lkml/[email protected]/
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=630ae80ea1dd253609cb50cff87f3248f901aca3
When building libsubcmd.a from objtool's Makefile, clear the
subdir to avoid it being appended onto OUTPUT and breaking the
build.
Ian Rogers (4):
tools lib subcmd: Add install target
objtool: Install libsubcmd in build
objtool: Properly support make V=1
objtool: Alter how HOSTCC is forced
tools/lib/subcmd/Makefile | 49 ++++++++++++++++++++++++++++
tools/objtool/Build | 2 --
tools/objtool/Makefile | 68 +++++++++++++++++++++++++++------------
3 files changed, 97 insertions(+), 22 deletions(-)
--
2.38.1.584.g0f3c55d4c2-goog
The Q variable was being used but never correctly set up. Add the
setting up and use in place of @.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/objtool/Makefile | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index fd9b3e3113c6..61a00b7acae9 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -47,6 +47,12 @@ CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
AWK = awk
MKDIR = mkdir
+ifeq ($(V),1)
+ Q =
+else
+ Q = @
+endif
+
BUILD_ORC := n
ifeq ($(SRCARCH),x86)
@@ -58,18 +64,18 @@ export srctree OUTPUT CFLAGS SRCARCH AWK
include $(srctree)/tools/build/Makefile.include
$(OBJTOOL_IN): fixdep FORCE
- @$(CONFIG_SHELL) ./sync-check.sh
- @$(MAKE) $(build)=objtool
+ $(Q)$(CONFIG_SHELL) ./sync-check.sh
+ $(Q)$(MAKE) $(build)=objtool
$(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
$(LIBSUBCMD_OUTPUT):
- @$(MKDIR) -p $@
+ $(Q)$(MKDIR) -p $@
$(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
- @$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
+ $(Q)$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
$@ install_headers
--
2.38.1.584.g0f3c55d4c2-goog
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 <[email protected]>
---
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
-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
This allows libsubcmd to be installed as a dependency.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/lib/subcmd/Makefile | 49 +++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 8f1a09cdfd17..e96566f8991c 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -17,6 +17,15 @@ RM = rm -f
MAKEFLAGS += --no-print-directory
+INSTALL = install
+
+# Use DESTDIR for installing into a different root directory.
+# This is useful for building a package. The program will be
+# installed in this directory as if it was the root directory.
+# Then the build tool can move it later.
+DESTDIR ?=
+DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
+
LIBFILE = $(OUTPUT)libsubcmd.a
CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
@@ -48,6 +57,18 @@ CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
+ifeq ($(LP64), 1)
+ libdir_relative = lib64
+else
+ libdir_relative = lib
+endif
+
+prefix ?=
+libdir = $(prefix)/$(libdir_relative)
+
+# Shell quotes
+libdir_SQ = $(subst ','\'',$(libdir))
+
all:
export srctree OUTPUT CC LD CFLAGS V
@@ -61,6 +82,34 @@ $(SUBCMD_IN): FORCE
$(LIBFILE): $(SUBCMD_IN)
$(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(SUBCMD_IN)
+define do_install_mkdir
+ if [ ! -d '$(DESTDIR_SQ)$1' ]; then \
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \
+ fi
+endef
+
+define do_install
+ if [ ! -d '$(DESTDIR_SQ)$2' ]; then \
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \
+ fi; \
+ $(INSTALL) $1 $(if $3,-m $3,) '$(DESTDIR_SQ)$2'
+endef
+
+install_lib: $(LIBFILE)
+ $(call QUIET_INSTALL, $(LIBFILE)) \
+ $(call do_install_mkdir,$(libdir_SQ)); \
+ cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
+
+install_headers:
+ $(call QUIET_INSTALL, headers) \
+ $(call do_install,exec-cmd.h,$(prefix)/include/subcmd,644); \
+ $(call do_install,help.h,$(prefix)/include/subcmd,644); \
+ $(call do_install,pager.h,$(prefix)/include/subcmd,644); \
+ $(call do_install,parse-options.h,$(prefix)/include/subcmd,644); \
+ $(call do_install,run-command.h,$(prefix)/include/subcmd,644);
+
+install: install_lib install_headers
+
clean:
$(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
--
2.38.1.584.g0f3c55d4c2-goog
Including from tools/lib can create inadvertent dependencies. Install
libsubcmd in the objtool build and then include the headers from
there.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/objtool/Build | 2 --
tools/objtool/Makefile | 33 +++++++++++++++++++++++++--------
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/tools/objtool/Build b/tools/objtool/Build
index 33f2ee5a46d3..a3cdf8af6635 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -16,8 +16,6 @@ objtool-y += libctype.o
objtool-y += str_error_r.o
objtool-y += librbtree.o
-CFLAGS += -I$(srctree)/tools/lib
-
$(OUTPUT)libstring.o: ../lib/string.c FORCE
$(call rule_mkdir)
$(call if_changed_dep,cc_o_c)
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index a3a9cc24e0e3..fd9b3e3113c6 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -12,9 +12,15 @@ srctree := $(patsubst %/,%,$(dir $(CURDIR)))
srctree := $(patsubst %/,%,$(dir $(srctree)))
endif
-SUBCMD_SRCDIR = $(srctree)/tools/lib/subcmd/
-LIBSUBCMD_OUTPUT = $(or $(OUTPUT),$(CURDIR)/)
-LIBSUBCMD = $(LIBSUBCMD_OUTPUT)libsubcmd.a
+LIBSUBCMD_DIR = $(srctree)/tools/lib/subcmd/
+ifneq ($(OUTPUT),)
+ LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
+else
+ LIBSUBCMD_OUTPUT = $(CURDIR)/libsubcmd
+endif
+LIBSUBCMD_DESTDIR = $(LIBSUBCMD_OUTPUT)
+LIBSUBCMD = $(LIBSUBCMD_OUTPUT)/libsubcmd.a
+CFLAGS += -I$(LIBSUBCMD_OUTPUT)/include
OBJTOOL := $(OUTPUT)objtool
OBJTOOL_IN := $(OBJTOOL)-in.o
@@ -28,7 +34,8 @@ INCLUDES := -I$(srctree)/tools/include \
-I$(srctree)/tools/arch/$(HOSTARCH)/include/uapi \
-I$(srctree)/tools/arch/$(SRCARCH)/include \
-I$(srctree)/tools/objtool/include \
- -I$(srctree)/tools/objtool/arch/$(SRCARCH)/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)
@@ -38,6 +45,7 @@ elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E -
CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
AWK = awk
+MKDIR = mkdir
BUILD_ORC := n
@@ -57,13 +65,22 @@ $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
$(QUIET_LINK)$(CC) $(OBJTOOL_IN) $(LDFLAGS) -o $@
-$(LIBSUBCMD): fixdep FORCE
- $(Q)$(MAKE) -C $(SUBCMD_SRCDIR) OUTPUT=$(LIBSUBCMD_OUTPUT)
+$(LIBSUBCMD_OUTPUT):
+ @$(MKDIR) -p $@
+
+$(LIBSUBCMD): fixdep FORCE $(LIBSUBCMD_OUTPUT)
+ @$(MAKE) -C $(LIBSUBCMD_DIR) O=$(LIBSUBCMD_OUTPUT) \
+ DESTDIR=$(LIBSUBCMD_DESTDIR) prefix= subdir= \
+ $@ install_headers
+
+$(LIBSUBCMD)-clean:
+ $(call QUIET_CLEAN, libsubcmd)
+ $(Q)$(RM) -r -- $(LIBSUBCMD_OUTPUT)
-clean:
+clean: $(LIBSUBCMD)-clean
$(call QUIET_CLEAN, objtool) $(RM) $(OBJTOOL)
$(Q)find $(OUTPUT) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
- $(Q)$(RM) $(OUTPUT)arch/x86/lib/inat-tables.c $(OUTPUT)fixdep $(LIBSUBCMD)
+ $(Q)$(RM) $(OUTPUT)arch/x86/lib/inat-tables.c $(OUTPUT)fixdep
FORCE:
--
2.38.1.584.g0f3c55d4c2-goog
On Mon 21 Nov 2022 16:11:22 GMT, Ian Rogers wrote:
> This allows libsubcmd to be installed as a dependency.
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/lib/subcmd/Makefile | 49 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> index 8f1a09cdfd17..e96566f8991c 100644
> --- a/tools/lib/subcmd/Makefile
> +++ b/tools/lib/subcmd/Makefile
> @@ -17,6 +17,15 @@ RM = rm -f
>
> MAKEFLAGS += --no-print-directory
>
> +INSTALL = install
> +
> +# Use DESTDIR for installing into a different root directory.
> +# This is useful for building a package. The program will be
> +# installed in this directory as if it was the root directory.
> +# Then the build tool can move it later.
> +DESTDIR ?=
> +DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
> +
> LIBFILE = $(OUTPUT)libsubcmd.a
>
> CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
> @@ -48,6 +57,18 @@ CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
>
> SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
>
> +ifeq ($(LP64), 1)
> + libdir_relative = lib64
> +else
> + libdir_relative = lib
> +endif
> +
> +prefix ?=
> +libdir = $(prefix)/$(libdir_relative)
> +
> +# Shell quotes
> +libdir_SQ = $(subst ','\'',$(libdir))
> +
> all:
>
> export srctree OUTPUT CC LD CFLAGS V
> @@ -61,6 +82,34 @@ $(SUBCMD_IN): FORCE
> $(LIBFILE): $(SUBCMD_IN)
> $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(SUBCMD_IN)
>
> +define do_install_mkdir
> + if [ ! -d '$(DESTDIR_SQ)$1' ]; then \
> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \
> + fi
> +endef
> +
> +define do_install
> + if [ ! -d '$(DESTDIR_SQ)$2' ]; then \
> + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \
> + fi; \
> + $(INSTALL) $1 $(if $3,-m $3,) '$(DESTDIR_SQ)$2'
> +endef
> +
> +install_lib: $(LIBFILE)
> + $(call QUIET_INSTALL, $(LIBFILE)) \
> + $(call do_install_mkdir,$(libdir_SQ)); \
> + cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
> +
Sorry for being late.
Is there a specific reason why you do not use 'mkdir -p
$(DESTDIR)/$(2); cp $(1) $(2)' or 'install $(addprefix -m,$(3)) -D $(1)
$(2)' for the install rules (cp. scripts/Makefile.{dtb,mod}inst)?
I think you could get rid of mkdir calls and the ..._SQ handling when
using one of them.
> +install_headers:
> + $(call QUIET_INSTALL, headers) \
Unlikely, but if one of the install commands fails, you probably want
make to exit with an error. Might you want to add 'set -e; \' here?
Kind regards,
Nicolas
> + $(call do_install,exec-cmd.h,$(prefix)/include/subcmd,644); \
> + $(call do_install,help.h,$(prefix)/include/subcmd,644); \
> + $(call do_install,pager.h,$(prefix)/include/subcmd,644); \
> + $(call do_install,parse-options.h,$(prefix)/include/subcmd,644); \
> + $(call do_install,run-command.h,$(prefix)/include/subcmd,644);
> +
> +install: install_lib install_headers
> +
> clean:
> $(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
> find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
> --
> 2.38.1.584.g0f3c55d4c2-goog
--
epost|xmpp: [email protected] irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
On Mon, Nov 21, 2022 at 10:48 PM Nicolas Schier <[email protected]> wrote:
>
> On Mon 21 Nov 2022 16:11:22 GMT, Ian Rogers wrote:
> > This allows libsubcmd to be installed as a dependency.
> >
> > Signed-off-by: Ian Rogers <[email protected]>
> > ---
> > tools/lib/subcmd/Makefile | 49 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> > index 8f1a09cdfd17..e96566f8991c 100644
> > --- a/tools/lib/subcmd/Makefile
> > +++ b/tools/lib/subcmd/Makefile
> > @@ -17,6 +17,15 @@ RM = rm -f
> >
> > MAKEFLAGS += --no-print-directory
> >
> > +INSTALL = install
> > +
> > +# Use DESTDIR for installing into a different root directory.
> > +# This is useful for building a package. The program will be
> > +# installed in this directory as if it was the root directory.
> > +# Then the build tool can move it later.
> > +DESTDIR ?=
> > +DESTDIR_SQ = '$(subst ','\'',$(DESTDIR))'
> > +
> > LIBFILE = $(OUTPUT)libsubcmd.a
> >
> > CFLAGS := -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
> > @@ -48,6 +57,18 @@ CFLAGS += $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
> >
> > SUBCMD_IN := $(OUTPUT)libsubcmd-in.o
> >
> > +ifeq ($(LP64), 1)
> > + libdir_relative = lib64
> > +else
> > + libdir_relative = lib
> > +endif
> > +
> > +prefix ?=
> > +libdir = $(prefix)/$(libdir_relative)
> > +
> > +# Shell quotes
> > +libdir_SQ = $(subst ','\'',$(libdir))
> > +
> > all:
> >
> > export srctree OUTPUT CC LD CFLAGS V
> > @@ -61,6 +82,34 @@ $(SUBCMD_IN): FORCE
> > $(LIBFILE): $(SUBCMD_IN)
> > $(QUIET_AR)$(RM) $@ && $(AR) rcs $@ $(SUBCMD_IN)
> >
> > +define do_install_mkdir
> > + if [ ! -d '$(DESTDIR_SQ)$1' ]; then \
> > + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$1'; \
> > + fi
> > +endef
> > +
> > +define do_install
> > + if [ ! -d '$(DESTDIR_SQ)$2' ]; then \
> > + $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$2'; \
> > + fi; \
> > + $(INSTALL) $1 $(if $3,-m $3,) '$(DESTDIR_SQ)$2'
> > +endef
> > +
> > +install_lib: $(LIBFILE)
> > + $(call QUIET_INSTALL, $(LIBFILE)) \
> > + $(call do_install_mkdir,$(libdir_SQ)); \
> > + cp -fpR $(LIBFILE) $(DESTDIR)$(libdir_SQ)
> > +
>
> Sorry for being late.
>
> Is there a specific reason why you do not use 'mkdir -p
> $(DESTDIR)/$(2); cp $(1) $(2)' or 'install $(addprefix -m,$(3)) -D $(1)
> $(2)' for the install rules (cp. scripts/Makefile.{dtb,mod}inst)?
>
> I think you could get rid of mkdir calls and the ..._SQ handling when
> using one of them.
Thanks for the feedback! Perhaps we can merge this as being consistent
with existing approaches:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/perf/Makefile#n185
and then refactor as you say.
> > +install_headers:
> > + $(call QUIET_INSTALL, headers) \
>
> Unlikely, but if one of the install commands fails, you probably want
> make to exit with an error. Might you want to add 'set -e; \' here?
Possibly. Again, I think we should aim for consistency and do this as
a follow up.
Thanks,
Ian
> Kind regards,
> Nicolas
>
>
> > + $(call do_install,exec-cmd.h,$(prefix)/include/subcmd,644); \
> > + $(call do_install,help.h,$(prefix)/include/subcmd,644); \
> > + $(call do_install,pager.h,$(prefix)/include/subcmd,644); \
> > + $(call do_install,parse-options.h,$(prefix)/include/subcmd,644); \
> > + $(call do_install,run-command.h,$(prefix)/include/subcmd,644);
> > +
> > +install: install_lib install_headers
> > +
> > clean:
> > $(call QUIET_CLEAN, libsubcmd) $(RM) $(LIBFILE); \
> > find $(or $(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
> > --
> > 2.38.1.584.g0f3c55d4c2-goog
>
> --
> epost|xmpp: [email protected] irc://oftc.net/nsc
> ↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
> -- frykten for herren er opphav til kunnskap --
On Mon, Nov 21, 2022 at 4:11 PM Ian Rogers <[email protected]> wrote:
>
> +ifneq ($(OUTPUT),)
> + LIBSUBCMD_OUTPUT = $(abspath $(OUTPUT))/libsubcmd
> +else
> + LIBSUBCMD_OUTPUT = $(CURDIR)/libsubcmd
> +endif
Seeing an else clause when the predicate is negated makes this read as:
if !foo:
baz()
else:
bar()
Consider using:
if foo:
bar()
else
baz()
in the future.
--
Thanks,
~Nick Desaulniers
On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <[email protected]> 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 <[email protected]>
> ---
> 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
>
--
Thanks,
~Nick Desaulniers
On Fri, Dec 9, 2022 at 10:17 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Mon, Nov 21, 2022 at 4:11 PM Ian Rogers <[email protected]> wrote:
> >
> > Install libsubcmd and then get headers from there, this avoids
> > inadvertent dependencies on things in tools/lib. Fix V=1
> > support. Clean up how HOSTCC is used to override CC to avoid CFLAGS
> > being set for say gcc, and then CC being overridden to clang. Support
> > HOSTCFLAGS as a make option.
>
> Ian, I'm terribly sorry about the delay in reviewing this; usually my
> turnaround time is much lower on code review...
>
> Anyways, the patchset LGTM; thanks for the work that went into this.
>
> Reviewed-by: Nick Desaulniers <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>
Sorry, I just spotted a potential issue in patch 4/4. You may retain
my above tags for the first three patches if you send a v3.
>
> >
> > v2. Include required "tools lib subcmd: Add install target" that is
> > already in Arnaldo's tree:
> > https://lore.kernel.org/lkml/[email protected]/
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=630ae80ea1dd253609cb50cff87f3248f901aca3
> > When building libsubcmd.a from objtool's Makefile, clear the
> > subdir to avoid it being appended onto OUTPUT and breaking the
> > build.
> >
> > Ian Rogers (4):
> > tools lib subcmd: Add install target
> > objtool: Install libsubcmd in build
> > objtool: Properly support make V=1
> > objtool: Alter how HOSTCC is forced
> >
> > tools/lib/subcmd/Makefile | 49 ++++++++++++++++++++++++++++
> > tools/objtool/Build | 2 --
> > tools/objtool/Makefile | 68 +++++++++++++++++++++++++++------------
> > 3 files changed, 97 insertions(+), 22 deletions(-)
> >
> > --
> > 2.38.1.584.g0f3c55d4c2-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Mon, Nov 21, 2022 at 4:11 PM Ian Rogers <[email protected]> wrote:
>
> Install libsubcmd and then get headers from there, this avoids
> inadvertent dependencies on things in tools/lib. Fix V=1
> support. Clean up how HOSTCC is used to override CC to avoid CFLAGS
> being set for say gcc, and then CC being overridden to clang. Support
> HOSTCFLAGS as a make option.
Ian, I'm terribly sorry about the delay in reviewing this; usually my
turnaround time is much lower on code review...
Anyways, the patchset LGTM; thanks for the work that went into this.
Reviewed-by: Nick Desaulniers <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
>
> v2. Include required "tools lib subcmd: Add install target" that is
> already in Arnaldo's tree:
> https://lore.kernel.org/lkml/[email protected]/
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=630ae80ea1dd253609cb50cff87f3248f901aca3
> When building libsubcmd.a from objtool's Makefile, clear the
> subdir to avoid it being appended onto OUTPUT and breaking the
> build.
>
> Ian Rogers (4):
> tools lib subcmd: Add install target
> objtool: Install libsubcmd in build
> objtool: Properly support make V=1
> objtool: Alter how HOSTCC is forced
>
> tools/lib/subcmd/Makefile | 49 ++++++++++++++++++++++++++++
> tools/objtool/Build | 2 --
> tools/objtool/Makefile | 68 +++++++++++++++++++++++++++------------
> 3 files changed, 97 insertions(+), 22 deletions(-)
>
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
--
Thanks,
~Nick Desaulniers
On Wed, Dec 14, 2022 at 10:25 AM Ian Rogers <[email protected]> wrote:
>
> On Fri, Dec 9, 2022 at 10:26 AM Nick Desaulniers
> <[email protected]> wrote:
> >
> > On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <[email protected]> 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 <[email protected]>
> > > ---
> > > 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
On Fri, Dec 9, 2022 at 10:26 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Wed, Dec 14, 2022 at 10:34 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Wed, Dec 14, 2022 at 10:25 AM Ian Rogers <[email protected]> wrote:
> >
> > On Fri, Dec 9, 2022 at 10:26 AM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > On Mon, Nov 21, 2022 at 4:12 PM Ian Rogers <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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