Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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 b/src/util/build_id.c > >> index 2993a80cfe..92250a1f5f 100644 > >> --- a/src/util/build_id.c
Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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" >> >> +#ifndef NT_GNU_BUILD_ID >> +#define NT_GNU_BUILD_ID 3 >> +#endif >> + >> +#ifn
Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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 no idea about the rest. I think it's Matt's call, since he
Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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.
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.
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\" -DPACKAGE_VERSION=\"17.1.0-devel\" "-D
[Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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 --- configure.ac | 6 +++ src/util/Makefile.sources | 2 + src/util/build_id.c | 109 ++ src/util/build_id.h | 38 4 files changed, 155 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..e4a5b48 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*) ;; @@ -1773,6 +1775,10 @@ AC_ARG_WITH([vulkan-icddir], AC_SUBST([VULKAN_ICD_INSTALL_DIR]) if test -n "$with_vulkan_drivers"; then +if test "x$ac_cv_func_dl_iterate_phdr" = xno; then +AC_MSG_ERROR([Vulkan drivers require the dl_iterate_phdr function]) +fi + VULKAN_DRIVERS=`IFS=', '; echo $with_vulkan_drivers` for driver in $VULKAN_DRIVERS; do case "x$driver" in diff --git a/src/util/Makefile.sources b/src/util/Makefile.sources index eec0311..40d183c 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..2993a80 --- /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 build_id_note { + ElfW(Nhdr) nhdr; + + char name[4]; /* Note name for build-id is "GNU\0" */ + uint8_t build_id[0]; +}; + +struct callback_data { + const char *filename; + struct build_id_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->filename); + if (ptr == NULL || ptr[strlen(data->filename)] != '\0') + return 0; + + for (unsigned i = 0; i < info->dlpi_phnum; i++) { + if (info->dlpi_phdr[i].p_type != PT_NOTE) + continue; + + struct build_id_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 build_id_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 build_id_note *)((char *)note + offset); + len -= offset; + } + } + + return 0; +} + +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 NULL; + + return data.note; +} + +unsigned +build_id_length(const struct build_id_note *note) +{ + return note->nhdr.n_descsz; +} + +void +build_id_read(const struct build_id_note *note, + unsigned char *build_id, size_t n) +{ + memcpy(build_id, note->build_id, n);
Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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, &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 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.
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, &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 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.
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, &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 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
[Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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"]) + 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..80cafe9 --- /dev/null +++ b/src/util/build_id.c @@ -0,0 +1,110 @@ +/* + * 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 build_id_note { + ElfW(Nhdr) nhdr; + + char name[4]; /* Note name for build-id is "GNU\0" */ + uint8_t build_id[0]; +}; + +struct callback_data { + const char *filename; + struct build_id_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->filename); + if (ptr == NULL || ptr[strlen(data->filename)] != '\0') + return 0; + + for (unsigned i = 0; i < info->dlpi_phnum; i++) { + if (info->dlpi_phdr[i].p_type != PT_NOTE) + continue; + + struct build_id_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 build_id_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 build_id_note *)((char *)note + offset); + len -= offset; + } + } + + return 0; +} + +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; + } +} + +unsigned +build_id_length(const struct build_id_note *note) +{ + return note->nhdr.n_descsz; +} + +void +build_id_read(const struct build_id_note *note, + unsigned char *build_id, size_t n) +{ + memcpy(build_id, note->build_id, n); +} + +#endif 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 @@ -0,0 +1,34 @@ +/* + * 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 re
Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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.
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.
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.
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, &data)) { > + 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.
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.
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, &data)) { > + 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 > @@ -0,0 +1,56
Re: [Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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.
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.
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.
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.
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.
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, &data)) { > + 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 > @@ -0
[Mesa-dev] [PATCH 1/2] util: Add utility build-id code.
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, &data)) { + 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 @@ -0,0 +1,56 @@ +/* + * 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