Re: [PATCH bpf-next v2 3/3] selftests/bpf: Add tests for symbol versioning for uprobe

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Sep 05, 2023 at 03:12:57PM +0000, Hengqi Chen wrote:
> This exercises the newly added dynsym symbol versioning logics.
> Now we accept symbols in form of func, func@LIB_VERSION or
> func@@LIB_VERSION.
> 
> The test rely on liburandom_read.so. For liburandom_read.so, we have:
> 
>     $ nm -D liburandom_read.so
>                      w __cxa_finalize@GLIBC_2.17
>                      w __gmon_start__
>                      w _ITM_deregisterTMCloneTable
>                      w _ITM_registerTMCloneTable
>     0000000000000000 A LIBURANDOM_READ_1.0.0
>     0000000000000000 A LIBURANDOM_READ_2.0.0
>     000000000000081c T urandlib_api@@LIBURANDOM_READ_2.0.0
>     0000000000000814 T urandlib_api@LIBURANDOM_READ_1.0.0
>     0000000000000824 T urandlib_api_sameoffset@LIBURANDOM_READ_1.0.0
>     0000000000000824 T urandlib_api_sameoffset@@LIBURANDOM_READ_2.0.0
>     000000000000082c T urandlib_read_without_sema@@LIBURANDOM_READ_1.0.0
>     00000000000007c4 T urandlib_read_with_sema@@LIBURANDOM_READ_1.0.0
>     0000000000011018 D urandlib_read_with_sema_semaphore@@LIBURANDOM_READ_1.0.0
> 
> For `urandlib_api`, specifying `urandlib_api` will cause a conflict because
> there are two symbols named urandlib_api and both are global bind.
> For `urandlib_api_sameoffset`, there are also two symbols in the .so, but
> both are at the same offset and essentially they refer to the same function
> so no conflict.
> 
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>

one nit below, but looks good

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka


> ---
>  tools/testing/selftests/bpf/Makefile          |  5 +-
>  .../testing/selftests/bpf/liburandom_read.map | 15 +++
>  .../testing/selftests/bpf/prog_tests/uprobe.c | 95 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/test_uprobe.c | 61 ++++++++++++
>  tools/testing/selftests/bpf/urandom_read.c    |  9 ++
>  .../testing/selftests/bpf/urandom_read_lib1.c | 41 ++++++++
>  6 files changed, 224 insertions(+), 2 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/liburandom_read.map
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_uprobe.c
> 

SNIP

> +void test_uprobe(void)
> +{
> +	LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
> +	struct test_uprobe *skel;
> +	FILE *urand_pipe = NULL;
> +	int urand_pid = 0, err;
> +
> +	skel = test_uprobe__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return;
> +
> +	urand_pipe = urand_spawn(&urand_pid);
> +	if (!ASSERT_OK_PTR(urand_pipe, "urand_spawn"))
> +		goto cleanup;
> +
> +	skel->bss->my_pid = urand_pid;
> +
> +	/* Manual attach uprobe to urandlib_api
> +	 * There are two `urandlib_api` symbols in .dynsym section:
> +	 *   - urandlib_api@LIBURANDOM_READ_1.0.0
> +	 *   - urandlib_api@LIBURANDOM_READ_1.0.0

nit, should that be version 2.0.0?               ^

> +	 * Both are global bind and would cause a conflict if user
> +	 * specify the symbol name without a version suffix
> +	 */
> +	uprobe_opts.func_name = "urandlib_api";
> +	skel->links.test4 = bpf_program__attach_uprobe_opts(skel->progs.test4,
> +							    urand_pid,
> +							    "./liburandom_read.so",
> +							    0 /* offset */,
> +							    &uprobe_opts);
> +	if (!ASSERT_ERR_PTR(skel->links.test4, "urandlib_api_attach_conflict"))
> +		goto cleanup;
> +
> +	uprobe_opts.func_name = "urandlib_api@LIBURANDOM_READ_1.0.0";
> +	skel->links.test4 = bpf_program__attach_uprobe_opts(skel->progs.test4,
> +							    urand_pid,
> +							    "./liburandom_read.so",
> +							    0 /* offset */,
> +							    &uprobe_opts);
> +	if (!ASSERT_OK_PTR(skel->links.test4, "urandlib_api_attach_ok"))
> +		goto cleanup;
> +
> +	/* Auto attach 3 uprobes to urandlib_api_sameoffset */
> +	err = test_uprobe__attach(skel);
> +	if (!ASSERT_OK(err, "skel_attach"))
> +		goto cleanup;
> +
> +	/* trigger urandom_read */
> +	ASSERT_OK(urand_trigger(&urand_pipe), "urand_exit_code");
> +
> +	ASSERT_EQ(skel->bss->test1_result, 1, "urandlib_api_sameoffset");
> +	ASSERT_EQ(skel->bss->test2_result, 1, "urandlib_api_sameoffset@v1");
> +	ASSERT_EQ(skel->bss->test3_result, 1, "urandlib_api_sameoffset@@v2");
> +	ASSERT_EQ(skel->bss->test4_result, 1, "urandlib_api");
> +
> +cleanup:
> +	if (urand_pipe)
> +		pclose(urand_pipe);
> +	test_uprobe__destroy(skel);
> +}

SNIP