On Mon, Nov 20, 2023 at 10:22:26AM -0800, Yonghong Song wrote:
>
> On 11/20/23 9:56 AM, Jiri Olsa wrote:
> > Adding fill_link_info test for uprobe_multi link.
> >
> > Setting up uprobes with bogus ref_ctr_offsets and cookie values
> > to test all the bpf_link_info::uprobe_multi fields.
> >
> > Acked-by: Song Liu <song@xxxxxxxxxx>
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> > .../selftests/bpf/prog_tests/fill_link_info.c | 191 ++++++++++++++++++
> > .../selftests/bpf/progs/test_fill_link_info.c | 6 +
> > 2 files changed, 197 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> > index 9294cb8d7743..fdf2c6b8c0cf 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/fill_link_info.c
> > @@ -7,6 +7,7 @@
> > #include <test_progs.h>
> > #include "trace_helpers.h"
> > #include "test_fill_link_info.skel.h"
> > +#include "bpf/libbpf_internal.h"
> > #define TP_CAT "sched"
> > #define TP_NAME "sched_switch"
> > @@ -300,6 +301,189 @@ static void test_kprobe_multi_fill_link_info(struct test_fill_link_info *skel,
> > bpf_link__destroy(link);
> > }
> > +/* Initialize semaphore variables so they don't end up in bss
> > + * section and we could get retrieve their offsets.
> > + */
> > +static short uprobe_link_info_sema_1 = 1;
> > +static short uprobe_link_info_sema_2 = 1;
> > +static short uprobe_link_info_sema_3 = 1;
>
> I guess The typical sema value starting value should be 0, right?
> If this is the case, the above is not a good example.
> So the issue is that current libbpf does not support
> retrieving offset from .bss section? Do you know why?
hum, I can't recall why it was the problem, because it seems to work
with .bss now when I try it ... anyway I think your suggestion below
is better
>
> In selftest udst.c, we have semaphore defined as
> usdt.c:unsigned short test_usdt0_semaphore SEC(".probes");
> usdt.c:unsigned short test_usdt3_semaphore SEC(".probes");
> usdt.c:unsigned short test_usdt12_semaphore SEC(".probes");
>
> Will the following work?
> static short uprobe_link_info_sema_1 SEC(".probes");
yes, that will work and it's better
> ...
>
> > +
> > +noinline void uprobe_link_info_func_1(void)
> > +{
> > + uprobe_link_info_sema_1++;
> > + asm volatile ("");
>
> The 'asm volatile' above intends to prevent compiler from
> doing 'implicit' inlining. So as a convention let us
> switch statement order to
>
> asm volatile ("");
> uprobe_link_info_sema_1++;
>
> Similarly for below.
ok
SNIP
> > +static void test_uprobe_multi_fill_link_info(struct test_fill_link_info *skel,
> > + bool retprobe, bool invalid)
> > +{
> > + LIBBPF_OPTS(bpf_uprobe_multi_opts, opts,
> > + .retprobe = retprobe,
> > + );
> > + const char *syms[3] = {
> > + "uprobe_link_info_func_1",
> > + "uprobe_link_info_func_2",
> > + "uprobe_link_info_func_3",
> > + };
> > + __u64 cookies[3] = {
> > + 0xdead,
> > + 0xbeef,
> > + 0xcafe,
> > + };
> > + const char *sema[3] = {
> > + "uprobe_link_info_sema_1",
> > + "uprobe_link_info_sema_2",
> > + "uprobe_link_info_sema_3",
> > + };
> > + __u64 *offsets, *ref_ctr_offsets;
> > + struct bpf_link *link;
> > + int link_fd, err;
> > +
> > + err = elf_resolve_syms_offsets("/proc/self/exe", 3, sema,
> > + (unsigned long **) &ref_ctr_offsets, STT_OBJECT);
> > + if (!ASSERT_OK(err, "elf_resolve_syms_offsets_object"))
> > + return;
> > +
> > + err = elf_resolve_syms_offsets("/proc/self/exe", 3, syms,
> > + (unsigned long **) &offsets, STT_FUNC);
> > + if (!ASSERT_OK(err, "elf_resolve_syms_offsets_func"))
> > + return;
>
> potential leak of ref_ctr_offsets?
ugh yep, will fix
>
> > +
> > + opts.syms = syms;
> > + opts.cookies = &cookies[0];
> > + opts.ref_ctr_offsets = (unsigned long *) &ref_ctr_offsets[0];
> > + opts.cnt = ARRAY_SIZE(syms);
> > +
> > + link = bpf_program__attach_uprobe_multi(skel->progs.umulti_run, 0,
> > + "/proc/self/exe", NULL, &opts);
> > + if (!ASSERT_OK_PTR(link, "bpf_program__attach_uprobe_multi"))
> > + goto out;
> > +
> > + link_fd = bpf_link__fd(link);
> > + if (invalid)
> > + verify_umulti_invalid_user_buffer(link_fd);
> > + else
> > + verify_umulti_link_info(link_fd, retprobe, offsets, cookies, ref_ctr_offsets);
> > +
> > + bpf_link__destroy(link);
> > +out:
> > + free(offsets);
>
> Should we free ref_ctr_offsets here?
yes, thanks
jirka
