[PATCH v4 0/3] tools build: Incorrect fixdep dependencies

[PATCH v4 0/3] tools build: Incorrect fixdep dependencies

[Brian Norris]

Hi all,

The following series consists of a few bugfixes on the topic of "misuse
of fixdep in the tools/ build tree." There is no listed maintainer for
tools/build, but there are for tools/bpf and tools/objtool, which are
the main pieces that affect most users, because they're built as part of
the main kernel build. I've addressed this series to a selection of
those maintainers, and those that have previously applied build changes
in tools/. I hope one of you can apply this series, pending favorable
review. Or feel free to point me to a different set of maintainers.

This patch series came out of poking around some build errors seen by me
and my coworkers, and I found that there were rather similar reports a
while back here:

    Subject: possible dependency error?
    https://lore.kernel.org/all/ZGVi9HbI43R5trN8@bhelgaas/

I reported some findings to that thread; see also subsequent discussion:

    https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/

One element of that discussion: these problems are already solved
consistently in Kbuild. tools/build purposely borrows some from Kbuild,
but also purposely does not actually use Kbuild. While it'd make my life
easier if tools/ would just adopt Kbuild (at least for the tools which
are built during kernel builds), I've chosen a path that I hope will
yield less resistance -- simply hacking up the existing tools/ build
without major changes to its design.

NB: I've also CC'd Kbuild folks, since Masahiro has already been so
helpful here, but note that this is not really a "kbuild" patch series.

Regards,
Brian

Changes in v4:
 - update tools/lib/bpf/.gitignore to exclude 'fixdep'
 - update tools/lib/bpf `make clean` target for fixdep
 - combine $(SHARED_OBJDIR) and $(STATIC_OBJDIR) rules

Changes in v3:
 - Drop unnecessary tools/build/Build

Changes in v2:
 - also fix libbpf shared library rules
 - ensure OUTPUT is always set, and always an absolute path
 - add backup $(Q) definition in tools/build/Makefile.include

Brian Norris (3):
  tools build: Correct libsubcmd fixdep dependencies
  tools build: Avoid circular .fixdep-in.o.cmd issues
  tools build: Correct bpf fixdep dependencies

 tools/build/Build            |  3 ---
 tools/build/Makefile         | 11 ++---------
 tools/build/Makefile.include | 12 +++++++++++-
 tools/lib/bpf/.gitignore     |  1 +
 tools/lib/bpf/Makefile       | 13 ++++++++++---
 tools/lib/subcmd/Makefile    |  2 +-
 6 files changed, 25 insertions(+), 17 deletions(-)
 delete mode 100644 tools/build/Build

-- 
2.45.2.993.g49e7a77208-goog

[PATCH v4 1/3] tools build: Correct libsubcmd fixdep dependencies

[Brian Norris]

All built targets need fixdep to be built first, before handling object
dependencies [1]. We're missing one such dependency before the libsubcmd
target.

This resolves .cmd file generation issues such that the following
sequence produces many fewer results:

  $ git clean -xfd tools/
  $ make tools/objtool
  $ grep "cannot find fixdep" $(find tools/objtool -name '*.cmd')

In particular, only a buggy tools/objtool/libsubcmd/.fixdep.o.cmd
remains, due to circular dependencies of fixdep on itself.

Such incomplete .cmd files don't usually cause a direct problem, since
they're designed to fail "open", but they can cause some subtle problems
that would otherwise be handled by proper fixdep'd dependency files. [2]

