Package: libplist Severity: normal Tags: patch Hi,
I've spent some time debugging a number of portability issues in libplist; the attached patch should fix libplist on most architectures. There remains a couple of unaligned accesses that I haven't been able to track down, due to prctl --unaligned=signal not working on albeniz (kernel still handling the unaligned trap instead of sending a SIGBUS). Patch tested on amd64, alpha, hppa. Please prepare an upload with this patch added and hopefully we'll have some good news this week-end :) Thanks, JB. -- System Information: Debian Release: squeeze/sid APT prefers unstable APT policy: (500, 'unstable') Architecture: amd64 (x86_64) Kernel: Linux 2.6.32.2 (SMP w/2 CPU cores) Locale: LANG=C, lc_ctype=fr...@euro (charmap=ISO-8859-15) Shell: /bin/sh linked to /bin/bash
Endianness, alignment and type-punning fixes for binary plist support - endianness issues: on big endian machines, writing out only part of an integer was broken (get_needed_bytes(x) < sizeof(x)) -> shift integer before memcpy() on big endian machines - alignment issues: unaligned reads when loading binary plist. Leads to slow runtime performance (kernel trapping and fixing things up), SIGBUS (kernel not helping us out) -> introduce get_unaligned() and have the compiler generate the code needed for the unaligned access (note that there remains unaligned accesses that I haven't been able to track down - I've seen 2 of them with test #2) - type-punning issues: breaking strict aliasing rules can lead to unexpected results as the compiler takes full advantage of the aliasing while optimizing -> introduce the plist_uint_ptr union instead of casting pointers Tested on amd64, alpha and hppa. -- Julien BLACHE <jbla...@debian.org> diff -ru orig/libplist-1.2/src/bplist.c libplist-1.2/src/bplist.c --- orig/libplist-1.2/src/bplist.c 2010-04-09 19:26:54.000000000 +0200 +++ libplist-1.2/src/bplist.c 2010-04-09 19:25:47.776737504 +0200 @@ -79,6 +79,24 @@ #endif } +union plist_uint_ptr +{ + void *src; + uint8_t *u8ptr; + uint16_t *u16ptr; + uint32_t *u32ptr; + uint64_t *u64ptr; +}; + +#define get_unaligned(ptr) \ + ({ \ + struct __attribute__((packed)) { \ + typeof(*(ptr)) __v; \ + } *__p = (void *) (ptr); \ + __p->__v; \ + }) + + static void byte_convert(uint8_t * address, size_t size) { #if G_BYTE_ORDER == G_LITTLE_ENDIAN @@ -95,23 +113,36 @@ #endif } -static uint32_t uint24_from_be(char *buff) +static uint32_t uint24_from_be(union plist_uint_ptr buf) { + union plist_uint_ptr tmp; uint32_t ret = 0; - uint8_t *tmp = (uint8_t *) &ret; - memcpy(tmp + 1, buff, 3 * sizeof(char)); - byte_convert(tmp, sizeof(uint32_t)); + + tmp.src = &ret; + + memcpy(tmp.u8ptr + 1, buf.u8ptr, 3 * sizeof(char)); + + byte_convert(tmp.u8ptr, sizeof(uint32_t)); return ret; } #define UINT_TO_HOST(x, n) \ - (n == 8 ? GUINT64_FROM_BE( *(uint64_t *)(x) ) : \ - (n == 4 ? GUINT32_FROM_BE( *(uint32_t *)(x) ) : \ - (n == 3 ? uint24_from_be( (char*)x ) : \ - (n == 2 ? GUINT16_FROM_BE( *(uint16_t *)(x) ) : \ - *(uint8_t *)(x) )))) - -#define be64dec(x) GUINT64_FROM_BE( *(uint64_t*)(x) ) + ({ \ + union plist_uint_ptr __up; \ + __up.src = x; \ + (n == 8 ? GUINT64_FROM_BE( get_unaligned(__up.u64ptr) ) : \ + (n == 4 ? GUINT32_FROM_BE( get_unaligned(__up.u32ptr) ) : \ + (n == 3 ? uint24_from_be( __up ) : \ + (n == 2 ? GUINT16_FROM_BE( get_unaligned(__up.u16ptr) ) : \ + *__up.u8ptr )))); \ + }) + +#define be64dec(x) \ + ({ \ + union plist_uint_ptr __up; \ + __up.src = x; \ + GUINT64_FROM_BE( get_unaligned(__up.u64ptr) ); \ + }) #define get_needed_bytes(x) \ ( ((uint64_t)x) < (1ULL << 8) ? 1 : \ @@ -646,6 +677,11 @@ //do not write 3bytes int node if (size == 3) size++; + +#if G_BYTE_ORDER == G_BIG_ENDIAN + val = val << ((sizeof(uint64_t) - size) * 8); +#endif + buff = (uint8_t *) malloc(sizeof(uint8_t) + size); buff[0] = BPLIST_UINT | Log2(size); memcpy(buff + 1, &val, size); @@ -656,7 +692,7 @@ static void write_real(GByteArray * bplist, double val) { - uint64_t size = get_real_bytes(*((uint64_t *) & val)); //cheat to know used space + uint64_t size = get_real_bytes(val); //cheat to know used space uint8_t *buff = (uint8_t *) malloc(sizeof(uint8_t) + size); buff[0] = BPLIST_REAL | Log2(size); if (size == sizeof(double)) @@ -748,6 +784,9 @@ for (i = 0, cur = node->children; cur && i < size; cur = cur->next, i++) { idx = *(uint64_t *) (g_hash_table_lookup(ref_table, cur)); +#if G_BYTE_ORDER == G_BIG_ENDIAN + idx = idx << ((sizeof(uint64_t) - dict_param_size) * 8); +#endif memcpy(buff + i * dict_param_size, &idx, dict_param_size); byte_convert(buff + i * dict_param_size, dict_param_size); } @@ -783,10 +822,16 @@ for (i = 0, cur = node->children; cur && i < size; cur = cur->next->next, i++) { idx1 = *(uint64_t *) (g_hash_table_lookup(ref_table, cur)); +#if G_BYTE_ORDER == G_BIG_ENDIAN + idx1 = idx1 << ((sizeof(uint64_t) - dict_param_size) * 8); +#endif memcpy(buff + i * dict_param_size, &idx1, dict_param_size); byte_convert(buff + i * dict_param_size, dict_param_size); idx2 = *(uint64_t *) (g_hash_table_lookup(ref_table, cur->next)); +#if G_BYTE_ORDER == G_BIG_ENDIAN + idx2 = idx2 << ((sizeof(uint64_t) - dict_param_size) * 8); +#endif memcpy(buff + (i + size) * dict_param_size, &idx2, dict_param_size); byte_convert(buff + (i + size) * dict_param_size, dict_param_size); } @@ -916,7 +961,12 @@ for (i = 0; i < num_objects; i++) { uint8_t *offsetbuff = (uint8_t *) malloc(offset_size); - memcpy(offsetbuff, offsets + i, offset_size); + +#if G_BYTE_ORDER == G_BIG_ENDIAN + offsets[i] = offsets[i] << ((sizeof(uint64_t) - offset_size) * 8); +#endif + + memcpy(offsetbuff, &offsets[i], offset_size); byte_convert(offsetbuff, offset_size); g_byte_array_append(bplist_buff, offsetbuff, offset_size); free(offsetbuff);