Re: [PATCH bpf-next 7/7] selftests/bpf: add urandom_read shared lib and USDTs

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, Mar 31, 2022 at 3:14 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On Fri, 25 Mar 2022, Andrii Nakryiko wrote:
>
> > Extend urandom_read helper binary to include USDTs of 4 combinations:
> > semaphore/semaphoreless (refcounted and non-refcounted) and based in
> > executable or shared library. We also extend urandom_read with ability
> > to report it's own PID to parent process and wait for parent process to
> > ready itself up for tracing urandom_read. We utilize popen() and
> > underlying pipe properties for proper signaling.
> >
> > Once urandom_read is ready, we add few tests to validate that libbpf's
> > USDT attachment handles all the above combinations of semaphore (or lack
> > of it) and static or shared library USDTs. Also, we validate that libbpf
> > handles shared libraries both with PID filter and without one (i.e., -1
> > for PID argument).
> >
> > Having the shared library case tested with and without PID is important
> > because internal logic differs on kernels that don't support BPF
> > cookies. On such older kernels, attaching to USDTs in shared libraries
> > without specifying concrete PID doesn't work in principle, because it's
> > impossible to determine shared library's load address to derive absolute
> > IPs for uprobe attachments. Without absolute IPs, it's impossible to
> > perform correct look up of USDT spec based on uprobe's absolute IP (the
> > only kind available from BPF at runtime). This is not the problem on
> > newer kernels with BPF cookie as we don't need IP-to-ID lookup because
> > BPF cookie value *is* spec ID.
> >
> > So having those two situations as separate subtests is good because
> > libbpf CI is able to test latest selftests against old kernels (e.g.,
> > 4.9 and 5.5), so we'll be able to disable PID-less shared lib attachment
> > for old kernels, but will still leave PID-specific one enabled to validate
> > this legacy logic is working correctly.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>
>
> haven't looked at this in depth yet, but hit a compilation error on
> aarch64:
>
>   LIB      liburandom_read.so
> /usr/bin/ld: /tmp/ccNy8cuv.o: relocation R_AARCH64_ADR_PREL_PG_HI21
> against symbol `urandlib_read_with_sema_semaphore' which may bind
> externally can not be used when making a shared object; recompile with
> -fPIC
> /tmp/ccNy8cuv.o: In function `urandlib_read_with_sema':
> /home/opc/src/bpf-next/tools/testing/selftests/bpf/urandom_read_lib1.c:12:(.text+0x10):
> dangerous relocation: unsupported relocation
> collect2: error: ld returned 1 exit status
> make: *** [Makefile:173:
> /home/opc/src/bpf-next/tools/testing/selftests/bpf/liburandom_read.so]
> Error 1
>
> following did fix it:
>
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 58da22c019a8..c89e2948276b 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -170,7 +170,7 @@ $(OUTPUT)/%:%.c
>
>  $(OUTPUT)/liburandom_read.so: urandom_read_lib1.c urandom_read_lib2.c
>         $(call msg,LIB,,$@)
> -       $(Q)$(CC) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
> +       $(Q)$(CC) $(CFLAGS) -fPIC $(LDFLAGS) $^ $(LDLIBS) --shared -o $@
>

Yep, forgot about -fPIC, thanks. Curious that it still works on x86 just fine :)

BTW, if you are interested in arm architecture, it would be great to
get some help to setup CI for ARMs. We have s390x and x86_64, it's a
great way to prevent lots of bugs. We just don't have anyone actively
supporting this for ARMs.

>  $(OUTPUT)/urandom_read: urandom_read.c urandom_read_aux.c
> $(OUTPUT)/liburandom_read.so
>         $(call msg,BINARY,,$@)
>