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

[Jonathan Gray]

On Fri, Feb 17, 2017 at 08:30:17AM -0800, Matt Turner wrote:
> On Fri, Feb 17, 2017 at 5:39 AM, Emil Velikov  
> wrote:
> > On 17 February 2017 at 01:10, Jonathan Gray  wrote:
> >> On Thu, Feb 16, 2017 at 04:25:02PM +, Emil Velikov wrote:
> >>> On 16 February 2017 at 14:23, Jonathan Gray  wrote:
> >>> > On Wed, Feb 15, 2017 at 11:11:50AM -0800, Matt Turner 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.
> >>> >>
> >>> >> Reviewed-by: Emil Velikov 
> >>> >
> >>> > I don't have time to dig into details right now but this broke the Mesa
> >>> > build on OpenBSD and likely other non-linux platforms:
> >>> >
> >>> > libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" 
> >>> > -DPACKAGE_TARNAME=\"mesa\" -DPACKAGE_VERSION=\"17.1.0-devel\" 
> >>> > "-DPACKAGE_STRING=\"Mesa 17.1.0-devel\"" 
> >>> > "-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";
> >>> >  -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"17.1.0-devel\" 
> >>> > -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
> >>> > -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 
> >>> > -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 
> >>> > -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" 
> >>> > -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1 
> >>> > -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1 
> >>> > -DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1 
> >>> > -DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1 
> >>> > -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 
> >>> > -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1 
> >>> > -DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 
> >>> > -DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 
> >>> > -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_FUNC_ATTRIBUTE_WEAK=1 
> >>> > -DHAVE_FUNC_ATTRIBUTE_ALIAS=1 -DHAVE_DLADDR=1 -DHAVE_CLOCK_GETTIME=1 
> >>> > -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -I. 
> >>> > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> >>> > -DDEBUG -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H 
> >>> > -DHAVE_STRTOF -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_DL_ITERATE_PHDR 
> >>> > -DHAVE_POSIX_MEMALIGN -DHAVE_LIBDRM -DGLX_USE_DRM 
> >>> > -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DENABLE_SHADER_CACHE 
> >>> > -DHAVE_MINCORE -I../../include -I../../src -I../../src/mapi 
> >>> > -I../../src/mesa -I../../src/gallium/include 
> >>> > -I../../src/gallium/auxiliary -fvisibility=hidden -Werror=pointer-arith 
> >>> > -g -O2 -Wall -std=gnu99 -Werror=implicit-function-declaration 
> >>> > -Werror=missing-prototypes -fno-math-errno -fno-trapping-math -MT 
> >>> > libmesautil_la-build_id.lo -MD -MP -MF 
> >>> > .deps/libmesautil_la-build_id.Tpo -c build_id.c  -fPIC -DPIC -o 
> >>> > .libs/libmesautil_la-build_id.o
> >>> > In file included from /usr/include/elf_abi.h:31,
> >>> >  from /usr/include/link_elf.h:10,
> >>> >  from /usr/include/link.h:39,
> >>> >  from build_id.c:25:
> >>> > /usr/include/sys/exec_elf.h:585: error: expected 
> >>> > specifier-qualifier-list before 'uint32_t'
> >>> > In file included from /usr/include/link.h:39,
> >>> >  from build_id.c:25:
> >>> > /usr/include/link_elf.h:22: error: expected specifier-qualifier-list 
> >>> > before 'caddr_t'
> >>> > /usr/include/link_elf.h:37: error: expected '=', ',', ';', 'asm' or 
> >>> > '__attribute__' before 'int'
> >>> > In file included from build_id.c:25:
> >>> > /usr/include/link.h:49: error: expected '=', ',', ';', 'asm' or 
> >>> > '__attribute__' before 'struct'
> >>> > /usr/include/link.h:65: error: expected specifier-qualifier-list before 
> >>> > 'caddr_t'
> >>> These look like issue in your platform code/headers. Perhaps some bad
> >>> interaction with the bits that Mesa defines ?
> >>>
> >>> Quick workaround is to check the function only when needed, roughly
> >>> like this pseudo code:
> >>>
> >>> if test $building_any_vulkan_driver = yes ;then
> >>> require_dl...=yes
> >>>
> >>> fi
> >>> 
> >>>
> >>> if test $require_dl... = yes ; then
> >>>AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
> >>> -DHAVE_DL_ITERATE_PHDR"], [AC_MSG_ERROR([required  not found])])
> >>> fi
> >>>
> >>>
> >>> Please give it a bash and send us a patch that works on your end.
> >>
> >> Leaning towards something along the lines of the following.
> >> With Nhdr struct definitions added to system exec_elf.h.
> >>
> > IMHO it makes little sense to build the file if no code uses it. That aside:
> 
> Agreed, but I think this will be used for shader cache as well.
> 
> >
> >> The need for sys/types.h here may go away shortly as well.
> >>
> >> diff --git a/src/util/build_id.c 

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

[Matt Turner]

