|
From: Florian K. <fl...@ei...> - 2015-01-19 09:45:41
|
On 19.01.2015 05:00, John Reiser wrote: > The code in LibVEX_Alloc works "by accident". The result happens to be correct > for current architectures (x86, x86_64, PowerPC, PowerPC-64, MIPS, MIPS-64, ARM, > ARM-64) and compilers (gcc, icc, clang, xlc, ...) However, an adversarial compiler > could force LibVEX_Alloc to return a value which is not properly aligned. > In particular, PowerPC-64 is on the cusp. Also, LibVEX_Alloc wastes space. > > The wasted space is easy to see. Suppose these are two consecutive calls: > p1 = LibVEX_Alloc(1); > p2 = LibVEX_Alloc(1); > If p2 and p1 reside in the same allocation block, then > (8 == ((char *)p2 - (char *)p1)) > and there are 7 wasted bytes beginning at (1+ (char *)p1). > > The alignment of the result is "correct by accident" because the code > enforces alignment only to (0 modulo 8). This happens to be enough because > (8 == my_offsetof(struct align,x)) in all current cases. However, it is > easy to imagine a case where (16 == my_offsetof(struct align,x)). > In particular, a compiler for PowerPC-64 is allowed to choose > (16 == alignof(void (*)(void))) > (perhaps motivated by (16 == sizeof(void (*)(void))) ) > and that would be enough to reveal the bug in LibVEX_Alloc. > > The existing code aligns the lowest pointer in an allocation block to a > multiple of 8 using code analogous to > static HChar permanent[N_PERMANENT_BYTES] __attribute__((aligned(8))); > and the code in LibVEX_Alloc does not check for (8 < my_offsetof(struct align,x)). Yes, we should fix the attribute spec. > Both the wasted space and the possible incorrect alignment happen because > LibVEX_Alloc aligns the size. Instead, LibVEX_Alloc should align the pointer: > > static unsigned const alignment_table[16] = [0,0,1,1, 3,3,3,3, 7,7,7,7, 7,7,7,7]; > nbytes += (0 == nbytes); /* 0 ==> 1; ISO C requires that malloc(0) be (GENSYM) */ > ALIGN = ((nbytes >= 0x10) ? 0xf : alignment_table[nbytes]); 0xf seems wasteful to me if only 8 byte alignment is needed. But that can be salvaged and auto-adjusted to whatever the current compiler uses. > curr = (char *)(~ALIGN & (ALIGN + (ptrdiff_t)private_LibVEX_alloc_next); > private_LibVEX_alloc_next = nbytes + curr; > return curr; I would be interested in some numbers. Would you mind running an experiment with, say, the tests from /perf ? Florian |