Merge branch 'rc-align' into 'master'

Align the reference counted allocations

Closes #1581

See merge request GNOME/glib!473

glib/garcbox.c

@@ -168,6 +168,9 @@
  * The data will be freed when its reference count drops to
  * zero.
  *
+ * The allocated data is guaranteed to be suitably aligned for any
+ * built-in type.
+ *
  * Returns: (transfer full) (not nullable): a pointer to the allocated memory
  *
  * Since: 2.58
@@ -177,7 +180,7 @@ g_atomic_rc_box_alloc (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, TRUE, FALSE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, TRUE, FALSE);
 }
 
 /**
@@ -192,6 +195,9 @@ g_atomic_rc_box_alloc (gsize block_size)
  * The data will be freed when its reference count drops to
  * zero.
  *
+ * The allocated data is guaranteed to be suitably aligned for any
+ * built-in type.
+ *
  * Returns: (transfer full) (not nullable): a pointer to the allocated memory
  *
  * Since: 2.58
@@ -201,7 +207,7 @@ g_atomic_rc_box_alloc0 (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, TRUE, TRUE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, TRUE, TRUE);
 }
 
 /**
@@ -262,7 +268,7 @@ gpointer
   g_return_val_if_fail (block_size > 0, NULL);
   g_return_val_if_fail (mem_block != NULL, NULL);
 
-  res = g_rc_box_alloc_full (block_size, TRUE, FALSE);
+  res = g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, TRUE, FALSE);
   memcpy (res, mem_block, block_size);
 
   return res;
@@ -339,13 +345,15 @@ g_atomic_rc_box_release_full (gpointer       mem_block,
 
   if (g_atomic_ref_count_dec (&real_box->ref_count))
     {
+      char *real_mem = (char *) real_box - real_box->private_offset;
+
       TRACE (GLIB_RCBOX_RELEASE (mem_block, 1));
 
       if (clear_func != NULL)
         clear_func (mem_block);
 
       TRACE (GLIB_RCBOX_FREE (mem_block));
-      g_free (real_box);
+      g_free (real_mem);
     }
 }
 

glib/grcbox.c

@@ -162,25 +162,49 @@
  * Since: 2.58.
  */
 
-#define G_RC_BOX(p)             (GRcBox *) (((char *) (p)) - G_RC_BOX_SIZE)
-
 /* We use the same alignment as GTypeInstance and GNU libc's malloc */
-#define STRUCT_ALIGNMENT        (2 * sizeof (gsize))
 #define ALIGN_STRUCT(offset)    ((offset + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT)
 
