On Thu, Sep 17, 2026 at 11:42:50AM -0700, Kees Cook wrote: > On Thu, Sep 17, 2026 at 05:06:19PM +0100, Lorenzo Stoakes (ARM) wrote: > > This can be done faster in C, so implement scripts/basic/depcheck to do so. > > > > It does as little work as possible, reading the .cmd files from a > > directory's targets and running stat on each dependency only a single time. > > Doesn't this run the risk of missing transitive deps? > > C.cmd: C.o depends on A.c and B.c and B.h > B.cmd: B.h depends on B.data and B.script > > depcheck looks at C.o and B.h's times and is happy so it drops C.cmd file, > but then see B.script has changed compared to B.h, so it keeps B.cmd, > and then the build runs and doesn't have the C.o dep list any more, > and C.o goes unbuilt? > > Also, I don't think this handles if_changed commands at all? The kbuild > rebuild condition is timestamps plus if_changed's command-line check.
tl;dr: I spent way too long looking at this. I think my conclusion is "Hm, this makes some cases of missed Makefile deps on generated files harder to find, but there don't *appear* to be any obviously wrong instances of this in the tree." Long version: There does appear to be a problem in the case of generated targets where the Makefile deps are build-order correct but lack explicit deps. As in, a clean build generates the needed dependencies due to some prior explicit Makefile rule dependency, and a later source-level "#include" for the generated file exists (and the build doesn't fail since the included file got generated before the source that "#include"d it got built), and then the normal dep tracking (-Wp,-MMD,... -> .cmd) catches it and any direct changes to that dep would normally get noticed going forward on incremental builds. However, with the depcheck .cmd pruning, changes to transitive deps (either via file contents or Kconfig options) will go unnoticed during incremental builds without that explicit dep (but stock doesn't miss it). Though actually it's kind of worse because it'll get noticed on rebuild #N+1 for N level of transitive depth, so something will break, and then you build again, and the changes from 2 builds ago suddenly get rebuilt... I think you maybe encountered an instance of this in your series, too, but it got exposed due to a clean build no longer having the ordering correct: https://lore.kernel.org/lkml/[email protected]/ See the trailing diff for a demo: git apply demo.diff make O=b defconfig # simulate a earlier-stage header generation... make O=b lib/byfile_anchor.o lib/byconf_anchor.o make O=b lib/ # trigger 1: the header's input file changes echo FROMFILE-two > lib/byfile_gen.in make O=b lib/ # trigger 2: a Kconfig value changes; no file is touched ./scripts/config --file b/.config --set-str LOCALVERSION -demo2 make O=b lib/ # what each object ended up holding strings b/lib/byfile_anchor.o | grep FROMFILE- # FROMFILE-two both trees strings b/lib/byfile_victim.o | grep FROMFILE- # FROMFILE-two stock # FROMFILE-one patched <-- stale strings b/lib/byconf_anchor.o | grep CONF- # CONF--demo2 both trees strings b/lib/byconf_victim.o | grep CONF- # CONF--demo2 stock # CONF- patched <-- stale This means adding depcheck would silently expose any current (and future) missing explicit deps on generated files where those files get generated by either an earlier build stage or an earlier rule. :( I went looking for the kind of missed Makefile dep for a generated file, and while it's not a very grep-able condition, I did look at stuff that fell into include/generated/, but it's safe due to the prepare/archprepare ordering AFAICT. So then I looked at places where -I had $(obj) added to it, and all of those seemed to have explicit Makefile deps too. Well, all except for arch/x86/boot/compressed/sev-handle-vc.c which includes "../../lib/inat.c" but that seems safe today due to ordering from arch/x86/lib/ being needed before arch/x86/boot/compressed/. So, yeah, it makes me nervous, and it may make incremental builds less idempotent. But I can't really find extant problems and the demo is slightly contrived, but not exactly an impossible situation. -Kees diff --git a/lib/Makefile b/lib/Makefile index dfab958327c5c..aed0bd9eb7d6a 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -350,3 +350,36 @@ CONTEXT_ANALYSIS_test_context-analysis.o := y obj-$(CONFIG_CONTEXT_ANALYSIS_TEST) += test_context-analysis.o subdir-$(CONFIG_FORTIFY_SOURCE) += test_fortify + +# --- depcheck demo --- +obj-y += byfile_anchor.o byfile_victim.o byconf_anchor.o byconf_victim.o + +quiet_cmd_byfile = GENFILE $@ + cmd_byfile = printf '\#define BYFILE_STR "%s"\n' "$$(cat $<)" > $@ + +quiet_cmd_byconf = GENCONF $@ + cmd_byconf = printf '\#define BYCONF_STR "CONF-%s"\n' '$(CONFIG_LOCALVERSION)' > $@ + +# Regenerated when its input file changes. +$(obj)/byfile_gen.h: $(src)/byfile_gen.in FORCE + $(call if_changed,byfile) + +# No input file: regenerated only when its command line changes, which here +# means when CONFIG_LOCALVERSION changes. No file is touched. +$(obj)/byconf_gen.h: FORCE + $(call if_changed,byconf) + +targets += byfile_gen.h byconf_gen.h + +# The anchors declare the dependency. That is what generates the headers on +# a clean build: fixdep only records a header once the object has compiled +# successfully, so on a first build there is no .cmd file to rely on. The +# victims include the same headers without declaring them, so the only +# record of that edge is the deps_ list in their .cmd files. +$(obj)/byfile_anchor.o: $(obj)/byfile_gen.h +$(obj)/byconf_anchor.o: $(obj)/byconf_gen.h + +CFLAGS_byfile_anchor.o += -I$(obj) +CFLAGS_byfile_victim.o += -I$(obj) +CFLAGS_byconf_anchor.o += -I$(obj) +CFLAGS_byconf_victim.o += -I$(obj) diff --git a/lib/byconf_anchor.c b/lib/byconf_anchor.c new file mode 100644 index 0000000000000..8035eaa352a3b --- /dev/null +++ b/lib/byconf_anchor.c @@ -0,0 +1,3 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "byconf_gen.h" +const char byconf_anchor_marker[] = BYCONF_STR; diff --git a/lib/byconf_victim.c b/lib/byconf_victim.c new file mode 100644 index 0000000000000..285a8de8bdf6e --- /dev/null +++ b/lib/byconf_victim.c @@ -0,0 +1,3 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "byconf_gen.h" +const char byconf_victim_marker[] = BYCONF_STR; diff --git a/lib/byfile_anchor.c b/lib/byfile_anchor.c new file mode 100644 index 0000000000000..9ea0b8ad55963 --- /dev/null +++ b/lib/byfile_anchor.c @@ -0,0 +1,3 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "byfile_gen.h" +const char byfile_anchor_marker[] = BYFILE_STR; diff --git a/lib/byfile_gen.in b/lib/byfile_gen.in new file mode 100644 index 0000000000000..7da9e5e0cd83c --- /dev/null +++ b/lib/byfile_gen.in @@ -0,0 +1 @@ +FROMFILE-one diff --git a/lib/byfile_victim.c b/lib/byfile_victim.c new file mode 100644 index 0000000000000..763c5ef8f1514 --- /dev/null +++ b/lib/byfile_victim.c @@ -0,0 +1,3 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "byfile_gen.h" +const char byfile_victim_marker[] = BYFILE_STR; -- Kees Cook
