From:   Jiri Olsa <olsajiri@gmail.com>
Date:   Thu, 9 Feb 2023 15:04:45 +0100
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Hao Luo <haoluo@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        bpf@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-perf-users@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@chromium.org>,
        Stanislav Fomichev <sdf@google.com>,
        Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [PATCH RFC 4/5] selftests/bpf: Add file_build_id test
Message-ID: <Y+T9fYbGcDDb2RFt@krava>
References: <20230201135737.800527-1-jolsa@kernel.org>
 <20230201135737.800527-5-jolsa@kernel.org>
 <CAEf4BzZ6BVeLV5mG=nB88Ni_8WSYTG0xhFgn-OEM2s6dc14yVA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAEf4BzZ6BVeLV5mG=nB88Ni_8WSYTG0xhFgn-OEM2s6dc14yVA@mail.gmail.com>
Precedence: bulk

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