[Spice-devel] [PATCH spice-gtk v7 2/4] Avoid clang warnings on casts with stricter alignment requirements

[Christophe Fergeau]

On Thu, Jun 08, 2017 at 06:17:31AM -0400, Frediano Ziglio wrote:
> > 
> > From: Christophe de Dinechin <dinechin at redhat.com>
> > 
> > For example, something like this:
> >     uint8_t  *p8;
> >     uint32_t *p32 = (uint32_t *) p8;
> > 
> > generates a warning like this:
> >   spice-channel.c:1350:10: error: cast from 'uint8_t *' (aka 'unsigned char
> >   *') to
> >       'uint32_t *' (aka 'unsigned int *') increases required alignment from 1
> >       to
> >       4 [-Werror,-Wcast-align]
> > 
> > The warning indicates that we end up with a pointer to data that
> > should be 4-byte aligned, but its value may be misaligned. On x86,
> > this does not make much of a difference, except a relatively minor
> > performance penalty. However, on platforms such as older ARM, misaligned
> > accesses are emulated by the kernel, and support for them is optional.
> > So we may end up with a fault.
> > 
> > The intent of the fix here is to make it easy to identify and rework
> > places where actual mis-alignment occurs. Wherever casts raise the warning,
> > they are replaced with a macro:
> > 
> > - SPICE_ALIGNED_CAST(type, value) casts value to type, and indicates that
> >   we believe the resulting pointer is aligned. If it is not, a runtime
> >   warning will be issued.
> > 
> > - SPICE_UNALIGNED_CAST(type, value) casts value to type, and indicates that
> >   we believe the resulting pointer is not always aligned
> > 
> > Any code using SPICE_UNALIGNED_CAST may need to be revisited in order
> > to improve performance, e.g. by using memcpy.
> > 
> > There are normally no warnings for SPICE_UNALIGNED_CAST, but it is possible
> > to emit debug messages for mis-alignment in SPICE_UNALIGNED_CAST.
> > Set environment variable SPICE_DEBUG_ALIGNMENT to activate this check.
> > 
> > Runtime warnings are enabled by the --enable-alignment-checks configure
> > option (or #define SPICE_DEBUG_ALIGNMENT).
> > 
> > Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> 
> Honestly I would start nacking this patch.
> 
> Or you disable the warning or you fix it where possible and leave
> the warning for a time when we'll really need to fix it.

This patch adds annotations indicating whether the data is known to be
aligned or not, this is better than just disabling the warning.
Then you can ask the question of what should the macro implementation
do, as it could just be a no-op, and we remove -Waligned-cast.
Personally I'd keep the macro which does the appropriate cast to silence
the warning, and have optional runtime checks if needed.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170608/0c5824d1/attachment.sig>