[Spice-devel] [PATCH spice-common] canvas_base: Avoid misaligned access decoding LZ4 data

[Frediano Ziglio]

> 
> On Thu, Jun 28, 2018 at 07:40:19AM +0100, Frediano Ziglio wrote:
> > Make code faster on platforms not supporting unaligned access
> > by default.
> 
> How does this work exactly? Rather than silencing the warning with
> SPICE_UNALIGNED_CAST, and then potentially dereferencing a non-aligned
> 32 bit value, and getting a trap from the kernel/cpu, we instead give a
> lot of hints to the compiler to let it know the value is not going to be
> aligned, and we expect it's going to emit the right code?
> 
> Christophe
> 

Same "trick" used in the marshaller. Yes, compiler will generate the
right code. The SPICE_UNALIGNED_CAST marks that the code is doing
a misaligned access (or helps discovering it), does not solve the problem.
This solves the problem :-)

OT: yes, strangely this is encoded as big endian!

Frediano

> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  common/canvas_base.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/common/canvas_base.c b/common/canvas_base.c
> > index 6bf6e5d..36546b3 100644
> > --- a/common/canvas_base.c
> > +++ b/common/canvas_base.c
> > @@ -45,6 +45,14 @@
> >  #include "mem.h"
> >  #include "macros.h"
> >  
> > +#include <spice/start-packed.h>
> > +typedef struct SPICE_ATTR_PACKED {
> > +    uint32_t v;
> > +} uint32_unaligned_t;
> > +#include <spice/end-packed.h>
> > +
> > +#define read_uint32_be(ptr) ntohl(((uint32_unaligned_t *)(ptr))->v)
> > +
> >  #define ROUND(_x) ((int)floor((_x) + 0.5))
> >  
> >   static inline int fix_to_int(SPICE_FIXED28_4 fixed)
> > @@ -572,7 +580,7 @@ static pixman_image_t *canvas_get_lz4(CanvasBase
> > *canvas, SpiceImage *image)
> >  
> >      do {
> >          // Read next compressed block
> > -        enc_size = ntohl(*SPICE_UNALIGNED_CAST(uint32_t *, data));
> > +        enc_size = read_uint32_be(data);
> >          data += 4;
> >          dec_size = LZ4_decompress_safe_continue(stream, (const char *)
> >          data,
> >                                                  (char *) dest, enc_size,
> >                                                  available);