[Spice-devel] [PATCH] demarshall: Add safe method for unaligned byte access.

[Christophe Fergeau]

On Fri, Nov 27, 2015 at 07:04:55AM -0500, Frediano Ziglio wrote:
> > 
> > On Wed, Nov 25, 2015 at 09:04:58AM -0500, Frediano Ziglio wrote:
> > > > 
> > > > Usefull for ARM. Enable with "USE_MEMCPY" define.
> > > >  Should be better to add "--use-memcpy" to configure.ac
> > > > 
> > > > Signed-off-by: Anton D. Kachalov <mouse at yandex-team.ru>
> > > 
> > > Are you sure there are not other way except memcpy?
> > 
> > An alternate way which I proposed was to do
> > uint16_t my_int = unaligned_int[0] | unaligned_int[1] << 8
> > but Anton said it was likely to be slower than a memcpy. I don't know
> > how these unaligned accesses are usually handled in ARM code.
> > I found
> > https://github.com/yura-pakhuchiy/snappy-c/commit/a33077391874b36fb4c430ff9ed5f6e437b65f6f
> > doinng something similar, but that's about it.
> > 
> > Christophe
> > 
> 
> Being architecture dependent usually the compiler is the best that know
> how to do it.
> 
> In the gcc word speaking the trick is to create a packed structure,
> turn off aliasing and packing, something like
> 
> struct unaligned_u16
> {
>     uint16_t u16;
> } __attribute__((__may_alias__)) __attribute__((packed));
> 
> static inline uint16_t read_u16ua(const void *p)
> {
>    return ((struct unaligned_u16 *)p)->u16;
> }

Are you sure this is doing the right thing? This is not very far from
what we currently have (struct { uint16_t field1; uint16_t field2; }
__attribute__((packed));
with field1 and field2 ending up unaligned. gcc is apparently not doing
the right thing here, and unaligned memory access traps are triggered,
causing this code to be slow.

Christohpe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151127/9c77d570/attachment.sig>