Re: [PATCH bpf-next v9 10/11] selftests/bpf: Provide weak definitions for cross-test functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Test flakiness and deterministic failure due to Identical Code Folding (ICF) of weak stubs.
- [Low] Missing `<alloca.h>` include for `alloca()` in `testing_helpers.c`.
--

commit 45cfa2851c72a65543e44de19ed32088ef920e33
Author: Ricardo B. Marlière <rbm@suse.com>

selftests/bpf: Provide weak definitions for cross-test functions

This commit replaces forward declarations of uprobe_multi_func_{1,2,3}() with
weak no-op stubs to allow the linker to resolve them when skeletons are
missing. It also moves stack_mprotect() to testing_helpers.c to make it
universally available and resolve an issue where the previous weak stub
returned 0.

> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> index 35adc3f6d443..5a864cd8ad1b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c

[ ... ]

> -/* defined in prog_tests/uprobe_multi_test.c */
> -void uprobe_multi_func_1(void);
> -void uprobe_multi_func_2(void);
> -void uprobe_multi_func_3(void);
> +/*
> + * Weak uprobe target stubs. noinline is required because
> + * uprobe_multi_test_run() takes their addresses to configure the BPF
> + * program's attachment points; an inlined function has no stable
> + * address in the binary to probe. The strong definitions in
> + * uprobe_multi_test.c take precedence when that translation unit is
> + * linked.
> + */
> +noinline __weak void uprobe_multi_func_1(void) {}
> +noinline __weak void uprobe_multi_func_2(void) {}
> +noinline __weak void uprobe_multi_func_3(void) {}

Because these three weak stubs are structurally identical empty functions,
can compiler and linker optimization passes like Identical Code Folding
(ICF) merge them into a single memory address?

If they are folded into a single address, bpf_program__attach_uprobe_multi()
will attach three separate uprobes to the exact same offset, each with a
different cookie.

Calling any of the functions would then execute the shared address,
triggering all three uprobes simultaneously. This would increment the BPF
program counters multiple times per call, causing assertions like:

    ASSERT_EQ(skel->bss->uprobe_multi_func_1_result, 1, ...);

to fail deterministically, as the result would be 3 instead of 1.

Would it make sense to make these stubs structurally unique, perhaps by
adding unique inline assembly to each?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
> index 6fbe1e995660..3f7ae76c209b 100644
> --- a/tools/testing/selftests/bpf/testing_helpers.c
> +++ b/tools/testing/selftests/bpf/testing_helpers.c

[ ... ]

> +int stack_mprotect(void)
> +{
> +	void *buf;
> +	long sz;
> +	int ret;
> +
> +	sz = sysconf(_SC_PAGESIZE);
> +	if (sz < 0)
> +		return sz;
> +
> +	buf = alloca(sz * 3);

Does this code rely on a glibc-specific extension to provide alloca() via
<stdlib.h>?

The original implementation in test_lsm.c included <malloc.h>, but the new
location in testing_helpers.c includes neither <malloc.h> nor <alloca.h>.

On systems with strict POSIX-compliant C libraries, such as musl, omitting
<alloca.h> can result in an implicit function declaration error and lead
to build failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260429-selftests-bpf_misconfig-v9-0-c311f06b4791@suse.com?part=10