On Fri, Feb 17, 2017 at 5:39 AM, Emil Velikov  wrote:
> On 17 February 2017 at 01:10, Jonathan Gray  wrote:
>> On Thu, Feb 16, 2017 at 04:25:02PM +, Emil Velikov wrote:
>>> On 16 February 2017 at 14:23, Jonathan Gray  wrote:
>>> > On Wed, Feb 15, 2017 at 11:11:50AM -0800, Matt Turner 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.
>>> >>
>>> >> Reviewed-by: Emil Velikov 
>>> >
>>> > I don't have time to dig into details right now but this broke the Mesa
>>> > build on OpenBSD and likely other non-linux platforms:
>>> >
>>> > libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 
>>> > -DPACKAGE_VERSION=\"17.1.0-devel\" "-DPACKAGE_STRING=\"Mesa 
>>> > 17.1.0-devel\"" 
>>> > "-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";
>>> >  -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"17.1.0-devel\" 
>>> > -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
>>> > -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
>>> > -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
>>> > -DLT_OBJDIR=\".libs/\" -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 
>>> > -DHAVE___BUILTIN_CLZLL=1 -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 
>>> > -DHAVE___BUILTIN_FFS=1 -DHAVE___BUILTIN_FFSLL=1 
>>> > -DHAVE___BUILTIN_POPCOUNT=1 -DHAVE___BUILTIN_POPCOUNTLL=1 
>>> > -DHAVE_FUNC_ATTRIBUTE_CONST=1 -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 
>>> > -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 
>>> > -DHAVE_FUNC_ATTRIBUTE_PACKED=1 -DHAVE_FUNC_ATTRIBUTE_PURE=1 
>>> > -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 -DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 
>>> > -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_FUNC_ATTRIBUTE_WEAK=1 
>>> > -DHAVE_FUNC_ATTRIBUTE_ALIAS=1 -DHAVE_DLADDR=1 -DHAVE_CLOCK_GETTIME=1 
>>> > -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -I. 
>>> > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
>>> > -DDEBUG -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H 
>>> > -DHAVE_STRTOF -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_DL_ITERATE_PHDR 
>>> > -DHAVE_POSIX_MEMALIGN -DHAVE_LIBDRM -DGLX_USE_DRM 
>>> > -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING -DENABLE_SHADER_CACHE 
>>> > -DHAVE_MINCORE -I../../include -I../../src -I../../src/mapi 
>>> > -I../../src/mesa -I../../src/gallium/include 
>>> > -I../../src/gallium/auxiliary -fvisibility=hidden -Werror=pointer-arith 
>>> > -g -O2 -Wall -std=gnu99 -Werror=implicit-function-declaration 
>>> > -Werror=missing-prototypes -fno-math-errno -fno-trapping-math -MT 
>>> > libmesautil_la-build_id.lo -MD -MP -MF .deps/libmesautil_la-build_id.Tpo 
>>> > -c build_id.c  -fPIC -DPIC -o .libs/libmesautil_la-build_id.o
>>> > In file included from /usr/include/elf_abi.h:31,
>>> >  from /usr/include/link_elf.h:10,
>>> >  from /usr/include/link.h:39,
>>> >  from build_id.c:25:
>>> > /usr/include/sys/exec_elf.h:585: error: expected specifier-qualifier-list 
>>> > before 'uint32_t'
>>> > In file included from /usr/include/link.h:39,
>>> >  from build_id.c:25:
>>> > /usr/include/link_elf.h:22: error: expected specifier-qualifier-list 
>>> > before 'caddr_t'
>>> > /usr/include/link_elf.h:37: error: expected '=', ',', ';', 'asm' or 
>>> > '__attribute__' before 'int'
>>> > In file included from build_id.c:25:
>>> > /usr/include/link.h:49: error: expected '=', ',', ';', 'asm' or 
>>> > '__attribute__' before 'struct'
>>> > /usr/include/link.h:65: error: expected specifier-qualifier-list before 
>>> > 'caddr_t'
>>> These look like issue in your platform code/headers. Perhaps some bad
>>> interaction with the bits that Mesa defines ?
>>>
>>> Quick workaround is to check the function only when needed, roughly
>>> like this pseudo code:
>>>
>>> if test $building_any_vulkan_driver = yes ;then
>>> require_dl...=yes
>>>
>>> fi
>>> 
>>>
>>> if test $require_dl... = yes ; then
>>>AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
>>> -DHAVE_DL_ITERATE_PHDR"], [AC_MSG_ERROR([required  not found])])
>>> fi
>>>
>>>
>>> Please give it a bash and send us a patch that works on your end.
>>
>> Leaning towards something along the lines of the following.
>> With Nhdr struct definitions added to system exec_elf.h.
>>
> IMHO it makes little sense to build the file if no code uses it. That aside:

Agreed, but I think this will be used for shader cache as well.

>
>> The need for sys/types.h here may go away shortly as well.
>>
>> diff --git a/src/util/build_id.c b/src/util/build_id.c
>> index 2993a80cfe..92250a1f5f 100644
>> --- a/src/util/build_id.c
>> +++ b/src/util/build_id.c
>> @@ -22,12 +22,22 @@
>>   */
>>
>>  #ifdef HAVE_DL_ITERATE_PHDR
>> +
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>>
>>  #include "build_id.h"

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

[Emil Velikov]

On 17 February 2017 at 01:10, Jonathan Gray  wrote:
> On Thu, Feb 16, 2017 at 04:25:02PM +, Emil Velikov wrote:
>> On 16 February 2017 at 14:23, Jonathan Gray  wrote:
>> > On Wed, Feb 15, 2017 at 11:11:50AM -0800, Matt Turner 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.
>> >>
>> >> Reviewed-by: Emil Velikov 
>> >
>> > I don't have time to dig into details right now but this broke the Mesa
>> > build on OpenBSD and likely other non-linux platforms:
>> >
>> > libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 
>> > -DPACKAGE_VERSION=\"17.1.0-devel\" "-DPACKAGE_STRING=\"Mesa 
>> > 17.1.0-devel\"" 
>> > "-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";
>> >  -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"17.1.0-devel\" 
>> > -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
>> > -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
>> > -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
>> > -DLT_OBJDIR=\".libs/\" -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 
>> > -DHAVE___BUILTIN_CLZLL=1 -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 
>> > -DHAVE___BUILTIN_FFS=1 -DHAVE___BUILTIN_FFSLL=1 
>> > -DHAVE___BUILTIN_POPCOUNT=1 -DHAVE___BUILTIN_POPCOUNTLL=1 
>> > -DHAVE_FUNC_ATTRIBUTE_CONST=1 -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 
>> > -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 
>> > -DHAVE_FUNC_ATTRIBUTE_PACKED=1 -DHAVE_FUNC_ATTRIBUTE_PURE=1 
>> > -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 -DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 
>> > -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_FUNC_ATTRIBUTE_WEAK=1 
>> > -DHAVE_FUNC_ATTRIBUTE_ALIAS=1 -DHAVE_DLADDR=1 -DHAVE_CLOCK_GETTIME=1 
>> > -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -I. 
>> > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
>> > -DDEBUG -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H 
>> > -DHAVE_STRTOF -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_DL_ITERATE_PHDR 
>> > -DHAVE_POSIX_MEMALIGN -DHAVE_LIBDRM -DGLX_USE_DRM -DGLX_INDIRECT_RENDERING 
>> > -DGLX_DIRECT_RENDERING -DENABLE_SHADER_CACHE -DHAVE_MINCORE 
>> > -I../../include -I../../src -I../../src/mapi -I../../src/mesa 
>> > -I../../src/gallium/include -I../../src/gallium/auxiliary 
>> > -fvisibility=hidden -Werror=pointer-arith -g -O2 -Wall -std=gnu99 
>> > -Werror=implicit-function-declaration -Werror=missing-prototypes 
>> > -fno-math-errno -fno-trapping-math -MT libmesautil_la-build_id.lo -MD -MP 
>> > -MF .deps/libmesautil_la-build_id.Tpo -c build_id.c  -fPIC -DPIC -o 
>> > .libs/libmesautil_la-build_id.o
>> > In file included from /usr/include/elf_abi.h:31,
>> >  from /usr/include/link_elf.h:10,
>> >  from /usr/include/link.h:39,
>> >  from build_id.c:25:
>> > /usr/include/sys/exec_elf.h:585: error: expected specifier-qualifier-list 
>> > before 'uint32_t'
>> > In file included from /usr/include/link.h:39,
>> >  from build_id.c:25:
>> > /usr/include/link_elf.h:22: error: expected specifier-qualifier-list 
>> > before 'caddr_t'
>> > /usr/include/link_elf.h:37: error: expected '=', ',', ';', 'asm' or 
>> > '__attribute__' before 'int'
>> > In file included from build_id.c:25:
>> > /usr/include/link.h:49: error: expected '=', ',', ';', 'asm' or 
>> > '__attribute__' before 'struct'
>> > /usr/include/link.h:65: error: expected specifier-qualifier-list before 
>> > 'caddr_t'
>> These look like issue in your platform code/headers. Perhaps some bad
>> interaction with the bits that Mesa defines ?
>>
>> Quick workaround is to check the function only when needed, roughly
>> like this pseudo code:
>>
>> if test $building_any_vulkan_driver = yes ;then
>> require_dl...=yes
>>
>> fi
>> 
>>
>> if test $require_dl... = yes ; then
>>AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
>> -DHAVE_DL_ITERATE_PHDR"], [AC_MSG_ERROR([required  not found])])
>> fi
>>
>>
>> Please give it a bash and send us a patch that works on your end.
>
> Leaning towards something along the lines of the following.
> With Nhdr struct definitions added to system exec_elf.h.
>
IMHO it makes little sense to build the file if no code uses it. That aside:

