From 0749643daaa818e78d68da27db5378f15eb31e0b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 8 Dec 2017 14:34:34 +0000 Subject: [PATCH 1/3] gobject: Assert that GObjects are at least as aligned as basic types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See the reasoning in the patch for why we believe GObjects *are* (already) as aligned as the basic types. We want to make this guarantee so that it’s guaranteed to be safe for people to ignore -Wcast-align warnings for GObjects which contain basic types. This typically happens with gdouble on 32-bit ARM platforms. The checks are slightly complicated by the need to support GObjects with custom constructors. We should expect that a custom construction function will chain up to g_object_constructor (which calls g_type_create_instance() as normal), but it’s possible that someone has done something crazy and uses a custom allocator which doesn’t return with the same alignment as GSlice. Hand them a warning in that case. If that is true, the code which uses their custom-constructed GObject can presumably already deal with the alignment it gets given. Signed-off-by: Philip Withnall Helps: #1231 --- gobject/gobject.c | 38 ++++++++++++++++++++++++++++++++++++++ gobject/gobject.h | 4 ++++ gobject/gtype.h | 4 ++++ 3 files changed, 46 insertions(+) diff --git a/gobject/gobject.c b/gobject/gobject.c index ae89b7462..ac94454cd 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -24,6 +24,8 @@ #include #include +#include "../glib/glib-private.h" + #include "gobject.h" #include "gtype-private.h" #include "gvaluecollector.h" @@ -1794,6 +1796,10 @@ g_object_get_type (void) * Similarly, #gfloat is promoted to #gdouble, so you must ensure that the value * you provide is a #gdouble, even for a property of type #gfloat. * + * Since GLib 2.72, all #GObjects are guaranteed to be aligned to at least the + * alignment of the largest basic GLib type (typically this is #guint64 or + * #gdouble). + * * Returns: (transfer full) (type GObject.Object): a new instance of * @object_type */ @@ -1816,6 +1822,26 @@ g_object_new (GType object_type, return object; } +/* Check alignment. (See https://gitlab.gnome.org/GNOME/glib/-/issues/1231.) + * This should never fail, since g_type_create_instance() uses g_slice_alloc0(). + * The GSlice allocator always aligns to the next power of 2 greater than the + * allocation size. The allocation size for a GObject is + * sizeof(GTypeInstance) + sizeof(guint) + sizeof(GData*) + * which is 12B on 32-bit platforms, and larger on 64-bit systems. In both + * cases, that’s larger than the 8B needed for a guint64 or gdouble. + * + * If GSlice falls back to malloc(), it’s documented to return something + * suitably aligned for any basic type. */ +static inline gboolean +g_object_is_aligned (GObject *object) +{ + return ((((guintptr) (void *) object) % + MAX (G_ALIGNOF (gdouble), + MAX (G_ALIGNOF (guint64), + MAX (G_ALIGNOF (gint), + G_ALIGNOF (glong))))) == 0); +} + static gpointer g_object_new_with_custom_constructor (GObjectClass *class, GObjectConstructParam *params, @@ -1903,6 +1929,16 @@ g_object_new_with_custom_constructor (GObjectClass *class, return NULL; } + if (!g_object_is_aligned (object)) + { + g_critical ("Custom constructor for class %s returned a non-aligned " + "GObject (which is invalid since GLib 2.72). Assuming any " + "code using this object doesn’t require it to be aligned. " + "Please fix your constructor to align to the largest GLib " + "basic type (typically gdouble or guint64).", + G_OBJECT_CLASS_NAME (class)); + } + /* g_object_init() will have marked the object as being in-construction. * Check if the returned object still is so marked, or if this is an * already-existing singleton (in which case we should not do 'constructed'). @@ -1969,6 +2005,8 @@ g_object_new_internal (GObjectClass *class, object = (GObject *) g_type_create_instance (class->g_type_class.g_type); + g_assert (g_object_is_aligned (object)); + if (CLASS_HAS_PROPS (class)) { GSList *node; diff --git a/gobject/gobject.h b/gobject/gobject.h index d74bebc7c..8bcb0cdbc 100644 --- a/gobject/gobject.h +++ b/gobject/gobject.h @@ -253,6 +253,10 @@ typedef void (*GWeakNotify) (gpointer data, * * All the fields in the `GObject` structure are private to the implementation * and should never be accessed directly. + * + * Since GLib 2.72, all #GObjects are guaranteed to be aligned to at least the + * alignment of the largest basic GLib type (typically this is #guint64 or + * #gdouble). */ struct _GObject { diff --git a/gobject/gtype.h b/gobject/gtype.h index 874a7c00c..c6bccc010 100644 --- a/gobject/gtype.h +++ b/gobject/gtype.h @@ -1981,6 +1981,10 @@ guint g_type_get_type_registration_serial (void); * } * ]| * + * Since GLib 2.72, the returned `MyObjectPrivate` pointer is guaranteed to be + * aligned to at least the alignment of the largest basic GLib type (typically + * this is #guint64 or #gdouble). + * * Note that this macro can only be used together with the `G_DEFINE_TYPE_*` * macros, since it depends on variable names from those macros. * From ed553e8e309d71a42d24d2015c0afbb92f2df7b6 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 8 Dec 2017 14:53:58 +0000 Subject: [PATCH 2/3] gtype: Eliminate -Wcast-align warnings with G_TYPE_CHECK_INSTANCE_CAST MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regardless of the actual alignment of the GTypeInstance in question, these do a runtime check on the type, so if the type was originally aligned correctly when allocated, it should be aligned correctly if the type check succeeds. -Wcast-align is meant to warn about casts between types, which this isn’t (if the check succeeds). Signed-off-by: Philip Withnall Fixes: #1231 --- gobject/gtype.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gobject/gtype.h b/gobject/gtype.h index c6bccc010..cb79caa82 100644 --- a/gobject/gtype.h +++ b/gobject/gtype.h @@ -2418,9 +2418,9 @@ const gchar * g_type_name_from_class (GTypeClass *g_class); /* --- implementation bits --- */ #ifndef G_DISABLE_CAST_CHECKS # define _G_TYPE_CIC(ip, gt, ct) \ - ((ct*) g_type_check_instance_cast ((GTypeInstance*) ip, gt)) + ((ct*) (void *) g_type_check_instance_cast ((GTypeInstance*) ip, gt)) # define _G_TYPE_CCC(cp, gt, ct) \ - ((ct*) g_type_check_class_cast ((GTypeClass*) cp, gt)) + ((ct*) (void *) g_type_check_class_cast ((GTypeClass*) cp, gt)) #else /* G_DISABLE_CAST_CHECKS */ # define _G_TYPE_CIC(ip, gt, ct) ((ct*) ip) # define _G_TYPE_CCC(cp, gt, ct) ((ct*) cp) From 7a8756d247a5c54be121668919ae9cac2f8ae4fb Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 8 Dec 2017 14:57:28 +0000 Subject: [PATCH 3/3] gobject: Add advice on larger alignment requirements for GObject members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now guarantee that GObjects will always be allocated at least as aligned as the basic types. If you want to put an element in your GObject which has higher alignment requirements, we can’t guarantee it will be aligned*. If you need it to be aligned, you’ll need to put it on the heap (aligned appropriately), or add appropriate padding in your GObject struct. *Actually, GSlice will guarantee that the whole GObject is aligned to at least the power of 2 greater than or equal to the size of the GObject, which means any element in the GObject struct should always be appropriate aligned if the compiler pads it appropriately. If malloc() is used, however, it doesn’t make that guarantee, so we can’t make that guarantee overall. Signed-off-by: Philip Withnall Helps: #1231 --- gobject/gobject.c | 4 +++- gobject/gobject.h | 6 +++++- gobject/gtype.h | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/gobject/gobject.c b/gobject/gobject.c index ac94454cd..7499d2ec4 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -1798,7 +1798,9 @@ g_object_get_type (void) * * Since GLib 2.72, all #GObjects are guaranteed to be aligned to at least the * alignment of the largest basic GLib type (typically this is #guint64 or - * #gdouble). + * #gdouble). If you need larger alignment for an element in a #GObject, you + * should allocate it on the heap (aligned), or arrange for your #GObject to be + * appropriately padded. * * Returns: (transfer full) (type GObject.Object): a new instance of * @object_type diff --git a/gobject/gobject.h b/gobject/gobject.h index 8bcb0cdbc..3dc4f7f48 100644 --- a/gobject/gobject.h +++ b/gobject/gobject.h @@ -256,7 +256,11 @@ typedef void (*GWeakNotify) (gpointer data, * * Since GLib 2.72, all #GObjects are guaranteed to be aligned to at least the * alignment of the largest basic GLib type (typically this is #guint64 or - * #gdouble). + * #gdouble). If you need larger alignment for an element in a #GObject, you + * should allocate it on the heap (aligned), or arrange for your #GObject to be + * appropriately padded. This guarantee applies to the #GObject (or derived) + * struct, the #GObjectClass (or derived) struct, and any private data allocated + * by G_ADD_PRIVATE(). */ struct _GObject { diff --git a/gobject/gtype.h b/gobject/gtype.h index cb79caa82..5c9077745 100644 --- a/gobject/gtype.h +++ b/gobject/gtype.h @@ -1983,7 +1983,9 @@ guint g_type_get_type_registration_serial (void); * * Since GLib 2.72, the returned `MyObjectPrivate` pointer is guaranteed to be * aligned to at least the alignment of the largest basic GLib type (typically - * this is #guint64 or #gdouble). + * this is #guint64 or #gdouble). If you need larger alignment for an element in + * the struct, you should allocate it on the heap (aligned), or arrange for your + * `MyObjectPrivate` struct to be appropriately padded. * * Note that this macro can only be used together with the `G_DEFINE_TYPE_*` * macros, since it depends on variable names from those macros.