From: Christophe de Dinechin <dinechin@xxxxxxxxxx> 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 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 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. Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx> --- spice-common | 2 +- src/channel-cursor.c | 6 +++--- src/channel-display-mjpeg.c | 2 +- src/continuation.h | 6 ++++-- src/decode-glz-tmpl.c | 2 +- src/spice-channel.c | 14 +++++++++----- 6 files changed, 19 insertions(+), 13 deletions(-) diff --git a/spice-common b/spice-common index af682b1..1239c82 160000 --- a/spice-common +++ b/spice-common @@ -1 +1 @@ -Subproject commit af682b1b06dea55007d9aa7c37cd443e4349e43f +Subproject commit 1239c82c54dee0244cca262cca0aa21071b23e24 diff --git a/src/channel-cursor.c b/src/channel-cursor.c index 609243b..650d408 100644 --- a/src/channel-cursor.c +++ b/src/channel-cursor.c @@ -340,7 +340,7 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor) memcpy(cursor->data, data, size); for (i = 0; i < hdr->width * hdr->height; i++) { pix_mask = get_pix_mask(data, size, i); - if (pix_mask && *((guint32*)data + i) == 0xffffff) { + if (pix_mask && *(SPICE_ALIGNED_CAST(guint32 *, data) + i) == 0xffffff) { cursor->data[i] = get_pix_hack(i, hdr->width); } else { cursor->data[i] |= (pix_mask ? 0 : 0xff000000); @@ -350,7 +350,7 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor) case SPICE_CURSOR_TYPE_COLOR16: for (i = 0; i < hdr->width * hdr->height; i++) { pix_mask = get_pix_mask(data, size, i); - pix = *((guint16*)data + i); + pix = *(SPICE_ALIGNED_CAST(guint16 *, data) + i); if (pix_mask && pix == 0x7fff) { cursor->data[i] = get_pix_hack(i, hdr->width); } else { @@ -364,7 +364,7 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor) for (i = 0; i < hdr->width * hdr->height; i++) { pix_mask = get_pix_mask(data, size + (sizeof(uint32_t) << 4), i); int idx = (i & 1) ? (data[i >> 1] & 0x0f) : ((data[i >> 1] & 0xf0) >> 4); - pix = *((uint32_t*)(data + size) + idx); + pix = *(SPICE_UNALIGNED_CAST(uint32_t *, (data + size)) + idx); if (pix_mask && pix == 0xffffff) { cursor->data[i] = get_pix_hack(i, hdr->width); } else { diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c index 3ae9d21..ee33b01 100644 --- a/src/channel-display-mjpeg.c +++ b/src/channel-display-mjpeg.c @@ -151,7 +151,7 @@ static gboolean mjpeg_decoder_decode_frame(gpointer video_decoder) #ifndef JCS_EXTENSIONS { uint8_t *s = lines[0]; - uint32_t *d = (uint32_t *)s; + uint32_t *d = SPICE_ALIGNED_CAST(uint32_t *, s); if (back_compat) { for (unsigned int j = lines_read * width; j > 0; ) { diff --git a/src/continuation.h b/src/continuation.h index 675a257..d1fd137 100644 --- a/src/continuation.h +++ b/src/continuation.h @@ -21,6 +21,7 @@ #ifndef _CONTINUATION_H_ #define _CONTINUATION_H_ +#include "spice-common.h" #include <stddef.h> #include <ucontext.h> #include <setjmp.h> @@ -48,8 +49,9 @@ int cc_release(struct continuation *cc); int cc_swap(struct continuation *from, struct continuation *to); #define offset_of(type, member) ((unsigned long)(&((type *)0)->member)) -#define container_of(obj, type, member) \ - (type *)(((char *)obj) - offset_of(type, member)) +#define container_of(obj, type, member) \ + SPICE_ALIGNED_CAST(type *, \ + (((char *)obj) - offset_of(type, member))) #endif /* diff --git a/src/decode-glz-tmpl.c b/src/decode-glz-tmpl.c index b337a8b..76d832c 100644 --- a/src/decode-glz-tmpl.c +++ b/src/decode-glz-tmpl.c @@ -178,7 +178,7 @@ static size_t FNAME(decode)(SpiceGlzDecoderWindow *window, uint64_t image_id, SpicePalette *plt) { uint8_t *ip = in_buf; - OUT_PIXEL *out_pix_buf = (OUT_PIXEL *)out_buf; + OUT_PIXEL *out_pix_buf = SPICE_ALIGNED_CAST(OUT_PIXEL *, out_buf); OUT_PIXEL *op = out_pix_buf; OUT_PIXEL *op_limit = out_pix_buf + size; diff --git a/src/spice-channel.c b/src/spice-channel.c index 3b6231e..6c52547 100644 --- a/src/spice-channel.c +++ b/src/spice-channel.c @@ -1312,6 +1312,7 @@ static void spice_channel_send_link(SpiceChannel *channel) { SpiceChannelPrivate *c = channel->priv; uint8_t *buffer, *p; + uint32_t *caps; int protocol, i; c->link_hdr.magic = SPICE_MAGIC; @@ -1356,14 +1357,15 @@ static void spice_channel_send_link(SpiceChannel *channel) memcpy(p, &c->link_hdr, sizeof(c->link_hdr)); p += sizeof(c->link_hdr); memcpy(p, &c->link_msg, sizeof(c->link_msg)); p += sizeof(c->link_msg); + // Need this to avoid error with -Wcast-align + caps = SPICE_UNALIGNED_CAST(uint32_t *,p); for (i = 0; i < c->common_caps->len; i++) { - *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->common_caps, uint32_t, i)); - p += sizeof(uint32_t); + *caps++ = GUINT32_TO_LE(g_array_index(c->common_caps, uint32_t, i)); } for (i = 0; i < c->caps->len; i++) { - *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, i)); - p += sizeof(uint32_t); + *caps++ = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, i)); } + p = (uint8_t *) caps; CHANNEL_DEBUG(channel, "channel type %d id %d num common caps %u num caps %u", c->channel_type, c->channel_id, @@ -1887,6 +1889,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel) SpiceChannelPrivate *c; int rc, num_caps, i; uint32_t *caps, num_channel_caps, num_common_caps; + uint8_t *caps_src; SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK; g_return_val_if_fail(channel != NULL, FALSE); @@ -1926,7 +1929,8 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel) /* see original spice/client code: */ /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */ - caps = (uint32_t *)((uint8_t *)c->peer_msg + GUINT32_FROM_LE(c->peer_msg->caps_offset)); + caps_src = (uint8_t *)c->peer_msg + GUINT32_FROM_LE(c->peer_msg->caps_offset); + caps = SPICE_UNALIGNED_CAST(uint32_t *, caps_src); g_array_set_size(c->remote_common_caps, num_common_caps); for (i = 0; i < num_common_caps; i++, caps++) { -- 2.11.0 (Apple Git-81) _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel