Re: [PATCH RFC 4/5] selftests/bpf: Add file_build_id test

From: Jiri Olsa
Date: Thu Feb 09 2023 - 09:04:59 EST


On Wed, Feb 08, 2023 at 03:58:06PM -0800, Andrii Nakryiko wrote:

SNIP

> > +
> > + /* parent, update child's pid and kick it */
> > + skel->bss->pid = child_pid;
> > +
> > + err = file_build_id__attach(skel);
> > + if (!ASSERT_OK(err, "file_build_id__attach"))
> > + goto out;
> > +
> > + err = write(go[1], &c, 1);
> > + if (!ASSERT_EQ(err, 1, "child_write_pipe"))
> > + goto out;
> > +
> > + /* wait for child to exit */
> > + waitpid(child_pid, &child_status, 0);
> > + if (!ASSERT_EQ(WEXITSTATUS(child_status), 0, "child_exit_value"))
> > + goto out;
> > +
> > + if (!ASSERT_OK(read_buildid("/bin/bash", &bid), "read_buildid"))
>
> can we use urandom_read for build_id ? And it would also be nice to
> check that build id fetching works for liburandom_read.so as well.

ok, will be better together with the shared library

SNIP

> > diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> > index 09a16a77bae4..f5557890e383 100644
> > --- a/tools/testing/selftests/bpf/trace_helpers.c
> > +++ b/tools/testing/selftests/bpf/trace_helpers.c
> > @@ -9,6 +9,7 @@
> > #include <poll.h>
> > #include <unistd.h>
> > #include <linux/perf_event.h>
> > +#include <linux/limits.h>
> > #include <sys/mman.h>
> > #include "trace_helpers.h"
> >
> > @@ -230,3 +231,37 @@ ssize_t get_rel_offset(uintptr_t addr)
> > fclose(f);
> > return -EINVAL;
> > }
> > +
> > +int read_buildid(const char *path, char **build_id)
> > +{
> > + char tmp[] = "/tmp/dataXXXXXX";
> > + char buf[PATH_MAX + 200];
> > + int err, fd;
> > + FILE *f;
> > +
> > + fd = mkstemp(tmp);
> > + if (fd == -1)
> > + return -1;
> > + close(fd);
> > +
> > + snprintf(buf, sizeof(buf),
> > + "readelf -n %s 2>/dev/null | grep 'Build ID' | awk '{print $3}' > %s",
> > + path, tmp);
> > +
>
> shelling out to readelf for this is unfortunate... maybe let's write a
> libelf-based helper to fetch build ID from .note section?

right, I was thinking of that, shouldn't be that hard
and will speed things up

thanks,
jirka