Re: [PATCHv4 11/13] selftests/bpf: Add reattach tests for uprobe syscall

[Jakub Sitnicki <jakub@cloudflare.com>]

On Tue, May 26, 2026 at 10:58 PM +02, Jiri Olsa wrote:
> Adding reattach tests for uprobe syscall tests to make sure
> we can re-attach and optimize same uprobe multiple times.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  .../selftests/bpf/prog_tests/uprobe_syscall.c | 114 ++++++++++++++++--
>  1 file changed, 104 insertions(+), 10 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> index 0868fb9793e0..969f4deba9fd 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> @@ -430,23 +430,28 @@ static void *check_attach(struct uprobe_syscall_executed *skel, trigger_t trigge
>  	return tramp;
>  }
>  
> -static void check_detach(void *addr, void *tramp)
> +static bool check_detach(void *addr, void *tramp)
>  {
>  	static const char nop10_prefix[] = { 0x66, 0x2e, 0x0f, 0x1f, 0x84 };
> +	bool ok = true;
>  
>  	/* [uprobes_trampoline] stays after detach */
> -	ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline");
> -	ASSERT_OK(memcmp(addr, nop10_prefix, 5), "nop10_prefix");
> +	if (!ASSERT_OK(find_uprobes_trampoline(tramp), "uprobes_trampoline"))
> +		ok = false;
> +	if (!ASSERT_OK(memcmp(addr, nop10_prefix, 5), "nop10_prefix"))
> +		ok = false;
> +	return ok;
>  }

Nit: Maybe apply the same pattern you used in
progs/get_func_args_test.c?

ok &= ASSERT_OK(...)
ok &= ASSERT_OK(...)

>  
> -static void check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
> -		  trigger_t trigger, void *addr, int executed)
> +static void *check(struct uprobe_syscall_executed *skel, struct bpf_link *link,
> +		   trigger_t trigger, void *addr, int executed)

Nit: Kinda wish that was called check_attach_detach().

>  {
>  	void *tramp;
>  
>  	tramp = check_attach(skel, trigger, addr, executed);
>  	bpf_link__destroy(link);
>  	check_detach(addr, tramp);
> +	return tramp;
>  }
>  
>  static void test_uprobe_legacy(void)
> @@ -457,6 +462,7 @@ static void test_uprobe_legacy(void)
>  	);
>  	struct bpf_link *link;
>  	unsigned long offset;
> +	void *tramp;
>  
>  	offset = get_uprobe_offset(&uprobe_test);
>  	if (!ASSERT_GE(offset, 0, "get_uprobe_offset"))
> @@ -474,7 +480,28 @@ static void test_uprobe_legacy(void)
>  	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
>  		goto cleanup;
>  
> -	check(skel, link, uprobe_test, uprobe_test, 2);
> +	tramp = check(skel, link, uprobe_test, uprobe_test, 2);
> +
> +	/* reattach and detach without triggering optimization */
> +	link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
> +					       0, "/proc/self/exe", offset, NULL);
> +	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
> +		goto cleanup;
> +

In theory we're missing a check here that an unoptimized uprobe was
installed. If nothing happened at all between the last check() and
check_destroy() below, the test would still pass.

Applies to the three similar changes after that one as well.

> +	bpf_link__destroy(link);
> +	if (!check_detach(uprobe_test, tramp))
> +		goto cleanup;
> +
> +	uprobe_test();
> +	ASSERT_EQ(skel->bss->executed, 2, "executed_no_probe");
> +
> +	/* reattach with triggering optimization */
> +	link = bpf_program__attach_uprobe_opts(skel->progs.test_uprobe,
> +				0, "/proc/self/exe", offset, NULL);
> +	if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_opts"))
> +		goto cleanup;
> +
> +	check(skel, link, uprobe_test, uprobe_test, 4);
>  
>  	/* uretprobe */
>  	skel->bss->executed = 0;

[...]