> The need for sys/types.h here may go away shortly as well.
>
> diff --git a/src/util/build_id.c b/src/util/build_id.c
> index 2993a80cfe..92250a1f5f 100644
> --- a/src/util/build_id.c
> +++ b/src/util/build_id.c
> @@ -22,12 +22,22 @@
>   */
>
>  #ifdef HAVE_DL_ITERATE_PHDR
> +
> +#include 
>  #include 
>  #include 
>  #include 
>
>  #include "build_id.h"
>
> +#ifndef NT_GNU_BUILD_ID
> +#define NT_GNU_BUILD_ID 3
> +#endif
> +
> +#ifndef ElfW
> +#define ElfW(type) Elf_##type
> +#endif
> +
AFAICT the ElfW macro is a Linux/Solaris thing and is missing from
OpenBSD/FreeBSD. So we do want this, but I have 

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

[Jonathan Gray]

On Thu, Feb 16, 2017 at 04:25:02PM +, Emil Velikov wrote:
> On 16 February 2017 at 14:23, Jonathan Gray  wrote:
> > On Wed, Feb 15, 2017 at 11:11:50AM -0800, Matt Turner 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.
> >>
> >> Reviewed-by: Emil Velikov 
> >
> > I don't have time to dig into details right now but this broke the Mesa
> > build on OpenBSD and likely other non-linux platforms:
> >
> > libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 
> > -DPACKAGE_VERSION=\"17.1.0-devel\" "-DPACKAGE_STRING=\"Mesa 17.1.0-devel\"" 
> > "-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";
> >  -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"17.1.0-devel\" 
> > -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 
> > -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 
> > -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" 
> > -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1 
> > -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1 
> > -DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1 
> > -DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1 
> > -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 
> > -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1 
> > -DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 
> > -DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 
> > -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 -DHAVE_FUNC_ATTRIBUTE_WEAK=1 
> > -DHAVE_FUNC_ATTRIBUTE_ALIAS=1 -DHAVE_DLADDR=1 -DHAVE_CLOCK_GETTIME=1 
> > -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -I. -D__STDC_CONSTANT_MACROS 
> > -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DDEBUG 
> > -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H -DHAVE_STRTOF 
> > -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_DL_ITERATE_PHDR -DHAVE_POSIX_MEMALIGN 
> > -DHAVE_LIBDRM -DGLX_USE_DRM -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING 
> > -DENABLE_SHADER_CACHE -DHAVE_MINCORE -I../../include -I../../src 
> > -I../../src/mapi -I../../src/mesa -I../../src/gallium/include 
> > -I../../src/gallium/auxiliary -fvisibility=hidden -Werror=pointer-arith -g 
> > -O2 -Wall -std=gnu99 -Werror=implicit-function-declaration 
> > -Werror=missing-prototypes -fno-math-errno -fno-trapping-math -MT 
> > libmesautil_la-build_id.lo -MD -MP -MF .deps/libmesautil_la-build_id.Tpo -c 
> > build_id.c  -fPIC -DPIC -o .libs/libmesautil_la-build_id.o
> > In file included from /usr/include/elf_abi.h:31,
> >  from /usr/include/link_elf.h:10,
> >  from /usr/include/link.h:39,
> >  from build_id.c:25:
> > /usr/include/sys/exec_elf.h:585: error: expected specifier-qualifier-list 
> > before 'uint32_t'
> > In file included from /usr/include/link.h:39,
> >  from build_id.c:25:
> > /usr/include/link_elf.h:22: error: expected specifier-qualifier-list before 
> > 'caddr_t'
> > /usr/include/link_elf.h:37: error: expected '=', ',', ';', 'asm' or 
> > '__attribute__' before 'int'
> > In file included from build_id.c:25:
> > /usr/include/link.h:49: error: expected '=', ',', ';', 'asm' or 
> > '__attribute__' before 'struct'
> > /usr/include/link.h:65: error: expected specifier-qualifier-list before 
> > 'caddr_t'
> These look like issue in your platform code/headers. Perhaps some bad
> interaction with the bits that Mesa defines ?
> 
> Quick workaround is to check the function only when needed, roughly
> like this pseudo code:
> 
> if test $building_any_vulkan_driver = yes ;then
> require_dl...=yes
>
> fi
> 
> 
> if test $require_dl... = yes ; then
>AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
> -DHAVE_DL_ITERATE_PHDR"], [AC_MSG_ERROR([required  not found])])
> fi
> 
> 
> Please give it a bash and send us a patch that works on your end.

Leaning towards something along the lines of the following.
With Nhdr struct definitions added to system exec_elf.h.

The need for sys/types.h here may go away shortly as well.