+#define G_RC_BOX(p)             (GRcBox *) (((char *) (p)) - G_RC_BOX_SIZE)
+
 gpointer
 g_rc_box_alloc_full (gsize    block_size,
+                     gsize    alignment,
                      gboolean atomic,
                      gboolean clear)
 {
-  /* sizeof GArcBox == sizeof GRcBox */
+  /* We don't do an (atomic ? G_ARC_BOX_SIZE : G_RC_BOX_SIZE) check, here
+   * because we have a static assertion that sizeof(GArcBox) == sizeof(GRcBox)
+   * inside grcboxprivate.h, and we don't want the compiler to unnecessarily
+   * warn about both branches of the conditional yielding identical results
+   */
   gsize private_size = G_ARC_BOX_SIZE;
+  gsize private_offset = 0;
   gsize real_size;
   char *allocated;
 
-  g_assert (block_size < (G_MAXSIZE - G_ARC_BOX_SIZE));
+  g_assert (alignment != 0);
+
+  /* We need to ensure that the private data is aligned */
+  if (private_size % alignment != 0)
+    {
+      private_offset = private_size % alignment;
+      private_size += (alignment - private_offset);
+    }
+
+  g_assert (block_size < (G_MAXSIZE - private_size));
   real_size = private_size + block_size;
 
+  /* The real allocated size must be a multiple of @alignment, to
+   * maintain the alignment of block_size
+   */
+  if (real_size % alignment != 0)
+    {
+      gsize offset = real_size % alignment;
+      g_assert (real_size < (G_MAXSIZE - (alignment - offset)));
+      real_size += (alignment - offset);
+    }
+
 #ifdef ENABLE_VALGRIND
   if (RUNNING_ON_VALGRIND)
     {
@@ -214,8 +238,18 @@ g_rc_box_alloc_full (gsize    block_size,
 
   if (atomic)
     {
-      GArcBox *real_box = (GArcBox *) allocated;
+      /* We leave the alignment padding at the top of the allocation,
+       * so we have an in memory layout of:
+       *
+       *  |[ offset ][ sizeof(GArcBox) ]||[ block_size ]|
+       */
+      GArcBox *real_box = (GArcBox *) (allocated + private_offset);
+      /* Store the real size */
       real_box->mem_size = block_size;
+      /* Store the alignment offset, to be used when freeing the
+       * allocated block
+       */
+      real_box->private_offset = private_offset;
 #ifndef G_DISABLE_ASSERT
       real_box->magic = G_BOX_MAGIC;
 #endif
@@ -223,8 +257,18 @@ g_rc_box_alloc_full (gsize    block_size,
     }
   else
     {
-      GRcBox *real_box = (GRcBox *) allocated;
+      /* We leave the alignment padding at the top of the allocation,
+       * so we have an in memory layout of:
+       *
+       *  |[ offset ][ sizeof(GRcBox) ]||[ block_size ]|
+       */
+      GRcBox *real_box = (GRcBox *) (allocated + private_offset);
+      /* Store the real size */
       real_box->mem_size = block_size;
+      /* Store the alignment offset, to be used when freeing the
+       * allocated block
+       */
+      real_box->private_offset = private_offset;
 #ifndef G_DISABLE_ASSERT
       real_box->magic = G_BOX_MAGIC;
 #endif
@@ -246,6 +290,9 @@ g_rc_box_alloc_full (gsize    block_size,
  * The data will be freed when its reference count drops to
  * zero.
  *
+ * The allocated data is guaranteed to be suitably aligned for any
+ * built-in type.
+ *
  * Returns: (transfer full) (not nullable): a pointer to the allocated memory
  *
  * Since: 2.58
@@ -255,7 +302,7 @@ g_rc_box_alloc (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, FALSE, FALSE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, FALSE, FALSE);
 }
 
 /**
@@ -270,6 +317,9 @@ g_rc_box_alloc (gsize block_size)
  * The data will be freed when its reference count drops to
  * zero.
  *
+ * The allocated data is guaranteed to be suitably aligned for any
+ * built-in type.
+ *
  * Returns: (transfer full) (not nullable): a pointer to the allocated memory
  *
  * Since: 2.58
@@ -279,7 +329,7 @@ g_rc_box_alloc0 (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, FALSE, TRUE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, FALSE, TRUE);
 }
 
 /**
@@ -339,7 +389,7 @@ gpointer
   g_return_val_if_fail (block_size > 0, NULL);
   g_return_val_if_fail (mem_block != NULL, NULL);
 
-  res = g_rc_box_alloc_full (block_size, FALSE, FALSE);
+  res = g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, FALSE, FALSE);
   memcpy (res, mem_block, block_size);
 
   return res;
@@ -416,13 +466,15 @@ g_rc_box_release_full (gpointer       mem_block,
 
   if (g_ref_count_dec (&real_box->ref_count))
     {
+      char *real_mem = (char *) real_box - real_box->private_offset;
+
       TRACE (GLIB_RCBOX_RELEASE (mem_block, 0));
 
       if (clear_func != NULL)
         clear_func (mem_block);
 
       TRACE (GLIB_RCBOX_FREE (mem_block));
-      g_free (real_box);
+      g_free (real_mem);
     }
 }
 

glib/grcboxprivate.h

@@ -27,6 +27,7 @@ typedef struct {
   grefcount ref_count;
 
   gsize mem_size;
+  gsize private_offset;
 
 #ifndef G_DISABLE_ASSERT
   /* A "magic" number, used to perform additional integrity
@@ -40,6 +41,7 @@ typedef struct {
   gatomicrefcount ref_count;
 
   gsize mem_size;
+  gsize private_offset;
 
 #ifndef G_DISABLE_ASSERT
   guint32 magic;
@@ -51,10 +53,18 @@ typedef struct {
 /* Keep the two refcounted boxes identical in size */
 G_STATIC_ASSERT (sizeof (GRcBox) == sizeof (GArcBox));
 
+/* This is the default alignment we use when allocating the
+ * refcounted memory blocks; it's similar to the alignment
+ * guaranteed by the malloc() in GNU's libc and by the GSlice
+ * allocator
+ */
+#define STRUCT_ALIGNMENT (2 * sizeof (gsize))
+
 #define G_RC_BOX_SIZE sizeof (GRcBox)
 #define G_ARC_BOX_SIZE sizeof (GArcBox)
 
 gpointer        g_rc_box_alloc_full     (gsize    block_size,
+                                         gsize    alignment,
                                          gboolean atomic,
                                          gboolean clear);
 

glib/tests/rcbox.c

@@ -220,6 +220,64 @@ test_atomic_rcbox_dup (void)
   g_assert_null (global_point_b);
 }
 
+/* The expected alignment of the refcounted data, absent any other
+ * alignment requirement, is `2 * sizeof(void*)`; GLib only really
+ * supports void* sized 8 or 4 (see the comment in gatomic.h)
+ */
+#if GLIB_SIZEOF_VOID_P == 8
+static const gsize rcbox_alignment = 16;
+#else
+static const gsize rcbox_alignment = 8;
+#endif
+
+/* verify that the refcounted allocation is properly aligned */
+static void
+test_rcbox_alignment (void)
+{
+  const gsize block_sizes[] = {
+    1,
+    2,
+    4,
+    sizeof (gint32) * 3,
+  };
+
+  int i;
+
+  for (i = 0; i < G_N_ELEMENTS (block_sizes); i++)
+    {
+      gpointer p = g_rc_box_alloc0 (block_sizes[i]);
+
+      g_assert_nonnull (p);
+      g_assert_true (((guintptr) p & (rcbox_alignment - 1)) == 0);
+
+      g_rc_box_release (p);
+    }
+}
+
+/* verify that the atomically refcounted allocation is properly aligned */
+static void
+test_atomic_rcbox_alignment (void)
+{
+  const gsize block_sizes[] = {
+    1,
+    2,
+    4,
+    sizeof (gint32) * 3,
+  };
+
+  int i;
+
+  for (i = 0; i < G_N_ELEMENTS (block_sizes); i++)
+    {
+      gpointer p = g_atomic_rc_box_alloc0 (block_sizes[i]);
+
+      g_assert_nonnull (p);
+      g_assert_true (((guintptr) p & (rcbox_alignment - 1)) == 0);
+
+      g_atomic_rc_box_release (p);
+    }
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -229,10 +287,12 @@ main (int   argc,
   g_test_add_func ("/rcbox/new", test_rcbox_new);
   g_test_add_func ("/rcbox/release-full", test_rcbox_release_full);
   g_test_add_func ("/rcbox/dup", test_rcbox_dup);
+  g_test_add_func ("/rcbox/alignment", test_rcbox_alignment);
 
   g_test_add_func ("/atomic-rcbox/new", test_atomic_rcbox_new);
   g_test_add_func ("/atomic-rcbox/release-full", test_atomic_rcbox_release_full);
   g_test_add_func ("/atomic-rcbox/dup", test_atomic_rcbox_dup);
+  g_test_add_func ("/atomic-rcbox/alignment", test_atomic_rcbox_alignment);
 
   return g_test_run ();
 }