| Message ID | 20180403160725.GE8577@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix libbacktrace zdebug decompression on big endian (PR other/85161) | expand |
On Tue, Apr 3, 2018 at 9:08 AM Jakub Jelinek <jakub@redhat.com> wrote: > As mentioned in the PR, GCC (and clang) predefines > {__BYTE_ORDER__,__ORDER_{LITTLE,BIG,PDP}_ENDIAN__} > macros, and {,sys/,machine/}endian.h headers predefine > {,__}{BYTE_ORDER,{LITTLE,BIG,PDP}_ENDIAN} > macros (depending on which target and feature test macros). > elf.c in GCC 8 used __BYTE_ORDER, which is endian.h macro, but > didn't include that header and it on glibc just happened to be included > indirectly because of default feature test macros from stdlib.h, > and used non-existing __ORDER_BIG_ENDIAN macro; as __BYTE_ORDER is always > non-zero when defined (1234, 4321 etc.), that means __builtin_bswap32 > was never used. > The following patch just uses the GCC/clang predefined macros if known to be > big or little endian, and otherwise just falls back to portable code (that > good compilers can still optimize). > Bootstrapped/regtested on x86_64-linux and i686-linux and tested on > powerpc64-linux, ok for trunk? > 2018-04-03 Jakub Jelinek <jakub@redhat.com> > PR other/85161 > * elf.c (elf_zlib_fetch): Fix up predefined macro names in test for > big endian, only use 32-bit loads if endianity macros are predefined > and indicate big or little endian. This is OK. Thanks for sorting that out. Ian
--- libbacktrace/elf.c.jj 2018-02-15 12:30:53.948579969 +0100 +++ libbacktrace/elf.c 2018-04-03 14:47:32.536550472 +0200 @@ -1086,12 +1086,19 @@ elf_zlib_fetch (const unsigned char **pp return 0; } +#if defined(__BYTE_ORDER__) && defined(__ORDER_LITTLE_ENDIAN__) \ + && defined(__ORDER_BIG_ENDIAN__) \ + && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ \ + || __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) /* We've ensured that PIN is aligned. */ next = *(const uint32_t *)pin; -#if __BYTE_ORDER == __ORDER_BIG_ENDIAN +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ next = __builtin_bswap32 (next); #endif +#else + next = pin[0] | (pin[1] << 8) | (pin[2] << 16) | (pin[3] << 24); +#endif val |= (uint64_t)next << bits; bits += 32;
Hi! As mentioned in the PR, GCC (and clang) predefines {__BYTE_ORDER__,__ORDER_{LITTLE,BIG,PDP}_ENDIAN__} macros, and {,sys/,machine/}endian.h headers predefine {,__}{BYTE_ORDER,{LITTLE,BIG,PDP}_ENDIAN} macros (depending on which target and feature test macros). elf.c in GCC 8 used __BYTE_ORDER, which is endian.h macro, but didn't include that header and it on glibc just happened to be included indirectly because of default feature test macros from stdlib.h, and used non-existing __ORDER_BIG_ENDIAN macro; as __BYTE_ORDER is always non-zero when defined (1234, 4321 etc.), that means __builtin_bswap32 was never used. The following patch just uses the GCC/clang predefined macros if known to be big or little endian, and otherwise just falls back to portable code (that good compilers can still optimize). Bootstrapped/regtested on x86_64-linux and i686-linux and tested on powerpc64-linux, ok for trunk? 2018-04-03 Jakub Jelinek <jakub@redhat.com> PR other/85161 * elf.c (elf_zlib_fetch): Fix up predefined macro names in test for big endian, only use 32-bit loads if endianity macros are predefined and indicate big or little endian. Jakub