diff --git a/src/util/build_id.c b/src/util/build_id.c
index 2993a80cfe..92250a1f5f 100644
--- a/src/util/build_id.c
+++ b/src/util/build_id.c
@@ -22,12 +22,22 @@
  */
 
 #ifdef HAVE_DL_ITERATE_PHDR
+
+#include 
 #include 
 #include 
 #include 
 
 #include "build_id.h"
 
+#ifndef NT_GNU_BUILD_ID
+#define NT_GNU_BUILD_ID 3
+#endif
+
+#ifndef ElfW
+#define ElfW(type) Elf_##type
+#endif
+
 #define ALIGN(val, align)  (((val) + (align) - 1) & ~((align) - 1))
 
 struct build_id_note {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 16 February 2017 at 14:23, Jonathan Gray  wrote:
> On Wed, Feb 15, 2017 at 11:11:50AM -0800, Matt Turner 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.
>>
>> Reviewed-by: Emil Velikov 
>
> I don't have time to dig into details right now but this broke the Mesa
> build on OpenBSD and likely other non-linux platforms:
>
> libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 
> -DPACKAGE_VERSION=\"17.1.0-devel\" "-DPACKAGE_STRING=\"Mesa 17.1.0-devel\"" 
> "-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";
>  -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"17.1.0-devel\" 
> -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 
> -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 
> -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" 
> -DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1 
> -DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1 
> -DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1 
> -DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1 
> -DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 
> -DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1 
> -DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 
> -DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 
> -DHAVE_FUNC_ATTRIBUTE_WEAK=1 -DHAVE_FUNC_ATTRIBUTE_ALIAS=1 -DHAVE_DLADDR=1 
> -DHAVE_CLOCK_GETTIME=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -I. 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DDEBUG 
> -DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H -DHAVE_STRTOF 
> -DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_DL_ITERATE_PHDR -DHAVE_POSIX_MEMALIGN 
> -DHAVE_LIBDRM -DGLX_USE_DRM -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING 
> -DENABLE_SHADER_CACHE -DHAVE_MINCORE -I../../include -I../../src 
> -I../../src/mapi -I../../src/mesa -I../../src/gallium/include 
> -I../../src/gallium/auxiliary -fvisibility=hidden -Werror=pointer-arith -g 
> -O2 -Wall -std=gnu99 -Werror=implicit-function-declaration 
> -Werror=missing-prototypes -fno-math-errno -fno-trapping-math -MT 
> libmesautil_la-build_id.lo -MD -MP -MF .deps/libmesautil_la-build_id.Tpo -c 
> build_id.c  -fPIC -DPIC -o .libs/libmesautil_la-build_id.o
> In file included from /usr/include/elf_abi.h:31,
>  from /usr/include/link_elf.h:10,
>  from /usr/include/link.h:39,
>  from build_id.c:25:
> /usr/include/sys/exec_elf.h:585: error: expected specifier-qualifier-list 
> before 'uint32_t'
> In file included from /usr/include/link.h:39,
>  from build_id.c:25:
> /usr/include/link_elf.h:22: error: expected specifier-qualifier-list before 
> 'caddr_t'
> /usr/include/link_elf.h:37: error: expected '=', ',', ';', 'asm' or 
> '__attribute__' before 'int'
> In file included from build_id.c:25:
> /usr/include/link.h:49: error: expected '=', ',', ';', 'asm' or 
> '__attribute__' before 'struct'
> /usr/include/link.h:65: error: expected specifier-qualifier-list before 
> 'caddr_t'
These look like issue in your platform code/headers. Perhaps some bad
interaction with the bits that Mesa defines ?

Quick workaround is to check the function only when needed, roughly
like this pseudo code:

if test $building_any_vulkan_driver = yes ;then
require_dl...=yes
   
fi


if test $require_dl... = yes ; then
   AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
-DHAVE_DL_ITERATE_PHDR"], [AC_MSG_ERROR([required  not found])])
fi


Please give it a bash and send us a patch that works on your end.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Jonathan Gray]

On Wed, Feb 15, 2017 at 11:11:50AM -0800, Matt Turner 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.
> 
> Reviewed-by: Emil Velikov 

I don't have time to dig into details right now but this broke the Mesa
build on OpenBSD and likely other non-linux platforms:

libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 
-DPACKAGE_VERSION=\"17.1.0-devel\" "-DPACKAGE_STRING=\"Mesa 17.1.0-devel\"" 
"-DPACKAGE_BUGREPORT=\"https://bugs.freedesktop.org/enter_bug.cgi?product=Mesa\";
 -DPACKAGE_URL=\"\" -DPACKAGE=\"mesa\" -DVERSION=\"17.1.0-devel\" 
-DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 
-DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 
-DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" 
-DYYTEXT_POINTER=1 -DHAVE___BUILTIN_CLZ=1 -DHAVE___BUILTIN_CLZLL=1 
-DHAVE___BUILTIN_CTZ=1 -DHAVE___BUILTIN_EXPECT=1 -DHAVE___BUILTIN_FFS=1 
-DHAVE___BUILTIN_FFSLL=1 -DHAVE___BUILTIN_POPCOUNT=1 
-DHAVE___BUILTIN_POPCOUNTLL=1 -DHAVE_FUNC_ATTRIBUTE_CONST=1 
-DHAVE_FUNC_ATTRIBUTE_FLATTEN=1 -DHAVE_FUNC_ATTRIBUTE_FORMAT=1 
-DHAVE_FUNC_ATTRIBUTE_MALLOC=1 -DHAVE_FUNC_ATTRIBUTE_PACKED=1 
-DHAVE_FUNC_ATTRIBUTE_PURE=1 -DHAVE_FUNC_ATTRIBUTE_UNUSED=1 
-DHAVE_FUNC_ATTRIBUTE_VISIBILITY=1 -DHAVE_FUNC_ATTRIBUTE_WARN_UNUSED_RESULT=1 
-DHAVE_FUNC_ATTRIBUTE_WEAK=1 -DHAVE_FUNC_ATTRIBUTE_ALIAS=1 -DHAVE_DLADDR=1 
-DHAVE_CLOCK_GETTIME=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 -I. 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DDEBUG 
-DTEXTURE_FLOAT_ENABLED -DUSE_X86_64_ASM -DHAVE_SYS_SYSCTL_H -DHAVE_STRTOF 
-DHAVE_MKOSTEMP -DHAVE_DLOPEN -DHAVE_DL_ITERATE_PHDR -DHAVE_POSIX_MEMALIGN 
-DHAVE_LIBDRM -DGLX_USE_DRM -DGLX_INDIRECT_RENDERING -DGLX_DIRECT_RENDERING 
-DENABLE_SHADER_CACHE -DHAVE_MINCORE -I../../include -I../../src 
-I../../src/mapi -I../../src/mesa -I../../src/gallium/include 
-I../../src/gallium/auxiliary -fvisibility=hidden -Werror=pointer-arith -g -O2 
-Wall -std=gnu99 -Werror=implicit-function-declaration 
-Werror=missing-prototypes -fno-math-errno -fno-trapping-math -MT 
libmesautil_la-build_id.lo -MD -MP -MF .deps/libmesautil_la-build_id.Tpo -c 
build_id.c  -fPIC -DPIC -o .libs/libmesautil_la-build_id.o
In file included from /usr/include/elf_abi.h:31,
 from /usr/include/link_elf.h:10,
 from /usr/include/link.h:39,
 from build_id.c:25:
/usr/include/sys/exec_elf.h:585: error: expected specifier-qualifier-list 
before 'uint32_t'
In file included from /usr/include/link.h:39,
 from build_id.c:25:
/usr/include/link_elf.h:22: error: expected specifier-qualifier-list before 
'caddr_t'
/usr/include/link_elf.h:37: error: expected '=', ',', ';', 'asm' or 
'__attribute__' before 'int'
In file included from build_id.c:25:
/usr/include/link.h:49: error: expected '=', ',', ';', 'asm' or '__attribute__' 
before 'struct'
/usr/include/link.h:65: error: expected specifier-qualifier-list before 
'caddr_t'
build_id.c:34: error: expected specifier-qualifier-list before 'ElfW'
build_id.c: In function 'build_id_find_nhdr_callback':
build_id.c:63: error: 'struct build_id_note' has no member named 'nhdr'
build_id.c:63: error: 'NT_GNU_BUILD_ID' undeclared (first use in this function)
build_id.c:63: error: (Each undeclared identifier is reported only once
build_id.c:63: error: for each function it appears in.)
build_id.c:64: error: 'struct build_id_note' has no member named 'nhdr'
build_id.c:65: error: 'struct build_id_note' has no member named 'nhdr'
build_id.c:66: error: 'struct build_id_note' has no member named 'name'
build_id.c:71: warning: implicit declaration of function 'ElfW'
build_id.c:71: error: 'Nhdr' undeclared (first use in this function)
build_id.c:72: error: 'struct build_id_note' has no member named 'nhdr'
build_id.c:73: error: 'struct build_id_note' has no member named 'nhdr'
build_id.c: In function 'build_id_find_nhdr':
build_id.c:90: warning: implicit declaration of function 'dl_iterate_phdr'
build_id.c: In function 'build_id_length':
build_id.c:99: error: 'const struct build_id_note' has no member named 'nhdr'
build_id.c: In function 'build_id_read':
build_id.c:106: error: 'const struct build_id_note' has no member named 
'build_id'
*** Error 1 in target 'libmesautil_la-build_id.lo'
mv -f .deps/libmesautil_la-strndup.Tpo .deps/libmesautil_la-strndup.Plo
mv -f sha1/.deps/libmesautil_la-sha1.Tpo sha1/.deps/libmesautil_la-sha1.Plo
*** Error 1 in src/util (Makefile:730 'libmesautil_la-build_id.lo')
*** Error 1 in src/util (Makefile:919 'all-recursive')
*** Error 2 in src/util (Makefile:596 'all')
*** Error 1 in src (Makefile:819 'all-recursive')
*** Error 2 in src (Makefile:584 'all')

adding a sys/types.h include before link.h gets slightly further

libtool: compile:  gcc -DPACKAGE_NAME=\"Mesa\" -DPACKAGE_TARNAME=\"mesa\" 

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

[Jason Ekstrand]

On Wed, Feb 15, 2017 at 10:57 AM, Matt Turner  wrote:

> On Wed, Feb 15, 2017 at 4:03 AM, Emil Velikov 
> 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  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, )) {
> >> +  return data.note;
> >> +   } else {
> >> +  return NULL;
> >> +   }
> > Nit:
> >
> >if (!dl_iterate_phdr(build_id_find_nhdr_callback, ))
> >   return NULL;
> >
> >return data.note;
> >
> >
> >> diff --git a/src/util/build_id.h b/src/util/build_id.h
> >> new file mode 100644
> >> index 000..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 
>
> Thanks. I'll incorporate these changes and send a v3.
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Wed, Feb 15, 2017 at 4:03 AM, Emil Velikov  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  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, )) {
>> +  return data.note;
>> +   } else {
>> +  return NULL;
>> +   }
> Nit:
>
>if (!dl_iterate_phdr(build_id_find_nhdr_callback, ))
>   return NULL;
>
>return data.note;
>
>
>> diff --git a/src/util/build_id.h b/src/util/build_id.h
>> new file mode 100644
>> index 000..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 

Thanks. I'll incorporate these changes and send a v3.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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  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, )) {
> +  return data.note;
> +   } else {
> +  return NULL;
> +   }
Nit:

   if (!dl_iterate_phdr(build_id_find_nhdr_callback, ))
  return NULL;

   return data.note;


> diff --git a/src/util/build_id.h b/src/util/build_id.h
> new file mode 100644
> index 000..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 

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

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Tue, Feb 14, 2017 at 12:43 PM, Nicholas Miell  wrote:
> On 02/14/2017 12:30 PM, Chad Versace wrote:
>>
>> On Tue 14 Feb 2017, Matt Turner 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 | 109
>>> ++
>>> src/util/build_id.h | 56 
>>> 4 files changed, 169 insertions(+)
>>> create mode 100644 src/util/build_id.c
>>> create mode 100644 src/util/build_id.h
>>
>>
>>
>>> +AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES
>>> -DHAVE_DL_ITERATE_PHDR"])
>>
>>
>> Nice. I wasn't aware of dl_iterate_phdr(). My code for querying the
>> build-id was less slick. It used open(2) on the library, then manually
>> parsed the ElfW(Ehdr) and ElfW(Shdr) to find the build-id node.
>
>
> I also reinvented the build ID lookup wheel and just to record this
> knowledge publicly for posterity:
>
> The struct link_map l_addr field seems to corresponds to the struct
> dl_phdr_info dlpi_addr field.
>
> You can retrieve the struct link_map for a symbol by passing the
> RTLD_DL_LINKMAP flag to dladdr1() or for a library handle returned by
> dlopen() by passing RTLD_DI_LINKMAP to dlinfo().
>
> This means you can find the note section for a loaded library directly
> without having to resort to string comparisons against library names,
which
> is probably more future-proof.

