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

Christophe de Dinechin christophe at dinechin.org
Wed May 17 08:37:42 UTC 2017


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 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 at redhat.com>
---
 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)



More information about the Spice-devel mailing list