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

[Eric Engestrom]

On Tuesday, 2016-12-06 11:04:03 +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.
> 
> 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..611e676 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;
> +
> +   assert(sizeof(qword) == sizeof(depth));

These sizes won't change at runtime; make it a static_assert()?

With that changed,
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

Cheers,
  Eric

> +   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
>