[1] This problem is better described in commit abb26210a395 ("perf
tools: Force fixdep compilation at the start of the build"). I don't
apply its solution here, because additional recursive make can be a bit
of overkill.

[2] Example failure case:

  cp -arl linux-src linux-src2
  cd linux-src2
  make O=/path/to/out
  cd ../linux-src
  rm -rf ../linux-src2
  make O=/path/to/out

Previously, we'd see errors like:

  make[6]: *** No rule to make target
  '/path/to/linux-src2/tools/include/linux/compiler.h', needed by
  '/path/to/out/tools/bpf/resolve_btfids/libsubcmd/exec-cmd.o'.  Stop.

Now, the properly-fixdep'd .cmd files will ignore a missing
/path/to/linux-src2/...

Link: https://lore.kernel.org/all/ZGVi9HbI43R5trN8@bhelgaas/
Link: https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/
Signed-off-by: Brian Norris <briannorris@chromium.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---

(no changes since v3)

Changes in v3:
 - update notes about failure cases
 - add Jiri's Acked-by

 tools/lib/subcmd/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index b87213263a5e..59b09f280e49 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -76,7 +76,7 @@ include $(srctree)/tools/build/Makefile.include
 
 all: fixdep $(LIBFILE)
 
-$(SUBCMD_IN): FORCE
+$(SUBCMD_IN): fixdep FORCE
 	@$(MAKE) $(build)=libsubcmd
 
 $(LIBFILE): $(SUBCMD_IN)
-- 
2.45.2.993.g49e7a77208-goog

[PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Brian Norris]

The 'fixdep' tool is used to post-process dependency files for various
reasons, and it runs after every object file generation command. This
even includes 'fixdep' itself.

In Kbuild, this isn't actually a problem, because it uses a single
command to generate fixdep (a compile-and-link command on fixdep.c), and
afterward runs the fixdep command on the accompanying .fixdep.cmd file.

In tools/ builds (which notably is maintained separately from Kbuild),
fixdep is generated in several phases:

 1. fixdep.c -> fixdep-in.o
 2. fixdep-in.o -> fixdep

Thus, fixdep is not available in the post-processing for step 1, and
instead, we generate .cmd files that look like:

  ## from tools/objtool/libsubcmd/.fixdep.o.cmd
  # cannot find fixdep (/path/to/linux/tools/objtool/libsubcmd//fixdep)
  [...]

These invalid .cmd files are benign in some respects, but cause problems
in others (such as the linked reports).

Because the tools/ build system is rather complicated in its own right
(and pointedly different than Kbuild), I choose to simply open-code the
rule for building fixdep, and avoid the recursive-make indirection that
produces the problem in the first place.

Link: https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

(no changes since v3)

Changes in v3:
 - Drop unnecessary tools/build/Build

 tools/build/Build    |  3 ---
 tools/build/Makefile | 11 ++---------
 2 files changed, 2 insertions(+), 12 deletions(-)
 delete mode 100644 tools/build/Build

diff --git a/tools/build/Build b/tools/build/Build
deleted file mode 100644
index 76d1a4960973..000000000000
--- a/tools/build/Build
+++ /dev/null
@@ -1,3 +0,0 @@
-hostprogs := fixdep
-
-fixdep-y := fixdep.o
diff --git a/tools/build/Makefile b/tools/build/Makefile
index 17cdf01e29a0..fea3cf647f5b 100644
--- a/tools/build/Makefile
+++ b/tools/build/Makefile
@@ -43,12 +43,5 @@ ifneq ($(wildcard $(TMP_O)),)
 	$(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
 endif
 
-$(OUTPUT)fixdep-in.o: FORCE
-	$(Q)$(MAKE) $(build)=fixdep
-
-$(OUTPUT)fixdep: $(OUTPUT)fixdep-in.o
-	$(QUIET_LINK)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
-
-FORCE:
-
-.PHONY: FORCE
+$(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
+	$(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
-- 
2.45.2.993.g49e7a77208-goog

[PATCH v4 3/3] tools build: Correct bpf fixdep dependencies

[Brian Norris]

The dependencies in tools/lib/bpf/Makefile are incorrect. Before we
recurse to build $(BPF_IN_STATIC), we need to build its 'fixdep'
executable.

I can't use the usual shortcut from Makefile.include:

  <target>: <sources> fixdep

because its 'fixdep' target relies on $(OUTPUT), and $(OUTPUT) differs
in the parent 'make' versus the child 'make' -- so I imitate it via
open-coding.

I tweak a few $(MAKE) invocations while I'm at it, because
1. I'm adding a new recursive make; and
2. these recursive 'make's print spurious lines about files that are "up
   to date" (which isn't normally a feature in Kbuild subtargets) or
   "jobserver not available" (see [1])

I also need to tweak the assignment of the OUTPUT variable, so that
relative path builds work. For example, for 'make tools/lib/bpf', OUTPUT
is unset, and is usually treated as "cwd" -- but recursive make will
change cwd and so OUTPUT has a new meaning. For consistency, I ensure
OUTPUT is always an absolute path.

And $(Q) gets a backup definition in tools/build/Makefile.include,
because Makefile.include is sometimes included without
tools/build/Makefile, so the "quiet command" stuff doesn't actually work
consistently without it.

After this change, top-level builds result in an empty grep result from:

  $ grep 'cannot find fixdep' $(find tools/ -name '*.cmd')

[1] https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html
If we're not using $(MAKE) directly, then we need to use more '+'.

Signed-off-by: Brian Norris <briannorris@chromium.org>
Acked-by: Jiri Olsa <jolsa@kernel.org>
---

Changes in v4:
 - update tools/lib/bpf/.gitignore to exclude 'fixdep'
 - update tools/lib/bpf `make clean` target for fixdep
 - combine $(SHARED_OBJDIR) and $(STATIC_OBJDIR) rules

Changes in v3:
 - add Jiri's Acked-by

Changes in v2:
 - also fix libbpf shared library rules
 - ensure OUTPUT is always set, and always an absolute path
 - add backup $(Q) definition in tools/build/Makefile.include

 tools/build/Makefile.include | 12 +++++++++++-
 tools/lib/bpf/.gitignore     |  1 +
 tools/lib/bpf/Makefile       | 13 ++++++++++---
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/build/Makefile.include b/tools/build/Makefile.include
index 8dadaa0fbb43..0e4de83400ac 100644
--- a/tools/build/Makefile.include
+++ b/tools/build/Makefile.include
@@ -1,8 +1,18 @@
 # SPDX-License-Identifier: GPL-2.0-only
 build := -f $(srctree)/tools/build/Makefile.build dir=. obj
 
+# More than just $(Q), we sometimes want to suppress all command output from a
+# recursive make -- even the 'up to date' printout.
+ifeq ($(V),1)
+  Q ?=
+  SILENT_MAKE = +$(Q)$(MAKE)
+else
+  Q ?= @
+  SILENT_MAKE = +$(Q)$(MAKE) --silent
+endif
+
 fixdep:
-	$(Q)$(MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
+	$(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
 
 fixdep-clean:
 	$(Q)$(MAKE) -C $(srctree)/tools/build clean
diff --git a/tools/lib/bpf/.gitignore b/tools/lib/bpf/.gitignore
index 0da84cb9e66d..f02725b123b3 100644
--- a/tools/lib/bpf/.gitignore
+++ b/tools/lib/bpf/.gitignore
@@ -5,3 +5,4 @@ TAGS
 tags
 cscope.*
 /bpf_helper_defs.h
+fixdep
diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 2cf892774346..1b22f0f37288 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -108,6 +108,8 @@ MAKEOVERRIDES=
 
 all:
 
+OUTPUT ?= ./
+OUTPUT := $(abspath $(OUTPUT))/
 export srctree OUTPUT CC LD CFLAGS V
 include $(srctree)/tools/build/Makefile.include
 
@@ -141,7 +143,10 @@ all: fixdep
 
 all_cmd: $(CMD_TARGETS) check
 
-$(BPF_IN_SHARED): force $(BPF_GENERATED)
+$(SHARED_OBJDIR) $(STATIC_OBJDIR):
+	$(Q)mkdir -p $@
+
+$(BPF_IN_SHARED): force $(BPF_GENERATED) | $(SHARED_OBJDIR)
 	@(test -f ../../include/uapi/linux/bpf.h -a -f ../../../include/uapi/linux/bpf.h && ( \
 	(diff -B ../../include/uapi/linux/bpf.h ../../../include/uapi/linux/bpf.h >/dev/null) || \
 	echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'" >&2 )) || true
@@ -151,9 +156,11 @@ $(BPF_IN_SHARED): force $(BPF_GENERATED)
 	@(test -f ../../include/uapi/linux/if_xdp.h -a -f ../../../include/uapi/linux/if_xdp.h && ( \
 	(diff -B ../../include/uapi/linux/if_xdp.h ../../../include/uapi/linux/if_xdp.h >/dev/null) || \
 	echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/if_xdp.h' differs from latest version at 'include/uapi/linux/if_xdp.h'" >&2 )) || true
+	$(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= OUTPUT=$(SHARED_OBJDIR) $(SHARED_OBJDIR)fixdep
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(SHARED_OBJDIR) CFLAGS="$(CFLAGS) $(SHLIB_FLAGS)"
 
-$(BPF_IN_STATIC): force $(BPF_GENERATED)
+$(BPF_IN_STATIC): force $(BPF_GENERATED) | $(STATIC_OBJDIR)
+	$(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= OUTPUT=$(STATIC_OBJDIR) $(STATIC_OBJDIR)fixdep
 	$(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
 
 $(BPF_HELPER_DEFS): $(srctree)/tools/include/uapi/linux/bpf.h
@@ -263,7 +270,7 @@ install_pkgconfig: $(PC_FILE)
 
 install: install_lib install_pkgconfig install_headers
 
-clean:
+clean: fixdep-clean
 	$(call QUIET_CLEAN, libbpf) $(RM) -rf $(CMD_TARGETS)		     \
 		*~ .*.d .*.cmd LIBBPF-CFLAGS $(BPF_GENERATED)		     \
 		$(SHARED_OBJDIR) $(STATIC_OBJDIR)			     \
-- 
2.45.2.993.g49e7a77208-goog

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Jiri Olsa]

On Mon, Jul 15, 2024 at 01:32:43PM -0700, Brian Norris wrote:
> The 'fixdep' tool is used to post-process dependency files for various
> reasons, and it runs after every object file generation command. This
> even includes 'fixdep' itself.
> 
> In Kbuild, this isn't actually a problem, because it uses a single
> command to generate fixdep (a compile-and-link command on fixdep.c), and
> afterward runs the fixdep command on the accompanying .fixdep.cmd file.
> 
> In tools/ builds (which notably is maintained separately from Kbuild),
> fixdep is generated in several phases:
> 
>  1. fixdep.c -> fixdep-in.o
>  2. fixdep-in.o -> fixdep
> 
> Thus, fixdep is not available in the post-processing for step 1, and
> instead, we generate .cmd files that look like:
> 
>   ## from tools/objtool/libsubcmd/.fixdep.o.cmd
>   # cannot find fixdep (/path/to/linux/tools/objtool/libsubcmd//fixdep)
>   [...]
> 
> These invalid .cmd files are benign in some respects, but cause problems
> in others (such as the linked reports).
> 
> Because the tools/ build system is rather complicated in its own right
> (and pointedly different than Kbuild), I choose to simply open-code the
> rule for building fixdep, and avoid the recursive-make indirection that
> produces the problem in the first place.
> 
> Link: https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
>  - Drop unnecessary tools/build/Build

Acked-by: Jiri Olsa <jolsa@kernel.org>

so usually Arnaldo takes changes for tools/build, Arnaldo, could you please take a look?
but still there'are the tools/lib/bpf bits..

thanks,
jirka

> 
>  tools/build/Build    |  3 ---
>  tools/build/Makefile | 11 ++---------
>  2 files changed, 2 insertions(+), 12 deletions(-)
>  delete mode 100644 tools/build/Build
> 
> diff --git a/tools/build/Build b/tools/build/Build
> deleted file mode 100644
> index 76d1a4960973..000000000000
> --- a/tools/build/Build
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -hostprogs := fixdep
> -
> -fixdep-y := fixdep.o
> diff --git a/tools/build/Makefile b/tools/build/Makefile
> index 17cdf01e29a0..fea3cf647f5b 100644
> --- a/tools/build/Makefile
> +++ b/tools/build/Makefile
> @@ -43,12 +43,5 @@ ifneq ($(wildcard $(TMP_O)),)
>  	$(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
>  endif
>  
> -$(OUTPUT)fixdep-in.o: FORCE
> -	$(Q)$(MAKE) $(build)=fixdep
> -
> -$(OUTPUT)fixdep: $(OUTPUT)fixdep-in.o
> -	$(QUIET_LINK)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
> -
> -FORCE:
> -
> -.PHONY: FORCE
> +$(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
> +	$(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
> -- 
> 2.45.2.993.g49e7a77208-goog
> 
> 

Re: [PATCH v4 3/3] tools build: Correct bpf fixdep dependencies

[Andrii Nakryiko]

On Mon, Jul 15, 2024 at 1:37 PM Brian Norris <briannorris@chromium.org> wrote:
>
> The dependencies in tools/lib/bpf/Makefile are incorrect. Before we
> recurse to build $(BPF_IN_STATIC), we need to build its 'fixdep'
> executable.
>
> I can't use the usual shortcut from Makefile.include:
>
>   <target>: <sources> fixdep
>
> because its 'fixdep' target relies on $(OUTPUT), and $(OUTPUT) differs
> in the parent 'make' versus the child 'make' -- so I imitate it via
> open-coding.
>
> I tweak a few $(MAKE) invocations while I'm at it, because
> 1. I'm adding a new recursive make; and
> 2. these recursive 'make's print spurious lines about files that are "up
>    to date" (which isn't normally a feature in Kbuild subtargets) or
>    "jobserver not available" (see [1])
>
> I also need to tweak the assignment of the OUTPUT variable, so that
> relative path builds work. For example, for 'make tools/lib/bpf', OUTPUT
> is unset, and is usually treated as "cwd" -- but recursive make will
> change cwd and so OUTPUT has a new meaning. For consistency, I ensure
> OUTPUT is always an absolute path.
>
> And $(Q) gets a backup definition in tools/build/Makefile.include,
> because Makefile.include is sometimes included without
> tools/build/Makefile, so the "quiet command" stuff doesn't actually work
> consistently without it.
>
> After this change, top-level builds result in an empty grep result from:
>
>   $ grep 'cannot find fixdep' $(find tools/ -name '*.cmd')
>
> [1] https://www.gnu.org/software/make/manual/html_node/MAKE-Variable.html
> If we're not using $(MAKE) directly, then we need to use more '+'.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> ---
>

LGTM

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> Changes in v4:
>  - update tools/lib/bpf/.gitignore to exclude 'fixdep'
>  - update tools/lib/bpf `make clean` target for fixdep
>  - combine $(SHARED_OBJDIR) and $(STATIC_OBJDIR) rules
>
> Changes in v3:
>  - add Jiri's Acked-by
>
> Changes in v2:
>  - also fix libbpf shared library rules
>  - ensure OUTPUT is always set, and always an absolute path
>  - add backup $(Q) definition in tools/build/Makefile.include
>
>  tools/build/Makefile.include | 12 +++++++++++-
>  tools/lib/bpf/.gitignore     |  1 +
>  tools/lib/bpf/Makefile       | 13 ++++++++++---
>  3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/tools/build/Makefile.include b/tools/build/Makefile.include
> index 8dadaa0fbb43..0e4de83400ac 100644
> --- a/tools/build/Makefile.include
> +++ b/tools/build/Makefile.include
> @@ -1,8 +1,18 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  build := -f $(srctree)/tools/build/Makefile.build dir=. obj
>
> +# More than just $(Q), we sometimes want to suppress all command output from a
> +# recursive make -- even the 'up to date' printout.
> +ifeq ($(V),1)
> +  Q ?=
> +  SILENT_MAKE = +$(Q)$(MAKE)
> +else
> +  Q ?= @
> +  SILENT_MAKE = +$(Q)$(MAKE) --silent
> +endif
> +
>  fixdep:
> -       $(Q)$(MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
> +       $(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= $(OUTPUT)fixdep
>
>  fixdep-clean:
>         $(Q)$(MAKE) -C $(srctree)/tools/build clean
> diff --git a/tools/lib/bpf/.gitignore b/tools/lib/bpf/.gitignore
> index 0da84cb9e66d..f02725b123b3 100644
> --- a/tools/lib/bpf/.gitignore
> +++ b/tools/lib/bpf/.gitignore
> @@ -5,3 +5,4 @@ TAGS
>  tags
>  cscope.*
>  /bpf_helper_defs.h
> +fixdep
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 2cf892774346..1b22f0f37288 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -108,6 +108,8 @@ MAKEOVERRIDES=
>
>  all:
>
> +OUTPUT ?= ./
> +OUTPUT := $(abspath $(OUTPUT))/
>  export srctree OUTPUT CC LD CFLAGS V
>  include $(srctree)/tools/build/Makefile.include
>
> @@ -141,7 +143,10 @@ all: fixdep
>
>  all_cmd: $(CMD_TARGETS) check
>
> -$(BPF_IN_SHARED): force $(BPF_GENERATED)
> +$(SHARED_OBJDIR) $(STATIC_OBJDIR):
> +       $(Q)mkdir -p $@
> +
> +$(BPF_IN_SHARED): force $(BPF_GENERATED) | $(SHARED_OBJDIR)
>         @(test -f ../../include/uapi/linux/bpf.h -a -f ../../../include/uapi/linux/bpf.h && ( \
>         (diff -B ../../include/uapi/linux/bpf.h ../../../include/uapi/linux/bpf.h >/dev/null) || \
>         echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h'" >&2 )) || true
> @@ -151,9 +156,11 @@ $(BPF_IN_SHARED): force $(BPF_GENERATED)
>         @(test -f ../../include/uapi/linux/if_xdp.h -a -f ../../../include/uapi/linux/if_xdp.h && ( \
>         (diff -B ../../include/uapi/linux/if_xdp.h ../../../include/uapi/linux/if_xdp.h >/dev/null) || \
>         echo "Warning: Kernel ABI header at 'tools/include/uapi/linux/if_xdp.h' differs from latest version at 'include/uapi/linux/if_xdp.h'" >&2 )) || true
> +       $(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= OUTPUT=$(SHARED_OBJDIR) $(SHARED_OBJDIR)fixdep
>         $(Q)$(MAKE) $(build)=libbpf OUTPUT=$(SHARED_OBJDIR) CFLAGS="$(CFLAGS) $(SHLIB_FLAGS)"
>
> -$(BPF_IN_STATIC): force $(BPF_GENERATED)
> +$(BPF_IN_STATIC): force $(BPF_GENERATED) | $(STATIC_OBJDIR)
> +       $(SILENT_MAKE) -C $(srctree)/tools/build CFLAGS= LDFLAGS= OUTPUT=$(STATIC_OBJDIR) $(STATIC_OBJDIR)fixdep
>         $(Q)$(MAKE) $(build)=libbpf OUTPUT=$(STATIC_OBJDIR)
>
>  $(BPF_HELPER_DEFS): $(srctree)/tools/include/uapi/linux/bpf.h
> @@ -263,7 +270,7 @@ install_pkgconfig: $(PC_FILE)
>
>  install: install_lib install_pkgconfig install_headers
>
> -clean:
> +clean: fixdep-clean
>         $(call QUIET_CLEAN, libbpf) $(RM) -rf $(CMD_TARGETS)                 \
>                 *~ .*.d .*.cmd LIBBPF-CFLAGS $(BPF_GENERATED)                \
>                 $(SHARED_OBJDIR) $(STATIC_OBJDIR)                            \
> --
> 2.45.2.993.g49e7a77208-goog
>
>

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Andrii Nakryiko]

On Tue, Jul 16, 2024 at 12:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jul 15, 2024 at 01:32:43PM -0700, Brian Norris wrote:
> > The 'fixdep' tool is used to post-process dependency files for various
> > reasons, and it runs after every object file generation command. This
> > even includes 'fixdep' itself.
> >
> > In Kbuild, this isn't actually a problem, because it uses a single
> > command to generate fixdep (a compile-and-link command on fixdep.c), and
> > afterward runs the fixdep command on the accompanying .fixdep.cmd file.
> >
> > In tools/ builds (which notably is maintained separately from Kbuild),
> > fixdep is generated in several phases:
> >
> >  1. fixdep.c -> fixdep-in.o
> >  2. fixdep-in.o -> fixdep
> >
> > Thus, fixdep is not available in the post-processing for step 1, and
> > instead, we generate .cmd files that look like:
> >
> >   ## from tools/objtool/libsubcmd/.fixdep.o.cmd
> >   # cannot find fixdep (/path/to/linux/tools/objtool/libsubcmd//fixdep)
> >   [...]
> >
> > These invalid .cmd files are benign in some respects, but cause problems
> > in others (such as the linked reports).
> >
> > Because the tools/ build system is rather complicated in its own right
> > (and pointedly different than Kbuild), I choose to simply open-code the
> > rule for building fixdep, and avoid the recursive-make indirection that
> > produces the problem in the first place.
> >
> > Link: https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> >
> > (no changes since v3)
> >
> > Changes in v3:
> >  - Drop unnecessary tools/build/Build
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> so usually Arnaldo takes changes for tools/build, Arnaldo, could you please take a look?
> but still there'are the tools/lib/bpf bits..

I think it should be fine for libbpf bits to go through Arnaldo's tree
and get back to bpf-next eventually. Unlikely that we'll have any
conflict in libbpf's Makefile specifically, we rarely change it.

>
> thanks,
> jirka
>
> >
> >  tools/build/Build    |  3 ---
> >  tools/build/Makefile | 11 ++---------
> >  2 files changed, 2 insertions(+), 12 deletions(-)
> >  delete mode 100644 tools/build/Build
> >
> > diff --git a/tools/build/Build b/tools/build/Build
> > deleted file mode 100644
> > index 76d1a4960973..000000000000
> > --- a/tools/build/Build
> > +++ /dev/null
> > @@ -1,3 +0,0 @@
> > -hostprogs := fixdep
> > -
> > -fixdep-y := fixdep.o
> > diff --git a/tools/build/Makefile b/tools/build/Makefile
> > index 17cdf01e29a0..fea3cf647f5b 100644
> > --- a/tools/build/Makefile
> > +++ b/tools/build/Makefile
> > @@ -43,12 +43,5 @@ ifneq ($(wildcard $(TMP_O)),)
> >       $(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
> >  endif
> >
> > -$(OUTPUT)fixdep-in.o: FORCE
> > -     $(Q)$(MAKE) $(build)=fixdep
> > -
> > -$(OUTPUT)fixdep: $(OUTPUT)fixdep-in.o
> > -     $(QUIET_LINK)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
> > -
> > -FORCE:
> > -
> > -.PHONY: FORCE
> > +$(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
> > +     $(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
> > --
> > 2.45.2.993.g49e7a77208-goog
> >
> >
>

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Brian Norris]

Hi Arnaldo,

On Tue, Jul 16, 2024 at 09:55:44AM +0200, Jiri Olsa wrote:
> so usually Arnaldo takes changes for tools/build, Arnaldo, could you please take a look?
> but still there'are the tools/lib/bpf bits..

Would you have some time to look at this series and whether it's ready
to be applied? Several folks have already acked one or more patches.

In case you've lost context on the series, here's a lore link for the
cover letter:

  [PATCH v4 0/3] tools build: Incorrect fixdep dependencies
  https://lore.kernel.org/all/20240715203325.3832977-1-briannorris@chromium.org/

Thanks,
Brian

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Arnaldo Carvalho de Melo]

On Fri, Jul 19, 2024 at 11:32:08AM -0700, Andrii Nakryiko wrote:
> On Tue, Jul 16, 2024 at 12:55 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Mon, Jul 15, 2024 at 01:32:43PM -0700, Brian Norris wrote:
> > > The 'fixdep' tool is used to post-process dependency files for various
> > > reasons, and it runs after every object file generation command. This
> > > even includes 'fixdep' itself.
> > >
> > > In Kbuild, this isn't actually a problem, because it uses a single
> > > command to generate fixdep (a compile-and-link command on fixdep.c), and
> > > afterward runs the fixdep command on the accompanying .fixdep.cmd file.
> > >
> > > In tools/ builds (which notably is maintained separately from Kbuild),
> > > fixdep is generated in several phases:
> > >
> > >  1. fixdep.c -> fixdep-in.o
> > >  2. fixdep-in.o -> fixdep
> > >
> > > Thus, fixdep is not available in the post-processing for step 1, and
> > > instead, we generate .cmd files that look like:
> > >
> > >   ## from tools/objtool/libsubcmd/.fixdep.o.cmd
> > >   # cannot find fixdep (/path/to/linux/tools/objtool/libsubcmd//fixdep)
> > >   [...]
> > >
> > > These invalid .cmd files are benign in some respects, but cause problems
> > > in others (such as the linked reports).
> > >
> > > Because the tools/ build system is rather complicated in its own right
> > > (and pointedly different than Kbuild), I choose to simply open-code the
> > > rule for building fixdep, and avoid the recursive-make indirection that
> > > produces the problem in the first place.
> > >
> > > Link: https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/
> > > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > > ---
> > >
> > > (no changes since v3)
> > >
> > > Changes in v3:
> > >  - Drop unnecessary tools/build/Build
> >
> > Acked-by: Jiri Olsa <jolsa@kernel.org>
> >
> > so usually Arnaldo takes changes for tools/build, Arnaldo, could you please take a look?
> > but still there'are the tools/lib/bpf bits..
> 
> I think it should be fine for libbpf bits to go through Arnaldo's tree
> and get back to bpf-next eventually. Unlikely that we'll have any
> conflict in libbpf's Makefile specifically, we rarely change it.

I got this series now in perf-tools-next,

Thanks,

- Arnaldo

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Thorsten Leemhuis]

On 15.07.24 22:32, Brian Norris wrote:
> The 'fixdep' tool is used to post-process dependency files for various
> reasons, and it runs after every object file generation command. This
> even includes 'fixdep' itself.

Lo! TWIMC, this change broke my daily arm64 and x86_64 Fedora vanilla RPM
builds on all Fedora releases when it hit -next a few days ago. Reverting
it fixes the problem.

The problem is related to the RPM magic somehow, as building worked fine
when when I omitted stuff like "-specs=/usr/lib/rpm/redhat/redhat-
hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1" from the
make call. So the real problem might be that space somewhere.


This is how the build fails on x86_64:

+ /usr/bin/make -s 'HOSTCFLAGS=-O2  -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection   ' 'HOSTLDFLAGS=-Wl,-z,relro -Wl,--as-needed  -Wl,-z,pack-relative-relocs -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1 -specs=/usr/lib/rpm/redhat/redhat-package-notes ' ARCH=x86_64 'KCFLAGS= ' WITH_GCOV=0 -j2 bzImage
/usr/bin/ld: /tmp/ccMoR0Wr.o: relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIE
/usr/bin/ld: failed to set dynamic section sizes: bad value
collect2: error: ld returned 1 exit status
make[4]: *** [Makefile:47: /builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.x86_64/tools/objtool/fixdep] Error 1
make[3]: *** [/builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.x86_64/tools/build/Makefile.include:15: fixdep] Error 2
make[2]: *** [Makefile:73: objtool] Error 2
make[1]: *** [/builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.x86_64/Makefile:1361: tools/objtool] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:226: __sub-make] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.ZQfBFY (%build)


This is how it fails on arm64:

+ /usr/bin/make -s 'HOSTCFLAGS=-O2  -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -mbranch-protection=standard -fasynchronous-unwind-tables -fstack-clash-protection   ' 'HOSTLDFLAGS=-Wl,-z,relro -Wl,--as-needed   -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1 -specs=/usr/lib/rpm/redhat/redhat-package-notes ' ARCH=arm64 'KCFLAGS= ' WITH_GCOV=0 -j4 vmlinuz.efi
/usr/bin/ld: /tmp/ccRCv9ot.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `stderr@@GLIBC_2.17' which may bind externally can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: /tmp/ccRCv9ot.o(.text+0x8): unresolvable R_AARCH64_ADR_PREL_PG_HI21 relocation against symbol `stderr@@GLIBC_2.17'
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status
make[4]: *** [Makefile:47: /builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.aarch64/tools/bpf/resolve_btfids/fixdep] Error 1
make[3]: *** [/builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.aarch64/tools/build/Makefile.include:15: fixdep] Error 2
make[2]: *** [Makefile:76: bpf/resolve_btfids] Error 2
make[1]: *** [/builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.aarch64/Makefile:1362: tools/bpf/resolve_btfids] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:226: __sub-make] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.60qjsr (%build)


Full logs:
https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-40-x86_64/07900504-next-next-all/builder-live.log.gz
https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-40-aarch64/07900504-next-next-all/builder-live.log.gz

My ppc64le builds worked fine.
 
Ciao, Thorsten

> In Kbuild, this isn't actually a problem, because it uses a single
> command to generate fixdep (a compile-and-link command on fixdep.c), and
> afterward runs the fixdep command on the accompanying .fixdep.cmd file.
> 
> In tools/ builds (which notably is maintained separately from Kbuild),
> fixdep is generated in several phases:
> 
>  1. fixdep.c -> fixdep-in.o
>  2. fixdep-in.o -> fixdep
> 
> Thus, fixdep is not available in the post-processing for step 1, and
> instead, we generate .cmd files that look like:
> 
>   ## from tools/objtool/libsubcmd/.fixdep.o.cmd
>   # cannot find fixdep (/path/to/linux/tools/objtool/libsubcmd//fixdep)
>   [...]
> 
> These invalid .cmd files are benign in some respects, but cause problems
> in others (such as the linked reports).
> 
> Because the tools/ build system is rather complicated in its own right
> (and pointedly different than Kbuild), I choose to simply open-code the
> rule for building fixdep, and avoid the recursive-make indirection that
> produces the problem in the first place.
> 
> Link: https://lore.kernel.org/all/Zk-C5Eg84yt6_nml@google.com/
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
>  - Drop unnecessary tools/build/Build
> 
>  tools/build/Build    |  3 ---
>  tools/build/Makefile | 11 ++---------
>  2 files changed, 2 insertions(+), 12 deletions(-)
>  delete mode 100644 tools/build/Build
> 
> diff --git a/tools/build/Build b/tools/build/Build
> deleted file mode 100644
> index 76d1a4960973..000000000000
> --- a/tools/build/Build
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -hostprogs := fixdep
> -
> -fixdep-y := fixdep.o
> diff --git a/tools/build/Makefile b/tools/build/Makefile
> index 17cdf01e29a0..fea3cf647f5b 100644
> --- a/tools/build/Makefile
> +++ b/tools/build/Makefile
> @@ -43,12 +43,5 @@ ifneq ($(wildcard $(TMP_O)),)
>  	$(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
>  endif
>  
> -$(OUTPUT)fixdep-in.o: FORCE
> -	$(Q)$(MAKE) $(build)=fixdep
> -
> -$(OUTPUT)fixdep: $(OUTPUT)fixdep-in.o
> -	$(QUIET_LINK)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
> -
> -FORCE:
> -
> -.PHONY: FORCE
> +$(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
> +	$(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Brian Norris]

Hi Thorsten,

On Mon, Aug 12, 2024 at 08:32:29AM +0200, Thorsten Leemhuis wrote:
> Lo! TWIMC, this change broke my daily arm64 and x86_64 Fedora vanilla RPM
> builds on all Fedora releases when it hit -next a few days ago. Reverting
> it fixes the problem.
> 
> The problem is related to the RPM magic somehow, as building worked fine
> when when I omitted stuff like "-specs=/usr/lib/rpm/redhat/redhat-
> hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1" from the
> make call. So the real problem might be that space somewhere.
> 
> 
> This is how the build fails on x86_64:
> 
> + /usr/bin/make -s 'HOSTCFLAGS=-O2  -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection   ' 'HOSTLDFLAGS=-Wl,-z,relro -Wl,--as-needed  -Wl,-z,pack-relative-relocs -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -Wl,--build-id=sha1 -specs=/usr/lib/rpm/redhat/redhat-package-notes ' ARCH=x86_64 'KCFLAGS= ' WITH_GCOV=0 -j2 bzImage
> /usr/bin/ld: /tmp/ccMoR0Wr.o: relocation R_X86_64_32 against `.rodata' can not be used when making a PIE object; recompile with -fPIE
> /usr/bin/ld: failed to set dynamic section sizes: bad value
> collect2: error: ld returned 1 exit status
> make[4]: *** [Makefile:47: /builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.x86_64/tools/objtool/fixdep] Error 1
> make[3]: *** [/builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.x86_64/tools/build/Makefile.include:15: fixdep] Error 2
> make[2]: *** [Makefile:73: objtool] Error 2
> make[1]: *** [/builddir/build/BUILD/kernel-next-20240812/linux-6.11.0-0.0.next.20240812.329.vanilla.fc40.x86_64/Makefile:1361: tools/objtool] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:226: __sub-make] Error 2
> error: Bad exit status from /var/tmp/rpm-tmp.ZQfBFY (%build)

I don't have a Fedora installation on hand at the moment, and the logs
don't seem to include most of the actual kernel build logs
(stdout+stderr of a V=1 build might help), but I think what you've
provided so far has highlighted one possible problem -- that the new
one-shot compile+link is ignoring HOSTCFLAGS, which were previously
respected via tools/build/Build.include. Could you try the following
diff? I'll cook a proper patch and description later, but for now:

--- a/tools/build/Makefile
+++ b/tools/build/Makefile
@@ -44,4 +44,4 @@ ifneq ($(wildcard $(TMP_O)),)
 endif
 
 $(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
-	$(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
+	$(QUIET_CC)$(HOSTCC) $(HOSTCFLAGS) $(KBUILD_HOSTLDFLAGS) -o $@ $<

Re: [PATCH v4 2/3] tools build: Avoid circular .fixdep-in.o.cmd issues

[Thorsten Leemhuis]

On 13.08.24 18:40, Brian Norris wrote:
> On Mon, Aug 12, 2024 at 08:32:29AM +0200, Thorsten Leemhuis wrote:
>> Lo! TWIMC, this change broke my daily arm64 and x86_64 Fedora vanilla RPM
>> builds on all Fedora releases when it hit -next a few days ago. Reverting
>> it fixes the problem.
>>
>> The problem is related to the RPM magic somehow, as building worked fine
>> when when I omitted stuff like "-specs=/usr/lib/rpm/redhat/redhat-
>> hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1" from the
>> make call. So the real problem might be that space somewhere.
> [...]
> I don't have a Fedora installation on hand at the moment, and the logs
> don't seem to include most of the actual kernel build logs
> (stdout+stderr of a V=1 build might help), but I think what you've
> provided so far has highlighted one possible problem -- that the new
> one-shot compile+link is ignoring HOSTCFLAGS, which were previously
> respected via tools/build/Build.include. Could you try the following
> diff? I'll cook a proper patch and description later, but for now:
> 
> --- a/tools/build/Makefile
> +++ b/tools/build/Makefile
> @@ -44,4 +44,4 @@ ifneq ($(wildcard $(TMP_O)),)
>  endif
>  
>  $(OUTPUT)fixdep: $(srctree)/tools/build/fixdep.c
> -	$(QUIET_CC)$(HOSTCC) $(KBUILD_HOSTLDFLAGS) -o $@ $<
> +	$(QUIET_CC)$(HOSTCC) $(HOSTCFLAGS) $(KBUILD_HOSTLDFLAGS) -o $@ $<

Many thx for looking into this. Seems that does resolve the problem (I
did not perform a full build, but without this the build fails after a
few seconds, and now it ran through).

I don't care much, but feel free to add a

Tested-by: Thorsten Leemhuis <linux@leemhuis.info>

Thx again!

Ciao, Thorsten