[Mesa-dev] [PATCH v2 1/2] virgl: Fix a strict-aliasing violation in the encoder

[Edward O'Callaghan]

On 12/06/2016 10:48 PM, Eric Engestrom wrote:
> On Tuesday, 2016-12-06 22:30:58 +1100, 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 double
>> to uint64_t interpretations of the memory.
>>
>> V.2: Use static_assert() instead of assert().
>>
>> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
>> Signed-off-by: Edward O'Callaghan <funfunctor at folklore1984.net>
>> ---
>>  src/gallium/drivers/virgl/virgl_encode.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/virgl/virgl_encode.c b/src/gallium/drivers/virgl/virgl_encode.c
>> index be72f70..58890df 100644
>> --- a/src/gallium/drivers/virgl/virgl_encode.c
>> +++ b/src/gallium/drivers/virgl/virgl_encode.c
>> @@ -21,6 +21,8 @@
>>   * USE OR OTHER DEALINGS IN THE SOFTWARE.
>>   */
>>  #include <stdint.h>
>> +#include <assert.h>
>> +#include <string.h>
>>  
>>  #include "util/u_format.h"
>>  #include "util/u_memory.h"
>> @@ -315,12 +317,16 @@ int virgl_encode_clear(struct virgl_context *ctx,
>>                        double depth, unsigned stencil)
>>  {
>>     int i;
>> +   uint64_t qword;
>> +
>> +   static_assert(sizeof(qword) == sizeof(depth), "");
> 
> Please fill in the string :)
> eg.: "`depth` needs to be a 64-bit floating point type"

ACK, done locally.

You can check my branch rather than sending another version:

https://cgit.freedesktop.org/~funfunctor/mesa/log/?h=strict-aliasing-vioations

Good to go? I'll push once the svga one gets Rb'ed also.

Thanks for the review.
Kind Regards,
Edward.

> 
>> +   memcpy(&qword, &depth, sizeof(qword));
>>  
>>     virgl_encoder_write_cmd_dword(ctx, VIRGL_CMD0(VIRGL_CCMD_CLEAR, 0, VIRGL_OBJ_CLEAR_SIZE));
>>     virgl_encoder_write_dword(ctx->cbuf, buffers);
>>     for (i = 0; i < 4; i++)
>>        virgl_encoder_write_dword(ctx->cbuf, color->ui[i]);
>> -   virgl_encoder_write_qword(ctx->cbuf, *(uint64_t *)&depth);
>> +   virgl_encoder_write_qword(ctx->cbuf, qword);
>>     virgl_encoder_write_dword(ctx->cbuf, stencil);
>>     return 0;
>>  }
>> -- 
>> 2.9.3
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161206/41842a81/attachment.sig>