[Mesa-dev] [PATCH v3 2/3] svga: Fix a strict-aliasing violation in shader dumper

Charmaine Lee charmainel at vmware.com
Tue Dec 13 22:50:25 UTC 2016


Change looks fine to me. Thanks.

Reviewed-by: Charmaine Lee <charmainel at vmware.com>
________________________________________
From: Roland Scheidegger
Sent: Tuesday, December 13, 2016 10:38 AM
To: Edward O'Callaghan; mesa-dev at lists.freedesktop.org
Cc: Brian Paul; Charmaine Lee
Subject: Re: [Mesa-dev] [PATCH v3 2/3] svga: Fix a strict-aliasing violation in shader dumper

adding CC: Charmaine

Am 12.12.2016 um 06:23 schrieb Edward O'Callaghan:
> Brian/Roland ping?
>
> On 12/07/2016 10:30 AM, Edward O'Callaghan wrote:
>> As per the C spec, it is illegal to alias pointers to different
>> types. This results in undefined behaviour after optimization
>> passes, resulting in very subtle bugs that happen only on a
>> full moon..
>>
>> Use a memcpy() as a well defined coercion between the isomorphic
>> bit-field interpretations of memory.
>>
>> V.2: Use C99 compat STATIC_ASSERT() over C11 static_assert().
>>
>> Signed-off-by: Edward O'Callaghan <funfunctor at folklore1984.net>
>> ---
>>  src/gallium/drivers/svga/svgadump/svga_shader_dump.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/svga/svgadump/svga_shader_dump.c b/src/gallium/drivers/svga/svgadump/svga_shader_dump.c
>> index 4ee1bf2..46126a5 100644
>> --- a/src/gallium/drivers/svga/svgadump/svga_shader_dump.c
>> +++ b/src/gallium/drivers/svga/svgadump/svga_shader_dump.c
>> @@ -30,6 +30,9 @@
>>   * @author Michal Krol <michal at vmware.com>
>>   */
>>
>> +#include <assert.h>
>> +#include <string.h>
>> +
>>  #include "svga_shader.h"
>>  #include "svga_shader_dump.h"
>>  #include "svga_shader_op.h"
>> @@ -413,6 +416,11 @@ dump_dstreg(struct sh_dstreg dstreg,
>>
>>  static void dump_srcreg( struct sh_srcreg srcreg, struct sh_srcreg *indreg, const struct dump_info *di )
>>  {
>> +   struct sh_reg srcreg_sh = {0};
>> +   /* bit-fields carefully aligned, ensure they stay that way. */
>> +   STATIC_ASSERT(sizeof(struct sh_reg) == sizeof(struct sh_srcreg));
>> +   memcpy(&srcreg_sh, &srcreg, sizeof(srcreg_sh));
>> +
>>     switch (srcreg.modifier) {
>>     case SVGA3DSRCMOD_NEG:
>>     case SVGA3DSRCMOD_BIASNEG:
>> @@ -427,7 +435,7 @@ static void dump_srcreg( struct sh_srcreg srcreg, struct sh_srcreg *indreg, cons
>>     case SVGA3DSRCMOD_NOT:
>>        _debug_printf( "!" );
>>     }
>> -   dump_reg( *(struct sh_reg *) &srcreg, indreg, di );
>> +   dump_reg(srcreg_sh, indreg, di );
>>     switch (srcreg.modifier) {
>>     case SVGA3DSRCMOD_NONE:
>>     case SVGA3DSRCMOD_NEG:
>>
>



More information about the mesa-dev mailing list