Re: [PATCH v1 11/14] tools build: Integrate libdebuginfod into test-all fast path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Short-circuiting `feature_check` prevents necessary re-evaluation of features falsely assumed present by `test-all`, causing fatal build failures.
- [Low] Unconditionally adding `-ldebuginfod` to `BUILD_ALL` breaks the `test-all` fast path even when explicitly disabled via `NO_LIBDEBUGINFOD=1`.
--

commit 53a01ad9ae3a2bcd156b498b52efecdd21be5e21
Author: Ian Rogers <irogers@google.com>

tools build: Integrate libdebuginfod into test-all fast path

This commit bundles libdebuginfod into test-all.c to avoid an individual
feature check sub-make fork. It short-circuits feature_check in
Makefile.feature to skip evaluations when a feature was pre-detected.

> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 2f192d3bf61b9..d8febe5d58005 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -8,7 +8,9 @@ endif
>  
>  feature_check = $(eval $(feature_check_code))
>  define feature_check_code
> -  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CC="$(CC)" CXX="$(CXX)" CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> +  ifeq ($(feature-$(1)),)
> +    feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CC="$(CC)" CXX="$(CXX)" CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) $(OUTPUT_FEATURES)test-$1.bin >/dev/null 2>/dev/null && echo 1 || echo 0)
> +  endif
>  endef

Does this short-circuit prevent the necessary evaluation of features omitted
from test-all?

When test-all.bin successfully compiles, Makefile.feature sets all features in
FEATURE_TESTS to 1. If features like libbfd or libcpupower are intentionally
omitted from test-all.c, they are falsely assumed present.

By skipping the evaluation when feature-$(1) is already set, the explicit
correction is missed. Could this cause fatal linker errors on systems that
lack these libraries?

[ ... ]

> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 704c687ed3ad1..dc828523b50ad 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -115,7 +115,7 @@ __BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.c,$(@F)) $(
>    BUILD_BFD = $(BUILD) -DPACKAGE='"perf"' -lbfd -ldl
>    BUILD_ALL = $(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -lslang \
>  	      $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -ldl -lz -llzma -lzstd \
> -	      $(shell $(PKG_CONFIG) --libs --cflags openssl 2>/dev/null)
> +	      $(shell $(PKG_CONFIG) --libs --cflags openssl 2>/dev/null) -ldebuginfod
>  
>  __BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS)

Will adding -ldebuginfod unconditionally here break the test-all fast path
even when NO_LIBDEBUGINFOD=1 is explicitly passed?

If a system lacks libdebuginfod-dev but the build is correctly configured to
opt out, it appears BUILD_ALL will still attempt to link against -ldebuginfod
and fail, forcing a slow fallback to individual feature tests.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512053539.3410189-1-irogers@google.com?part=11