That's really neat. Thanks for the information.

It doesn't look like the BSDs or musl libc support RTLD_DL_LINKMAP. In the
interest of avoiding future problems I'll stick with the string comparison
for now.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Jason Ekstrand]

On Tue, Feb 14, 2017 at 11:34 AM, Chad Versace 
wrote:

> On Tue 14 Feb 2017, Matt Turner wrote:
> > On Tue, Feb 14, 2017 at 11:21 AM, Matt Turner 
> wrote:
> > > On Tue, Feb 14, 2017 at 11:18 AM, Matt Turner 
> wrote:
> > >> On Tue, Feb 14, 2017 at 10:59 AM, Jason Ekstrand <
> ja...@jlekstrand.net> wrote:
> > >>> I'm not sure how I feel about the silent fall-backs.  At least in
> the Vulkan
> > >>> driver, we should fail to compile if we can't get build-id.
> Otherwise,
> > >>> you'll end up compiling a driver that will always fail device
> creation.
> > >>
> > >> That was really an attempt to preempt questions about Windows.
> > >>
> > >> I am happy to drop it.
> > >
> > > Think-o. No, it's necessary for systems that don't have
> > > dl_iterate_phdr (Windows, AFAIK).
> >
> > I promise I'll stop replying to myself after this...
> >
> > I guess dropping the fallback and simply wrapping build_id.c in
> > HAVE_DL_ITERATE_PHDR is sufficient? Windows will build an empty source
> > file, which is fine because no code should ever attempt to use it on
> > Windows; and there would be no chance of the Vulkan driver calling
> > (non-existent) fallback code.
>
> I'm in favor of dropping the fallback stubs.
>

I don't really care too much how it works so long as anv fails to compile.
I could see the stubs maybe being useful if you wanted to have the shader
cache stuff silently and automatically fall back to disabled.  Or, the
shader cache stuff could have a #if.  It doesn't matter to me.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Nicholas Miell]

On 02/14/2017 12:30 PM, Chad Versace wrote:

On Tue 14 Feb 2017, Matt Turner 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   | 109 ++
 src/util/build_id.h   |  56 
 4 files changed, 169 insertions(+)
 create mode 100644 src/util/build_id.c
 create mode 100644 src/util/build_id.h




+AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES -DHAVE_DL_ITERATE_PHDR"])


Nice. I wasn't aware of dl_iterate_phdr(). My code for querying the
build-id was less slick. It used open(2) on the library, then manually
parsed the ElfW(Ehdr) and ElfW(Shdr) to find the build-id node.


I also reinvented the build ID lookup wheel and just to record this 
knowledge publicly for posterity:


The struct link_map l_addr field seems to corresponds to the struct 
dl_phdr_info dlpi_addr field.


You can retrieve the struct link_map for a symbol by passing the 
RTLD_DL_LINKMAP flag to dladdr1() or for a library handle returned by 
dlopen() by passing RTLD_DI_LINKMAP to dlinfo().


This means you can find the note section for a loaded library directly 
without having to resort to string comparisons against library names, 
which is probably more future-proof.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Chad Versace]

On Tue 14 Feb 2017, Matt Turner 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   | 109 
> ++
>  src/util/build_id.h   |  56 
>  4 files changed, 169 insertions(+)
>  create mode 100644 src/util/build_id.c
>  create mode 100644 src/util/build_id.h


> +AC_CHECK_FUNC([dl_iterate_phdr], [DEFINES="$DEFINES -DHAVE_DL_ITERATE_PHDR"])

Nice. I wasn't aware of dl_iterate_phdr(). My code for querying the
build-id was less slick. It used open(2) on the library, then manually
parsed the ElfW(Ehdr) and ElfW(Shdr) to find the build-id node.


> diff --git a/src/util/build_id.c b/src/util/build_id.c
> new file mode 100644
> index 000..a2e21b7
> --- /dev/null
> +++ b/src/util/build_id.c


> +#define ALIGN(val, align)  (((val) + (align) - 1) & ~((align) - 1))
> +
> +struct note {
> +   ElfW(Nhdr) nhdr;
> +
> +   char name[4];

Because nothing requires ElfW(Nhdr).n_namesz to be 4, please a comment
here explaining that we hardcoded 4 because the note name is "GNU".

> +   uint8_t build_id[0];
> +};
> +
> +struct callback_data {
> +   const char *name;
> +   struct note *note;
> +};
> +
> +static int
> +build_id_find_nhdr_callback(struct dl_phdr_info *info, size_t size, void 
> *data_)
> +{

This function looks correct to me.

> +   struct callback_data *data = data_;
> +
> +   char *ptr = strstr(info->dlpi_name, data->name);
> +   if (ptr == NULL || ptr[strlen(data->name)] != '\0')
> +  return 0;
> +
> +   for (unsigned i = 0; i < info->dlpi_phnum; i++) {
> +  if (info->dlpi_phdr[i].p_type != PT_NOTE)
> + continue;
> +
> +  struct note *note = (void *)(info->dlpi_addr +
> +   info->dlpi_phdr[i].p_vaddr);

> +  ptrdiff_t len = info->dlpi_phdr[i].p_filesz;

I was initially worried that the len should use p_memsz instead of
p_filesz. But the elf manpage says it's ok; if p_memsz > p_filesz, then
the excess bytes are mere padding.

> +
> +  while (len >= sizeof(struct note)) {
> + if (note->nhdr.n_type == NT_GNU_BUILD_ID &&
> +note->nhdr.n_descsz != 0 &&
> +note->nhdr.n_namesz == 4 &&
> +memcmp(note->name, "GNU", 4) == 0) {
> +data->note = note;
> +return 1;
> + }
> +
> + size_t offset = sizeof(ElfW(Nhdr)) +
> + ALIGN(note->nhdr.n_namesz, 4) +
> + ALIGN(note->nhdr.n_descsz, 4);
> + note = (struct note *)((char *)note + offset);
> + len -= offset;
> +  }
> +   }
> +
> +   return 0;
> +}
> +
> +const struct note *
> +build_id_find_nhdr(const char *name)

