Re: [PATCH v1 1/7] tools build: Add feature check for elfutils libasm

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `ASMLIBS` definition lacks required dependencies (`-lebl` and `-ldl`) for static builds and older `elfutils` versions, and relies on an unexported symbol in newer versions, guaranteeing spurious feature test failures.
- [Medium] Passing `NULL` to the third parameter of `disasm_begin` triggers a `-Wint-conversion` error, spuriously failing the feature test.
--

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

tools build: Add feature check for elfutils libasm

This commit introduces a build-time feature test for the elfutils libasm
library in the shared tools/build system. It adds the test program and 
updates the Makefiles to attempt building it to detect support.

> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 62909a9c799d3..f0f39a2ab2035 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
[ ... ]
> @@ -184,6 +185,11 @@ ifeq ($(findstring -static,${LDFLAGS}),-static)
>    DWARFLIBS += -ldl
>  endif
>  
> +ASMLIBS := -lasm -ldw -lelf
> +ifeq ($(findstring -static,${LDFLAGS}),-static)
> +  ASMLIBS += -lz -llzma -lbz2 -lzstd
> +endif

[Severity: Medium]
Does this ASMLIBS definition miss required legacy dependencies? For static 
builds on older elfutils versions, omitting -lebl and -ldl here might cause 
the feature test to fail to link, silently disabling the libasm feature.

> diff --git a/tools/build/feature/test-libasm.c b/tools/build/feature/test-libasm.c
> new file mode 100644
> index 0000000000000..f3598ad1cc9e1
> --- /dev/null
> +++ b/tools/build/feature/test-libasm.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stddef.h>
> +#include <elfutils/libasm.h>
> +#include <gelf.h>
> +
> +typedef struct ebl Ebl;
> +extern Ebl *ebl_openbackend (Elf *elf);
> +extern void ebl_closebackend (Ebl *ebl);
> +
> +int main(void)
> +{
> +	Elf *elf = elf_begin(0, ELF_C_READ, NULL);
> +	Ebl *ebl = ebl_openbackend(elf);

[Severity: Medium]
Is it safe to rely on ebl_openbackend() here? Modern versions of elfutils 
(>= 0.177) do not export ebl_openbackend dynamically. Could this cause 
unintended linker errors and spuriously fail the feature test?

> +	DisasmCtx_t *ctx = disasm_begin(ebl, elf, NULL);

[Severity: Medium]
Does passing NULL as the third parameter here trigger a -Wint-conversion 
compiler warning? Since the feature test is compiled with -Werror, expecting
an integer or boolean but receiving a pointer will cause a build failure 
and silently disable the libasm feature.

> +	disasm_end(ctx);
> +	ebl_closebackend(ebl);
> +	elf_end(elf);
> +	return 0;
> +}

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