Re: [PATCH v4 05/10] selftests/bpf: Tolerate test file compilation failures

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Masking test compilation failures breaks the linking step and aborts the test runner, completely breaking the test suite.
--

--- Patch [5]: [PATCH v4 05/10] selftests/bpf: Tolerate test file compilation failures ---
commit fa90c61b664da4eb761a6d8759626a636f99483a
Author: Ricardo B. Marlière <[email protected]>

selftests/bpf: Tolerate test file compilation failures

This commit modifies the BPF selftests Makefile to make individual test
compilation non-fatal. It removes the output object on failure, prints a
SKIP-TEST marker, and skips BTFIDS post-processing for absent objects.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 72f576a8236a1..e67f9c4cb5d70 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -688,11 +688,14 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o:			\
>  		      $(TRUNNER_TESTS_DIR)/%.c				\
>  		      | $(TRUNNER_OUTPUT)/%.test.d
>  	$$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
> -	$(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) -MMD -MT $$@ -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> +	$(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) -MMD -MT $$@ -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F) || \
> +		($(RM) $$(@F); printf '  %-12s %s\n' 'SKIP-TEST' '$$(notdir $$@)' 1>&2)

[Severity: High]
Will masking the compilation failure here break the subsequent linking step
and the test runner?

Because Make evaluates $$^ in the linker command line, it expands to all
defined .test.o prerequisites, including the missing ones. The missing
.test.o file is then passed to the compiler during linking, which fails with a
'No such file or directory' error and halts the build.

Furthermore, even if the linker step is bypassed by filtering out missing
files, the test runner itself aborts execution entirely if a test function
is NULL:

tools/testing/selftests/bpf/test_progs.c:main() {
    ...
    if ((test->run_test == NULL && test->run_serial_test == NULL) ||
        (test->run_test != NULL && test->run_serial_test != NULL)) {
        fprintf(stderr, "Test %d:%s must have either test_%s() or "
                "serial_test_%sl() defined.\n", test->test_num,
                test->test_name, test->test_name, test->test_name);
        exit(EXIT_ERR_SETUP_INFRA);
    }
    ...
}

When test_progs initializes, it enumerates all tests by scanning .c files.
The missing .test.o file causes its weak function symbol to resolve to NULL,
aborting the entire test suite on startup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5