Please rename the 'name' parameter to be clearer. I was confused,
thinking that name was the name of ELF section for the note. It wasn't
until I read patch 2, and saw name="libvulkan.so" instead
name=".note.gnu.build-id" that I was able to understand this patch.

> +{
> +   struct callback_data data = {
> +  .name = name,
> +  .note = NULL,
> +   };
> +
> +   if (dl_iterate_phdr(build_id_find_nhdr_callback, )) {
> +  return data.note;
> +   } else {
> +  return NULL;
> +   }
> +}
> +
> +unsigned
> +build_id_length(const struct note *note)
> +{
> +   return note->nhdr.n_descsz;
> +}
> +
> +void
> +build_id_read(const struct note *note, unsigned char *build_id)
> +{

As I explain in patch 2, I think this function needs a 'size' parameter,
à la snprintf, due to Vulkan weirdness regarding restrictons on usage of
malloc.

> +   memcpy(build_id, note->build_id, note->nhdr.n_descsz);
> +}

> diff --git a/src/util/build_id.h b/src/util/build_id.h
> new file mode 100644
> index 000..0eaecf9
> --- /dev/null
> +++ b/src/util/build_id.h


> +struct note;

Please namespace struct note. Please :)

My only hard request is the size_t parameter for build_id_read(). My
other comments are just minor nits, that you can ignore.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Tue, Feb 14, 2017 at 11:39 AM, Kristian H. Kristensen
 wrote:
> Matt Turner  writes:
>> diff --git a/src/util/build_id.c b/src/util/build_id.c
>> new file mode 100644
>> index 000..a2e21b7
>> --- /dev/null
>> +++ b/src/util/build_id.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * Copyright © 2016 Intel Corporation
>
> I don't like it either, but we're in 2017 now.

Heh. I actually did write it in 2016.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Kristian H. Kristensen]

Matt Turner  writes:

> 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   | 109 
> ++
>  src/util/build_id.h   |  56 
>  4 files changed, 169 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"])
> +
>  case "$host_os" in
>  darwin*)
>  ;;
> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
> index a68a5fe..4c12e5f 100644
> --- a/src/util/Makefile.sources
> +++ b/src/util/Makefile.sources
> @@ -2,6 +2,8 @@ MESA_UTIL_FILES :=\
>   bitscan.c \
>   bitscan.h \
>   bitset.h \
> + build_id.c \
> + build_id.h \
>   crc32.c \
>   crc32.h \
>   debug.c \
> diff --git a/src/util/build_id.c b/src/util/build_id.c
> new file mode 100644
> index 000..a2e21b7
> --- /dev/null
> +++ b/src/util/build_id.c
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright © 2016 Intel Corporation

I don't like it either, but we're in 2017 now.

Kristian

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifdef HAVE_DL_ITERATE_PHDR
> +#include 
> +#include 
> +#include 
> +
> +#include "build_id.h"
> +
> +#define ALIGN(val, align)  (((val) + (align) - 1) & ~((align) - 1))
> +
> +struct note {
> +   ElfW(Nhdr) nhdr;
> +
> +   char name[4];
> +   uint8_t build_id[0];
> +};
> +
> +struct callback_data {
> +   const char *name;
> +   struct note *note;
> +};
> +
> +static int
> +build_id_find_nhdr_callback(struct dl_phdr_info *info, size_t size, void 
> *data_)
> +{
> +   struct callback_data *data = data_;
> +
> +   char *ptr = strstr(info->dlpi_name, data->name);
> +   if (ptr == NULL || ptr[strlen(data->name)] != '\0')
> +  return 0;
> +
> +   for (unsigned i = 0; i < info->dlpi_phnum; i++) {
> +  if (info->dlpi_phdr[i].p_type != PT_NOTE)
> + continue;
> +
> +  struct note *note = (void *)(info->dlpi_addr +
> +   info->dlpi_phdr[i].p_vaddr);
> +  ptrdiff_t len = info->dlpi_phdr[i].p_filesz;
> +
> +  while (len >= sizeof(struct note)) {
> + if (note->nhdr.n_type == NT_GNU_BUILD_ID &&
> +note->nhdr.n_descsz != 0 &&
> +note->nhdr.n_namesz == 4 &&
> +memcmp(note->name, "GNU", 4) == 0) {
> +data->note = note;
> +return 1;
> + }
> +
> + size_t offset = sizeof(ElfW(Nhdr)) +
> + ALIGN(note->nhdr.n_namesz, 4) +
> + ALIGN(note->nhdr.n_descsz, 4);
> + note = (struct note *)((char *)note + offset);
> + len -= offset;
> +  }
> +   }
> +
> +   return 0;
> +}
> +
> +const struct note *
> +build_id_find_nhdr(const char *name)
> +{
> +   struct callback_data data = {
> +  .name = name,
> +  .note = NULL,
> +   };
> +
> +   if (dl_iterate_phdr(build_id_find_nhdr_callback, )) {
> +  return data.note;
> +   } else {
> +  return NULL;
> +   }
> +}
> +
> +unsigned
> +build_id_length(const struct note *note)
> +{
> +   return note->nhdr.n_descsz;
> +}
> +
> +void
> +build_id_read(const struct note *note, unsigned char *build_id)
> +{
> +   memcpy(build_id, note->build_id, note->nhdr.n_descsz);
> +}
> +
> +#endif
> diff --git a/src/util/build_id.h b/src/util/build_id.h
> new file mode 100644
> index 000..0eaecf9
> --- /dev/null
> +++ b/src/util/build_id.h

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

[Chad Versace]

On Tue 14 Feb 2017, Matt Turner 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.

Heh. Sunday night I wrote a prototype of this myself. I'm strongly in
favor of basing Vulkan unique id's on the ELF build-id instead of the
past and current approaches we've used.

Still reviewing the patch...

> ---
>  configure.ac  |   2 +
>  src/util/Makefile.sources |   2 +
>  src/util/build_id.c   | 109 
> ++
>  src/util/build_id.h   |  56 
>  4 files changed, 169 insertions(+)
>  create mode 100644 src/util/build_id.c
>  create mode 100644 src/util/build_id.h
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Chad Versace]

