<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 15, 2017 at 10:57 AM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Feb 15, 2017 at 4:03 AM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> wrote:<br>
> Hi Matt,<br>
><br>
> Afaics dl_iterate_phdr is available on musl, (some?) BSDs and Solaris<br>
> - thank you for opting for it.<br>
><br>
> Out of curiosity:<br>
> Have you checked if on those platforms the "GNU\0" strcmp is<br>
> applicable and not another string ? Worth adding a note/comment ?<br>
<br>
</span>It's generated by GNU ld in all cases I'm aware of, so I think it's the same.<br>
<div><div class="h5"><br>
> On 14 February 2017 at 23:58, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>> Provides the ability to read the .note.gnu.build-id section of ELF<br>
>> binaries, which is inserted by the --build-id=... flag to ld.<br>
>> ---<br>
>>  <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>              |   2 +<br>
>>  src/util/Makefile.sources |   2 +<br>
>>  src/util/build_id.c       | 110 ++++++++++++++++++++++++++++++<wbr>++++++++++++++++<br>
>>  src/util/build_id.h       |  34 ++++++++++++++<br>
>>  4 files changed, 148 insertions(+)<br>
>>  create mode 100644 src/util/build_id.c<br>
>>  create mode 100644 src/util/build_id.h<br>
>><br>
>> diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
>> index f001743..99c74f0 100644<br>
>> --- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
>> +++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
>> @@ -768,6 +768,8 @@ LIBS="$LIBS $DLOPEN_LIBS"<br>
>>  AC_CHECK_FUNCS([dladdr])<br>
>>  LIBS="$save_LIBS"<br>
>><br>
>> +AC_CHECK_FUNC([dl_iterate_<wbr>phdr], [DEFINES="$DEFINES -DHAVE_DL_ITERATE_PHDR"])<br>
>> +<br>
> What we want here is a local (in-configure) variable -> have_foo,<br>
> which will be checked by ANV/others that depend on an actual<br>
> implementation.<br>
> As-is things will compile but we'll get a link error.<br>
><br>
><br>
>> +const struct build_id_note *<br>
>> +build_id_find_nhdr(const char *filename)<br>
>> +{<br>
>> +   struct callback_data data = {<br>
>> +      .filename = filename,<br>
>> +      .note = NULL,<br>
>> +   };<br>
>> +<br>
>> +   if (dl_iterate_phdr(build_id_<wbr>find_nhdr_callback, &data)) {<br>
>> +      return data.note;<br>
>> +   } else {<br>
>> +      return NULL;<br>
>> +   }<br>
> Nit:<br>
><br>
>    if (!dl_iterate_phdr(build_id_<wbr>find_nhdr_callback, &data))<br>
>       return NULL;<br>
><br>
>    return data.note;<br>
><br>
><br>
>> diff --git a/src/util/build_id.h b/src/util/build_id.h<br>
>> new file mode 100644<br>
>> index 0000000..20db4ac<br>
>> --- /dev/null<br>
>> +++ b/src/util/build_id.h<br>
><br>
>> +struct build_id_note;<br>
>> +<br>
>> +const struct build_id_note *<br>
>> +build_id_find_nhdr(const char *filename);<br>
>> +<br>
>> +unsigned<br>
>> +build_id_length(const struct build_id_note *note);<br>
>> +<br>
>> +void<br>
>> +build_id_read(const struct build_id_note *note,<br>
>> +              unsigned char *build_id, size_t n);<br>
><br>
> With the configure fix, one can bring back the stubs here. Having a<br>
> function declaration w/o a definition is a bit iffy.<br>
<br>
</div></div>I think having stubs was a bad idea from the outset. In any case one<br>
of these stubs is called, something bad is going to happen.<br>
<br>
I'll wrap the header in #ifdef HAVE_DL_ITERATE_PHDR instead.<span class=""><br></span></blockquote><div><br></div><div>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 <br><br></div><div>#define HAVE_BUILD_ID<br><br></div><div>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.<br><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> With the above:<br>
> Reviewed-by: Emil Velikov <<a href="mailto:emil.velikov@collabora.com">emil.velikov@collabora.com</a>><br>
<br>
</span>Thanks. I'll incorporate these changes and send a v3.<br>
<div class="HOEnZb"><div class="h5">______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>