[Spice-commits] common/canvas_base.c common/mem.c common/mem.h common/sw_canvas.c configure.ac

Christophe Fergau teuf at kemper.freedesktop.org
Tue Jun 20 13:21:04 UTC 2017


 common/canvas_base.c |   14 +++++++++-----
 common/mem.c         |   22 ++++++++++++++++++++++
 common/mem.h         |   41 +++++++++++++++++++++++++++++++++++++++++
 common/sw_canvas.c   |   11 ++++++-----
 configure.ac         |    8 ++++++++
 5 files changed, 86 insertions(+), 10 deletions(-)

New commits:
commit 858a0bfae9925ea6e363573de4113090c7821133
Author: Christophe de Dinechin <dinechin at redhat.com>
Date:   Tue May 30 14:51:49 2017 +0200

    Avoid clang warnings on casts with stricter alignment requirements
    
    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. This check is disabled unless
      --enable-alignment-checks is passed at configure time
    
    - 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
    by configuring with CFLAGS=-DSPICE_DEBUG_ALIGNMENT.
    
    Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>

diff --git a/common/canvas_base.c b/common/canvas_base.c
index 8f653e0..5815e9c 100644
--- a/common/canvas_base.c
+++ b/common/canvas_base.c
@@ -389,7 +389,7 @@ static pixman_image_t *canvas_get_quic(CanvasBase *canvas, SpiceImage *image,
     quic_data->current_chunk = 0;
 
     if (quic_decode_begin(quic_data->quic,
-                          (uint32_t *)image->u.quic.data->chunk[0].data,
+                          SPICE_UNALIGNED_CAST(uint32_t *,image->u.quic.data->chunk[0].data),
                           image->u.quic.data->chunk[0].len >> 2,
                           &type, &width, &height) == QUIC_ERROR) {
         spice_warning("quic decode begin failed");
@@ -523,8 +523,8 @@ static void canvas_fix_alignment(uint8_t *bits,
         uint8_t *dest = bits;
         for (row = height - 1; row > 0; --row) {
             uint32_t *dest_aligned, *dest_misaligned;
-            dest_aligned = (uint32_t *)(dest + stride_pixman*row);
-            dest_misaligned = (uint32_t*)(dest + stride_encoded*row);
+            dest_aligned = SPICE_ALIGNED_CAST(uint32_t *,dest + stride_pixman*row);
+            dest_misaligned = SPICE_UNALIGNED_CAST(uint32_t*,dest + stride_encoded*row);
             memmove(dest_aligned, dest_misaligned, stride_encoded);
         }
     }
@@ -1889,7 +1889,8 @@ static int quic_usr_more_space(QuicUsrContext *usr, uint32_t **io_ptr, int rows_
     }
     quic_data->current_chunk++;
 
-    *io_ptr = (uint32_t *)quic_data->chunks->chunk[quic_data->current_chunk].data;
+    *io_ptr = SPICE_ALIGNED_CAST(uint32_t *,
+                                 quic_data->chunks->chunk[quic_data->current_chunk].data);
     return quic_data->chunks->chunk[quic_data->current_chunk].len >> 2;
 }
 
@@ -1945,6 +1946,7 @@ static void canvas_mask_pixman(CanvasBase *canvas,
     int needs_invert;
     pixman_region32_t mask_region;
     uint32_t *mask_data;
+    uint8_t *mask_data_src;
     int mask_x, mask_y;
     int mask_width, mask_height, mask_stride;
     pixman_box32_t extents;
@@ -2006,7 +2008,9 @@ static void canvas_mask_pixman(CanvasBase *canvas,
     /* round down X to even 32 pixels (i.e. uint32_t) */
     extents.x1 = extents.x1 & ~(0x1f);
 
-    mask_data = (uint32_t *)((uint8_t *)mask_data + mask_stride * extents.y1 + extents.x1 / 32);
+    mask_data_src = (uint8_t *)mask_data + mask_stride * extents.y1 + extents.x1 / 32;
+    mask_data = SPICE_UNALIGNED_CAST(uint32_t *, mask_data_src);
+
     mask_x -= extents.x1;
     mask_y -= extents.y1;
     mask_width = extents.x2 - extents.x1;
diff --git a/common/mem.c b/common/mem.c
index e430b5d..8b96548 100644
--- a/common/mem.c
+++ b/common/mem.c
@@ -294,3 +294,25 @@ size_t spice_buffer_remove(SpiceBuffer *buffer, size_t len)
     buffer->offset -= len;
     return len;
 }
+
+#ifdef SPICE_DEBUG_ALIGNMENT
+void spice_alignment_warning(const char *loc, void *p, unsigned sz)
+{
+    static const char *last_loc = NULL;
+    if (loc != last_loc) {
+        last_loc = loc;
+        spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_WARNING, loc, __FUNCTION__,
+                  "Misaligned access at %p, alignment %u", p, sz);
+    }
+}
+
+void spice_alignment_debug(const char *loc, void *p, unsigned sz)
+{
+    static const char *last_loc = NULL;
+    if (loc != last_loc) {
+        last_loc = loc;
+        spice_log(SPICE_LOG_DOMAIN, G_LOG_LEVEL_DEBUG, loc, __FUNCTION__,
+                  "Expected misaligned access at %p, alignment %u", p, sz);
+    }
+}
+#endif // SPICE_DEBUG_ALIGNMENT
diff --git a/common/mem.h b/common/mem.h
index cf24d0a..1c7cfa3 100644
--- a/common/mem.h
+++ b/common/mem.h
@@ -19,6 +19,7 @@
 #ifndef _H_MEM
 #define _H_MEM
 
+#include "log.h"
 #include <stdlib.h>
 #include <spice/macros.h>
 
@@ -139,6 +140,46 @@ size_t spice_strnlen(const char *str, size_t max_len);
 
 #endif
 
+/* Cast to a type with stricter alignment constraints (to build with clang) */
+
+/* Misaligned cast to a type with stricter alignment */
+#ifndef SPICE_DEBUG_ALIGNMENT
+#define SPICE_UNALIGNED_CAST(type, value) ((type)(void *)(value))
+#define SPICE_ALIGNED_CAST(type, value)   ((type)(void *)(value))
+
+#else // SPICE_DEBUG_ALIGNMENT
+#define SPICE_ALIGNED_CAST(type, value)                                 \
+    ((type)spice_alignment_check(G_STRLOC,                              \
+                                 (void *)(value),                       \
+                                 __alignof(*((type)0))))
+
+#define SPICE_UNALIGNED_CAST(type, value)                               \
+    ((type)spice_alignment_weak_check(G_STRLOC,                         \
+                                      (void *)(value),                  \
+                                      __alignof(*((type)0))))
+
+extern void spice_alignment_warning(const char *loc, void *p, unsigned sz);
+extern void spice_alignment_debug(const char *loc, void *p, unsigned sz);
+
+static inline void *spice_alignment_check(const char *loc,
+                                          void *ptr, unsigned sz)
+{
+    if (G_UNLIKELY(((uintptr_t) ptr & (sz-1U)) != 0))
+        spice_alignment_warning(loc, ptr, sz);
+    return ptr;
+
+}
+
+static inline void *spice_alignment_weak_check(const char *loc,
+                                               void *ptr, unsigned sz)
+{
+    if (G_UNLIKELY(((uintptr_t) ptr & (sz-1U)) != 0))
+        spice_alignment_debug(loc, ptr, sz);
+    return ptr;
+
+}
+#endif // SPICE_DEBUG_ALIGNMENT
+
 #define spice_new(struct_type, n_structs) _SPICE_NEW(struct_type, n_structs, malloc)
 #define spice_new0(struct_type, n_structs) _SPICE_NEW(struct_type, n_structs, malloc0)
 #define spice_renew(struct_type, mem, n_structs) _SPICE_RENEW(struct_type, mem, n_structs, realloc)
diff --git a/common/sw_canvas.c b/common/sw_canvas.c
index 9dc11ca..c347a56 100644
--- a/common/sw_canvas.c
+++ b/common/sw_canvas.c
@@ -355,8 +355,8 @@ static void clear_dest_alpha(pixman_image_t *dest,
     }
 
     stride = pixman_image_get_stride(dest);
-    data = (uint32_t *) (
-        (uint8_t *)pixman_image_get_data(dest) + y * stride + 4 * x);
+    data = SPICE_ALIGNED_CAST(uint32_t *,
+                              (uint8_t *)pixman_image_get_data(dest) + y * stride + 4 * x);
 
     if ((*data & 0xff000000U) == 0xff000000U) {
         spice_pixman_fill_rect_rop(dest,
@@ -1010,7 +1010,7 @@ static void canvas_put_image(SpiceCanvas *spice_canvas,
     src = pixman_image_create_bits(PIXMAN_x8r8g8b8,
                                    src_width,
                                    src_height,
-                                   (uint32_t*)src_data,
+                                   SPICE_ALIGNED_CAST(uint32_t*,src_data),
                                    src_stride);
 
 
@@ -1264,9 +1264,10 @@ SpiceCanvas *canvas_create_for_data(int width, int height, uint32_t format,
 {
     pixman_image_t *image;
 
-
     image = pixman_image_create_bits(spice_surface_format_to_pixman(format),
-                                     width, height, (uint32_t *) data, stride);
+                                     width, height,
+                                     SPICE_ALIGNED_CAST(uint32_t *,data),
+                                     stride);
 
     return canvas_create_common(image, format,
                                 bits_cache,
diff --git a/configure.ac b/configure.ac
index 6d9f378..efd7317 100644
--- a/configure.ac
+++ b/configure.ac
@@ -28,6 +28,14 @@ AM_PROG_CC_C_O
 
 SPICE_CHECK_SYSDEPS
 
+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])])
+
 # Checks for libraries
 PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
 


More information about the Spice-commits mailing list