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

[Matt Turner]

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.

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

Thanks. I'll incorporate these changes and send a v3.