[glib: 1/2] gobject: Fix strict aliasing warnings with g_set_object()



commit 51acb01f73da2ba7eb8838745df05bdd044a2636
Author: Philip Withnall <withnall endlessm com>
Date:   Tue Feb 18 12:10:07 2020 +0000

    gobject: Fix strict aliasing warnings with g_set_object()
    
    When calling `g_set_object()` for a type derived from `GObject`, GCC 9.2
    was giving the following strict aliasing warning:
    ```
    ../../source/malcontent/libmalcontent-ui/user-controls.c:1001:21: warning: dereferencing type-punned 
pointer will break strict-aliasing rules [-Wstrict-aliasing]
     1001 |   if (g_set_object (&self->user, user))
    /opt/gnome/install/include/glib-2.0/gobject/gobject.h:744:33: note: in definition of macro ‘g_set_object’
      744 |   (g_set_object) ((GObject **) (object_ptr), (GObject *) (new_object)) \
          |                                 ^~~~~~~~~~
    ```
    
    This was due to the `(GObject **)` cast.
    
    Pass the pointer through a union to squash this warning. We already do
    some size and type checks of the dereferenced type, which should catch
    casual errors. The `g_object_ref()` and `g_object_unref()` calls which
    subsequently happen inside the `g_set_object()` function also do some
    dynamic type checks.
    
    Add a test.
    
    Signed-off-by: Philip Withnall <withnall endlessm com>

 gobject/gobject.h         | 19 +++++++++++++++++++
 gobject/tests/reference.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)
---
diff --git a/gobject/gobject.h b/gobject/gobject.h
index 91b9f6328..2e07fa724 100644
--- a/gobject/gobject.h
+++ b/gobject/gobject.h
@@ -738,12 +738,31 @@ static inline gboolean
   return TRUE;
 }
 
+/* We need GCC for __extension__, which we need to sort out strict aliasing of @object_ptr */
+#if defined(__GNUC__)
+
+#define g_set_object(object_ptr, new_object) \
+  (G_GNUC_EXTENSION ({ \
+    G_STATIC_ASSERT (sizeof *(object_ptr) == sizeof (new_object)); \
+    /* Only one access, please; work around type aliasing */ \
+    union { char *in; GObject **out; } _object_ptr; \
+    _object_ptr.in = (char *) (object_ptr); \
+    /* Check types match */ \
+    (void) (0 ? *(object_ptr) = (new_object), FALSE : FALSE); \
+    (g_set_object) (_object_ptr.out, (GObject *) new_object); \
+  })) \
+  GLIB_AVAILABLE_MACRO_IN_2_44
+
+#else  /* if !defined(__GNUC__) */
+
 #define g_set_object(object_ptr, new_object) \
  (/* Check types match. */ \
   0 ? *(object_ptr) = (new_object), FALSE : \
   (g_set_object) ((GObject **) (object_ptr), (GObject *) (new_object)) \
  )
 
+#endif  /* !defined(__GNUC__) */
+
 /**
  * g_assert_finalize_object: (skip)
  * @object: (transfer full) (type GObject.Object): an object
diff --git a/gobject/tests/reference.c b/gobject/tests/reference.c
index 9508ee741..c6f4d5127 100644
--- a/gobject/tests/reference.c
+++ b/gobject/tests/reference.c
@@ -217,6 +217,35 @@ test_set_function (void)
   g_assert_null (tmp_weak);
 }
 
+static void
+test_set_derived_type (void)
+{
+  GBinding *obj = NULL;
+  GObject *o = NULL;
+  GBinding *b = NULL;
+
+  g_test_summary ("Check that g_set_object() doesn’t give strict aliasing "
+                  "warnings when used on types derived from GObject");
+
+  g_assert_false (g_set_object (&o, NULL));
+  g_assert_null (o);
+
+  g_assert_false (g_set_object (&b, NULL));
+  g_assert_null (b);
+
+  obj = g_object_new (my_object_get_type (), NULL);
+
+  g_assert_true (g_set_object (&o, G_OBJECT (obj)));
+  g_assert_true (o == G_OBJECT (obj));
+
+  g_assert_true (g_set_object (&b, obj));
+  g_assert_true (b == obj);
+
+  g_object_unref (obj);
+  g_clear_object (&b);
+  g_clear_object (&o);
+}
+
 static void
 toggle_cb (gpointer data, GObject *obj, gboolean is_last)
 {
@@ -774,6 +803,7 @@ main (int argc, char **argv)
   g_test_add_func ("/object/clear-function", test_clear_function);
   g_test_add_func ("/object/set", test_set);
   g_test_add_func ("/object/set-function", test_set_function);
+  g_test_add_func ("/object/set/derived-type", test_set_derived_type);
   g_test_add_func ("/object/value", test_object_value);
   g_test_add_func ("/object/initially-unowned", test_initially_unowned);
   g_test_add_func ("/object/weak-pointer", test_weak_pointer);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]