[glib/th/gobject-new-parameter-list: 987/987] gobject: limit required stack by heap allocating parameter list in g_object_new_valist()

[Thomas Haller <thaller src gnome org>]

commit 72ea7520105cfe53d08703e5a517d31b65a04fac
Author: Thomas Haller <thaller redhat com>
Date:   Thu Dec 12 12:28:01 2019 +0100

    gobject: limit required stack by heap allocating parameter list in g_object_new_valist()
    
    The previous code consumed a larger additional amount of stack space.
    That is because it would allocate the temporary buffer for GValues on
    the stack with "g_newa (GValue, 1)" and thus the required stack
    space grew with the number of arguments. Granted, this is already
    a variadic C function, so the caller already placed that many elements
    on the stack. For example, on the stack there are the property names
    and the pointers to the arguments, which should amount to roughly
    O(n_args * 16) (on 64 bit, with pointers being 8 bytes large).
    
    That means in the previous version the stack space would grow linear with
    the already used stack space. However, a GValue is an additional 24 bytes
    (on 64 bitxs), which probably more than doubles the required stack space.
    Avoid that, by allocating the temporary list on the heap after a certain
    threshold. This probably more than doubles the number of possible
    arguments before the stack overflows.
    
    Also, previously the heap allocated params array only grew one element
    per iteration. Of course, it is likely that libc anyway reallocates
    the buffers by growing the space exponentially. So realloc(ptr, 1)
    probably does not O() scale worse than doubling the buffer sizes ourselves.
    However, it seems nicer to keep track of the allocated sizes ourself, and
    only call realloc() when we determine that we are out of space.

 gobject/gobject.c | 50 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 15 deletions(-)
---
diff --git a/gobject/gobject.c b/gobject/gobject.c
index 3e860da26..fa47e09d9 100644
--- a/gobject/gobject.c
+++ b/gobject/gobject.c
@@ -2218,13 +2218,15 @@ g_object_new_valist (GType        object_type,
 
   if (first_property_name)
     {
-      GObjectConstructParam stack_params[16];
-      GObjectConstructParam *params;
+      GObjectConstructParam params_stack[1];
+      GValue values_stack[G_N_ELEMENTS (params_stack)];
       const gchar *name;
-      gint n_params = 0;
+      GObjectConstructParam *params = params_stack;
+      GValue *values = values_stack;
+      guint n_params = 0;
+      guint n_params_alloc = G_N_ELEMENTS (params_stack);
 
       name = first_property_name;
-      params = stack_params;
 
       do
         {
@@ -2236,24 +2238,39 @@ g_object_new_valist (GType        object_type,
           if (!g_object_new_is_valid_property (object_type, pspec, name, params, n_params))
             break;
 
-          if (n_params == 16)
+          if (G_UNLIKELY (n_params == n_params_alloc))
             {
-              params = g_new (GObjectConstructParam, n_params + 1);
-              memcpy (params, stack_params, sizeof stack_params);
+              guint i;
+
+              if (n_params_alloc == G_N_ELEMENTS (params_stack))
+                {
+                  n_params_alloc = G_N_ELEMENTS (params_stack) * 2u;
+                  params = g_new (GObjectConstructParam, n_params_alloc);
+                  values = g_new (GValue, n_params_alloc);
+                  memcpy (params, params_stack, sizeof (GObjectConstructParam) * n_params);
+                  memcpy (values, values_stack, sizeof (GValue) * n_params);
+                }
+              else
+                {
+                  n_params_alloc *= 2u;
+                  params = g_realloc (params, sizeof (GObjectConstructParam) * n_params_alloc);
+                  values = g_realloc (values, sizeof (GValue) * n_params_alloc);
+                }
+
+              for (i = 0; i < n_params; i++)
+                params[i].value = &values[i];
             }
-          else if (n_params > 16)
-            params = g_renew (GObjectConstructParam, params, n_params + 1);
 
           params[n_params].pspec = pspec;
-          params[n_params].value = g_newa (GValue, 1);
-          memset (params[n_params].value, 0, sizeof (GValue));
+          params[n_params].value = &values[n_params];
+          memset (&values[n_params], 0, sizeof (GValue));
 
-          G_VALUE_COLLECT_INIT (params[n_params].value, pspec->value_type, var_args, 0, &error);
+          G_VALUE_COLLECT_INIT (&values[n_params], pspec->value_type, var_args, 0, &error);
 
           if (error)
             {
               g_critical ("%s: %s", G_STRFUNC, error);
-              g_value_unset (params[n_params].value);
+              g_value_unset (&values[n_params]);
               g_free (error);
               break;
             }
@@ -2267,8 +2284,11 @@ g_object_new_valist (GType        object_type,
       while (n_params--)
         g_value_unset (params[n_params].value);
 
-      if (params != stack_params)
-        g_free (params);
+      if (G_UNLIKELY (n_params_alloc != G_N_ELEMENTS (params_stack)))
+        {
+          g_free (params);
+          g_free (values);
+        }
     }
   else
     /* Fast case: no properties passed in. */