luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-11-10 11:26:16 +01:00
Code Issues Packages Projects Releases Wiki Activity

gobject: limit required stack by heap allocating parameter list in g_object_new_valist()

Browse Source 
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 is not bad, because it 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 bit), which probably more than doubles the
required stack space. Let's 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 clearer to keep track of the allocated sizes ourself, and
only call realloc() when we determine that we are out of space.
Especially because we need to update the value pointers on reallocation.

Note that we now require a heap allocation both for the "params" and the
"values" list. Theoretically that could be combined by using one buffer
for both. But that would make the code more complicated.

Now we pre-allocate buffers for 16 elements on the stack. That
is (16 * (16 + 24) bytes (or 640 bytes) on the stack. I think that
is still acceptable.
This commit is contained in:
Thomas Haller 2019-12-12 12:28:01 +01:00
parent 355c99a077
commit fb3977e9b4
1 changed files with 35 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
gobject/gobject.c
View File

@ -2218,13 +2218,15 @@ g_object_new_valist (GType object_type,
if (first_property_name)
{
GObjectConstructParam stack_params[16];
GObjectConstructParam *params;
GObjectConstructParam params_stack[16];
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. */