[Mesa-dev] [PATCH 1/2] util: Add utility build-id code.

[Jason Ekstrand]

On Wed, Feb 15, 2017 at 10:57 AM, Matt Turner <mattst88 at gmail.com> wrote:

> On Wed, Feb 15, 2017 at 4:03 AM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
> > Hi Matt,
> >
> > Afaics dl_iterate_phdr is available on musl, (some?) BSDs and Solaris
> > - thank you for opting for it.
> >
> > Out of curiosity:
> > Have you checked if on those platforms the "GNU\0" strcmp is
> > applicable and not another string ? Worth adding a note/comment ?
>
> It's generated by GNU ld in all cases I'm aware of, so I think it's the
> same.
>
> > On 14 February 2017 at 23:58, Matt Turner <mattst88 at gmail.com> wrote:
> >> Provides the ability to read the .note.gnu.build-id section of ELF
> >> binaries, which is inserted by the --build-id=... flag to ld.
> >> ---
> >>  configure.ac              |   2 +
> >>  src/util/Makefile.sources |   2 +
> >>  src/util/build_id.c       | 110 ++++++++++++++++++++++++++++++
> ++++++++++++++++
> >>  src/util/build_id.h       |  34 ++++++++++++++
> >>  4 files changed, 148 insertions(+)
> >>  create mode 100644 src/util/build_id.c
> >>  create mode 100644 src/util/build_id.h
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index f001743..99c74f0 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -768,6 +768,8 @@ LIBS="$LIBS $DLOPEN_LIBS"
> >>  AC_CHECK_FUNCS([dladdr])
> >>  LIBS="$save_LIBS"
> >>
> >> +AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
> -DHAVE_DL_ITERATE_PHDR"])
> >> +
> > What we want here is a local (in-configure) variable -> have_foo,
> > which will be checked by ANV/others that depend on an actual
> > implementation.
> > As-is things will compile but we'll get a link error.
> >
> >
> >> +const struct build_id_note *
> >> +build_id_find_nhdr(const char *filename)
> >> +{
> >> +   struct callback_data data = {
> >> +      .filename = filename,
> >> +      .note = NULL,
> >> +   };
> >> +
> >> +   if (dl_iterate_phdr(build_id_find_nhdr_callback, &data)) {
> >> +      return data.note;
> >> +   } else {
> >> +      return NULL;
> >> +   }
> > Nit:
> >
> >    if (!dl_iterate_phdr(build_id_find_nhdr_callback, &data))
> >       return NULL;
> >
> >    return data.note;
> >
> >
> >> diff --git a/src/util/build_id.h b/src/util/build_id.h
> >> new file mode 100644
> >> index 0000000..20db4ac
> >> --- /dev/null
> >> +++ b/src/util/build_id.h
> >
> >> +struct build_id_note;
> >> +
> >> +const struct build_id_note *
> >> +build_id_find_nhdr(const char *filename);
> >> +
> >> +unsigned
> >> +build_id_length(const struct build_id_note *note);
> >> +
> >> +void
> >> +build_id_read(const struct build_id_note *note,
> >> +              unsigned char *build_id, size_t n);
> >
> > With the configure fix, one can bring back the stubs here. Having a
> > function declaration w/o a definition is a bit iffy.
>
> I think having stubs was a bad idea from the outset. In any case one
> of these stubs is called, something bad is going to happen.
>
> I'll wrap the header in #ifdef HAVE_DL_ITERATE_PHDR instead.
>

For the sake of other code which needs to #if around whether or not to call
these functions, it might be a good idea to have a

#define HAVE_BUILD_ID

inside of the #ifdef HAVE_DL_ITERATE_PHDR.  It's a bit more convenient and
allows for a potential future where we have some method for getting a
build-id on windows or similar.

--Jason


> > With the above:
> > Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>
> Thanks. I'll incorporate these changes and send a v3.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170215/a42ba987/attachment.html>