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

[Emil Velikov]

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 ?


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.

With the above:
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

I really like that there's no manual parsing the ELF header.

Thanks
Emil