[Spice-devel] [PATCH spice-gtk v8 3/4] Avoid clang warnings on casts with stricter alignment requirements
Christophe de Dinechin
christophe at dinechin.org
Thu Jun 8 15:38:53 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 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 if you configure with --enable-alignment-warnings.
- SPICE_UNALIGNED_CAST(type, value) casts value to type, and indicates that
we believe the resulting pointer is not always aligned. A debug message
is issued if you configure with --enable-alignment-warnings.
Any code using SPICE_UNALIGNED_CAST may need to be revisited in order
to improve performance, e.g. by using memcpy. A few easy fixes have been
made in his patch.
Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
---
configure.ac | 8 ++++++++
spice-common | 2 +-
src/channel-cursor.c | 8 +++++---
src/channel-display-mjpeg.c | 2 +-
src/channel-main.c | 2 +-
src/continuation.h | 6 ++++--
src/decode-glz-tmpl.c | 2 +-
src/spice-channel.c | 26 ++++++++++++++++----------
8 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/configure.ac b/configure.ac
index 62acafc..caa289a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -565,6 +565,14 @@ else
SPICE_WARNING([No D-Bus support, desktop integration and USB redirection may not work properly])
fi
+AC_ARG_ENABLE([alignment-checks],
+ AS_HELP_STRING([--enable-alignment-checks],
+ [Enable runtime checks for cast alignment @<:@default=no@:>@]),
+ [],
+ enable_alignment_checks="no")
+AS_IF([test "x$enable_alignment_checks" = "xyes"],
+ [AC_DEFINE([SPICE_DEBUG_ALIGNMENT], 1, [Enable runtime checks for cast alignment])])
+
SPICE_CHECK_LZ4
dnl ===========================================================================
diff --git a/spice-common b/spice-common
index af682b1..8cbfedc 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit af682b1b06dea55007d9aa7c37cd443e4349e43f
+Subproject commit 8cbfedc08fdf6be68cabd5afd781f61ae01b577b
diff --git a/src/channel-cursor.c b/src/channel-cursor.c
index 558c920..14053a9 100644
--- a/src/channel-cursor.c
+++ b/src/channel-cursor.c
@@ -381,10 +381,11 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor)
SpiceCursorHeader *hdr = &scursor->header;
display_cursor *cursor;
size_t size;
- gint i, pix_mask, pix;
+ guint32 i, pix_mask, pix;
const guint8* data;
guint8 *rgba;
guint8 val;
+ guint32 palette[16];
CHANNEL_DEBUG(channel, "%s: flags %x, size %u", __FUNCTION__,
scursor->flags, scursor->data_size);
@@ -433,7 +434,7 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor)
size /= 2u;
for (i = 0; i < hdr->width * hdr->height; i++) {
pix_mask = get_pix_mask(data, size, i);
- pix = *((guint16*)data + i);
+ pix = *(SPICE_UNALIGNED_CAST(guint16 *, data) + i);
if (pix_mask && pix == 0x7fff) {
cursor->data[i] = get_pix_hack(i, hdr->width);
} else {
@@ -444,10 +445,11 @@ static display_cursor *set_cursor(SpiceChannel *channel, SpiceCursor *scursor)
break;
case SPICE_CURSOR_TYPE_COLOR4:
size = ((unsigned int)(SPICE_ALIGN(hdr->width, 2) / 2)) * hdr->height;
+ memcpy(palette, data + size, sizeof(palette));
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 = palette[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/channel-main.c b/src/channel-main.c
index 29dd42d..1202d13 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1872,7 +1872,7 @@ static void main_agent_handle_xfer_status(SpiceMainChannel *channel,
_("The spice agent reported an error during the file transfer"));
break;
case VD_AGENT_FILE_XFER_STATUS_NOT_ENOUGH_SPACE: {
- uint64_t *free_space = (uint64_t *)(msg->data);
+ uint64_t *free_space = SPICE_ALIGNED_CAST(uint64_t *, msg->data);
gchar *free_space_str = g_format_size(*free_space);
gchar *file_size_str = g_format_size(spice_file_transfer_task_get_total_bytes(xfer_task));
error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
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 77ac9cd..b196a26 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;
SpiceLinkMess link_msg;
@@ -1357,14 +1358,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, &link_msg, sizeof(link_msg)); p += sizeof(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,
@@ -1888,6 +1890,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);
@@ -1927,18 +1930,21 @@ 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);
g_array_set_size(c->remote_common_caps, num_common_caps);
+ caps = &g_array_index(c->remote_common_caps, uint32_t, 0);
+ memcpy(caps, caps_src, num_common_caps * sizeof(uint32_t));
for (i = 0; i < num_common_caps; i++, caps++) {
- g_array_index(c->remote_common_caps, uint32_t, i) = GUINT32_FROM_LE(*caps);
- CHANNEL_DEBUG(channel, "got common caps %d:0x%X", i, GUINT32_FROM_LE(*caps));
+ *caps = GUINT32_FROM_LE(*caps);
+ CHANNEL_DEBUG(channel, "got common caps %d:0x%X", i, *caps);
}
g_array_set_size(c->remote_caps, num_channel_caps);
+ caps_src += num_common_caps * sizeof(uint32_t);
+ memcpy(caps, caps_src, num_channel_caps * sizeof(uint32_t));
for (i = 0; i < num_channel_caps; i++, caps++) {
- g_array_index(c->remote_caps, uint32_t, i) = GUINT32_FROM_LE(*caps);
- CHANNEL_DEBUG(channel, "got channel caps %d:0x%X", i, GUINT32_FROM_LE(*caps));
+ *caps = GUINT32_FROM_LE(*caps);
+ CHANNEL_DEBUG(channel, "got channel caps %d:0x%X", i, *caps);
}
if (!spice_channel_test_common_capability(channel,
--
2.11.0 (Apple Git-81)
More information about the Spice-devel
mailing list