[Mesa-dev] [PATCH] gallium: Fix blend color alignment for 32-bit systems
nhaehnle at gmail.com
Wed Jul 13 08:15:00 UTC 2016
On 12.07.2016 15:44, Roland Scheidegger wrote:
> Am 12.07.2016 um 13:40 schrieb Nicolai Hähnle:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>> This fixes a regression introduced by commit d8d6091a8.
>> Heap allocations may be only 8-byte aligned on 32-bit system, and so having
>> members with 16-byte alignment (such as in the case where pipe_blend_color is
>> embedded in radeonsi's si_context) is undefined behavior which indeed causes
>> crashes when compiled with gcc -O3.
>> Rather than track down and fix all allocation sites where a pipe_blend_color
>> may be embedded, assume that the original compiler bug only affects 64-bits.
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96835
>> Cc: Tim Rowley <timothy.o.rowley at intel.com>
>> Cc: Chuck Atkins <chuck.atkins at kitware.com>
>> Cc: <mesa-stable at lists.freedesktop.org>
>> This should fix the linked bug report. The big question is whether the
>> assumption about the original compiler problem is correct?
>> src/gallium/include/pipe/p_state.h | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
>> index a73a771..1986495 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -335,8 +335,13 @@ struct pipe_blend_color
>> * unaligned accessors resulting in a segfault. Specifically several
>> * versions of the Intel compiler are known to be affected but it's likely
>> * others are as well.
>> + *
>> + * This only applies on 64-bit architectures, and adding the alignment on
>> + * 32-bit architectures causes bugs because heap allocations are not
>> + * sufficiently aligned.
>> - PIPE_ALIGN_VAR(16) float color;
>> + PIPE_ALIGN_VAR(sizeof(void *) >= 8 ? 16 : 4)
>> + float color;
> Honestly, I'd rather get rid of it, this gets really hacky. (If the
> compiler bug was happening in the driver, it could easily do alignment
> on its own, even if possibly at the cost of a copy).
> Note that technically, malloc allocations aren't guaranteed to be 16
> byte on a 64bit arch. Rather, malloc() has to honor biggest alignment
> for a standard type (and no, __mm128 doesn't count), which on x64 linux
> is long double (128bit). No idea if that's the case everywhere (even if
> it's not, there's of course some possibility it will return 16 byte
> aligned addresses anyway).
Yeah, I'm really not happy about this either. Tim, Chuck, given that the
original fix created a new bug, is this something you could just fix in
the affected driver instead?
More information about the mesa-dev