[glib/th/gobject-new-parameter-list: 2/2] gobject: allocate parameter list for g_object_new_valist() entirely on stack

[Thomas Haller <thaller src gnome org>]

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

    gobject: allocate parameter list for g_object_new_valist() entirely on stack
    
    When parsing the argument list for g_object_new_valist() we need to
    build a temporary list.
    
    Note that already previously, all the GValue instances were allocate on the
    stack, with g_newa(). That means, previously we allocated on the stack
    (16 * sizeof (GObjectConstructParam) + n * sizeof (GValue)) bytes (which
    is (256 + n * 24), on 64 bit).
    
    Go one step further and allocate also the excess parameter list on the stack.
    Now we require O(n * 16 + n * 56) on the stack.
    
    Maybe it is a problem to allocate that much data on the stack. If that is
    a concern, we should limit the amount of bytes we allocate on the stack.
    On the other hand, n itself is the number of parameters of g_object_new_valist()
    and themself on the stack. That means, n itself is limited by an
    unknown amount of available stack and each parameter in the valist list
    probably requires 12 or 16 bytes (on 64 bit, for the aligned name pointer and
    the argument, depending on whether the type is int or a pointer).
    
    Also, previously the buffer was not grown exponentially during
    
      g_renew (GObjectConstructParam, params, n_params + 1)
    
    although I think it should be.

 gobject/gobject.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)
---
diff --git a/gobject/gobject.c b/gobject/gobject.c
index b5b1e88c4..9906c07d1 100644
--- a/gobject/gobject.c
+++ b/gobject/gobject.c
@@ -2217,12 +2217,13 @@ g_object_new_valist (GType        object_type,
   if (first_property_name)
     {
       GObjectConstructParam stack_params[16];
-      GObjectConstructParam *params;
+      GObjectConstructParam *params_old;
+      GObjectConstructParam *params = stack_params;
       const gchar *name;
-      gint n_params = 0;
+      guint n_params = 0;
+      guint n_params_alloc = G_N_ELEMENTS (stack_params);
 
       name = first_property_name;
-      params = stack_params;
 
       do
         {
@@ -2234,13 +2235,27 @@ 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);
+              /* Note that we allocate the new list of params on the stack but cannot free
+               * the old lists, as they were allocated with alloca.
+               * But since we double the size of the allocated list, the combined size of
+               * all the unused, old lists is itself smaller than the new allocation.
+               *
+               * In other words, for n arguments we allocate less than (n * 2 * sizeof 
(GObjectConstructParam))
+               * for the params list, and additional (n * sizeof (GValue)) for the values. On 64 bit,
+               * GObjectConstructParam is 16 bytes and GValue is 24 bytes. So the total memory allocated
+               * with alloca() is less than (n * 56) bytes.
+               *
+               * Also note that for g_object_new_valist() the number of arguments is on
+               * the stack to begin with. That number is itself limited by the available
+               * stack, and we merely use additional stack space that scales linearely
+               * with the number of arguments. */
+              params_old = params;
+              params = g_newa (GObjectConstructParam, n_params_alloc * 2u);
+              memcpy (params, params_old, sizeof (params[0]) * n_params_alloc);
+              n_params_alloc *= 2u;
             }
-          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);
@@ -2264,9 +2279,6 @@ g_object_new_valist (GType        object_type,
 
       while (n_params--)
         g_value_unset (params[n_params].value);
-
-      if (params != stack_params)
-        g_free (params);
     }
   else
     /* Fast case: no properties passed in. */