On Tue 14 Feb 2017, Matt Turner wrote:
> On Tue, Feb 14, 2017 at 11:21 AM, Matt Turner  wrote:
> > On Tue, Feb 14, 2017 at 11:18 AM, Matt Turner  wrote:
> >> On Tue, Feb 14, 2017 at 10:59 AM, Jason Ekstrand  
> >> wrote:
> >>> I'm not sure how I feel about the silent fall-backs.  At least in the 
> >>> Vulkan
> >>> driver, we should fail to compile if we can't get build-id.  Otherwise,
> >>> you'll end up compiling a driver that will always fail device creation.
> >>
> >> That was really an attempt to preempt questions about Windows.
> >>
> >> I am happy to drop it.
> >
> > Think-o. No, it's necessary for systems that don't have
> > dl_iterate_phdr (Windows, AFAIK).
> 
> I promise I'll stop replying to myself after this...
> 
> I guess dropping the fallback and simply wrapping build_id.c in
> HAVE_DL_ITERATE_PHDR is sufficient? Windows will build an empty source
> file, which is fine because no code should ever attempt to use it on
> Windows; and there would be no chance of the Vulkan driver calling
> (non-existent) fallback code.

I'm in favor of dropping the fallback stubs.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Tue, Feb 14, 2017 at 11:21 AM, Matt Turner  wrote:
> On Tue, Feb 14, 2017 at 11:18 AM, Matt Turner  wrote:
>> On Tue, Feb 14, 2017 at 10:59 AM, Jason Ekstrand  
>> wrote:
>>> I'm not sure how I feel about the silent fall-backs.  At least in the Vulkan
>>> driver, we should fail to compile if we can't get build-id.  Otherwise,
>>> you'll end up compiling a driver that will always fail device creation.
>>
>> That was really an attempt to preempt questions about Windows.
>>
>> I am happy to drop it.
>
> Think-o. No, it's necessary for systems that don't have
> dl_iterate_phdr (Windows, AFAIK).

I promise I'll stop replying to myself after this...

I guess dropping the fallback and simply wrapping build_id.c in
HAVE_DL_ITERATE_PHDR is sufficient? Windows will build an empty source
file, which is fine because no code should ever attempt to use it on
Windows; and there would be no chance of the Vulkan driver calling
(non-existent) fallback code.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Tue, Feb 14, 2017 at 11:18 AM, Matt Turner  wrote:
> On Tue, Feb 14, 2017 at 10:59 AM, Jason Ekstrand  wrote:
>> I'm not sure how I feel about the silent fall-backs.  At least in the Vulkan
>> driver, we should fail to compile if we can't get build-id.  Otherwise,
>> you'll end up compiling a driver that will always fail device creation.
>
> That was really an attempt to preempt questions about Windows.
>
> I am happy to drop it.

Think-o. No, it's necessary for systems that don't have
dl_iterate_phdr (Windows, AFAIK).
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Matt Turner]

On Tue, Feb 14, 2017 at 10:59 AM, Jason Ekstrand  wrote:
> I'm not sure how I feel about the silent fall-backs.  At least in the Vulkan
> driver, we should fail to compile if we can't get build-id.  Otherwise,
> you'll end up compiling a driver that will always fail device creation.

That was really an attempt to preempt questions about Windows.

I am happy to drop it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Jason Ekstrand]

On Tue, Feb 14, 2017 at 10:51 AM, Matt Turner  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   | 109 ++
> 
>  src/util/build_id.h   |  56 
>  4 files changed, 169 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"])
> +
>  case "$host_os" in
>  darwin*)
>  ;;
> diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources
> index a68a5fe..4c12e5f 100644
> --- a/src/util/Makefile.sources
> +++ b/src/util/Makefile.sources
> @@ -2,6 +2,8 @@ MESA_UTIL_FILES :=  \
> bitscan.c \
> bitscan.h \
> bitset.h \
> +   build_id.c \
> +   build_id.h \
> crc32.c \
> crc32.h \
> debug.c \
> diff --git a/src/util/build_id.c b/src/util/build_id.c
> new file mode 100644
> index 000..a2e21b7
> --- /dev/null
> +++ b/src/util/build_id.c
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifdef HAVE_DL_ITERATE_PHDR
> +#include 
> +#include 
> +#include 
> +
> +#include "build_id.h"
> +
> +#define ALIGN(val, align)  (((val) + (align) - 1) & ~((align) - 1))
> +
> +struct note {
> +   ElfW(Nhdr) nhdr;
> +
> +   char name[4];
> +   uint8_t build_id[0];
> +};
> +
> +struct callback_data {
> +   const char *name;
> +   struct note *note;
> +};
> +
> +static int
> +build_id_find_nhdr_callback(struct dl_phdr_info *info, size_t size, void
> *data_)
> +{
> +   struct callback_data *data = data_;
> +
> +   char *ptr = strstr(info->dlpi_name, data->name);
> +   if (ptr == NULL || ptr[strlen(data->name)] != '\0')
> +  return 0;
> +
> +   for (unsigned i = 0; i < info->dlpi_phnum; i++) {
> +  if (info->dlpi_phdr[i].p_type != PT_NOTE)
> + continue;
> +
> +  struct note *note = (void *)(info->dlpi_addr +
> +   info->dlpi_phdr[i].p_vaddr);
> +  ptrdiff_t len = info->dlpi_phdr[i].p_filesz;
> +
> +  while (len >= sizeof(struct note)) {
> + if (note->nhdr.n_type == NT_GNU_BUILD_ID &&
> +note->nhdr.n_descsz != 0 &&
> +note->nhdr.n_namesz == 4 &&
> +memcmp(note->name, "GNU", 4) == 0) {
> +data->note = note;
> +return 1;
> + }
> +
> + size_t offset = sizeof(ElfW(Nhdr)) +
> + ALIGN(note->nhdr.n_namesz, 4) +
> + ALIGN(note->nhdr.n_descsz, 4);
> + note = (struct note *)((char *)note + offset);
> + len -= offset;
> +  }
> +   }
> +
> +   return 0;
> +}
> +
> +const struct note *
> +build_id_find_nhdr(const char *name)
> +{
> +   struct callback_data data = {
> +  .name = name,
> +  .note = NULL,
> +   };
> +
> +   if (dl_iterate_phdr(build_id_find_nhdr_callback, )) {
> +  return data.note;
> +   } else {
> +  return NULL;
> +   }
> +}
> +
> +unsigned
> +build_id_length(const struct note *note)
> +{
> +   return note->nhdr.n_descsz;
> +}
> +
> +void
> +build_id_read(const struct note *note, unsigned char *build_id)
> +{
> +   memcpy(build_id, note->build_id, note->nhdr.n_descsz);
> +}
> +
> +#endif
> diff --git a/src/util/build_id.h b/src/util/build_id.h
> new file mode 100644
> index 000..0eaecf9
> --- /dev/null
> +++