On Wed, May 27, 2026 at 01:32:48PM +0200, Jakub Sitnicki wrote:
> 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 <[email protected]>
> > ---
> > .../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(...)
ok, safes space ;-)
>
> >
> > -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().
would need to check the existing code.. let's keep it
>
> > {
> > 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.
right, we can add extra check that there's int3 in here (and below)
thanks,
jirka
