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 <[email protected]>
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);
