Bug in boxed handling causing memory corruption

[daym]

Message:

Something like g_boxed_free NULL

then corrupts heap.

Test:

diff --git a/test/gtk.scm b/test/gtk.scm
index 2252f96..006b1b7 100644
--- a/test/gtk.scm
+++ b/test/gtk.scm
@@ -134,6 +134,17 @@
         (tree-view (tree-view:new)))
     (tree-view:set-model tree-view tree-store)))
 
+(test-assert "tree-view:set-model with null GClosure, then read it"
+  (let ((tree-store (tree-store:new (vector <GClosure>)))
+        (tree-view (tree-view:new))
+        (iter (make <GtkTreeIter>))
+        (value (make <GValue>)))
+    (tree-view:set-model tree-view tree-store)
+    (tree-store:append! tree-store iter #f)
+    (tree-store:get-value! tree-store (tree-model:iter-first tree-store) 0 value)
+    (value)
+    (exit 1)))
+
 (test-assert "load DrawingArea"
   (every load-by-name?
          '("Gtk" "Gtk" "Gtk" "Gtk" "Gtk")

[LordYuuma]

Uhm, what exactly are you trying to achieve here? The tree-store:append! call appears to be storing a <GClosure> with value NULL, which in my opinion can not end well.

[daym]

In my opinion, bindings should not corrupt the heap.

I don't store anything anywhere here, explicitly.

This is completely normal stuff.

In general, for all the issues I open here:

(1) it actually happens to me in a normal program I wrote, without trying to make it happen
(2) I find a minimal reproducer (I hope I got it to corrupt in general--otherwise it's gonna be difficult to find)
(3) I write a similar program in C to compare

So I did here. This totally works, and the output is (nil):

#include <gtk/gtk.h>

int main(int argc, char* argv[]) {
        GtkTreeIter iter;
        void* p = NULL;
        gtk_init(&argc, &argv);
        GtkTreeStore* store = gtk_tree_store_new(1, G_TYPE_CLOSURE);
        gtk_tree_store_append(store, &iter, NULL);
        gtk_tree_model_get(GTK_TREE_MODEL(store), &iter, 0, &p, -1);
        g_warning("%p", p);
        return 0;
}

Now, I'm reading the Gtk source code...

In this case, I use G_TYPE_CLOSURE as a workaround in order to store a Guix <package> record into a GtkTreeStore row. When I read a Guix manifest, I do not have the package yet (only name, version, output name and provenance)--the respective package is lazy-loaded on demand.

[LordYuuma]

Hmm, my results inside Guix environment are slightly different, but as we know from #96, we can't trust those. I've pushed a commit with a test, that has been inspired by yours, perhaps that solves it or perhaps it breaks the CI.

[daym]

Thanks.

For reference, this is in gtktreedatalist.c :

void
_gtk_tree_data_list_free (GtkTreeDataList *list,
                          GType           *column_headers)
{
  GtkTreeDataList *tmp, *next;
  gint i = 0;

  tmp = list;

  while (tmp)
    {
      next = tmp->next;
      if (g_type_is_a (column_headers [i], G_TYPE_STRING))
        g_free ((gchar *) tmp->data.v_pointer);
      else if (g_type_is_a (column_headers [i], G_TYPE_OBJECT) && tmp->data.v_pointer != NULL)
        g_object_unref (tmp->data.v_pointer);
      else if (g_type_is_a (column_headers [i], G_TYPE_BOXED) && tmp->data.v_pointer != NULL)
        g_boxed_free (column_headers [i], (gpointer) tmp->data.v_pointer);
      else if (g_type_is_a (column_headers [i], G_TYPE_VARIANT) && tmp->data.v_pointer != NULL)
        g_variant_unref ((gpointer) tmp->data.v_pointer);

      g_slice_free (GtkTreeDataList, tmp);
      i++;
      tmp = next;
    }
}

That suggests that g_boxed_free, g_object_unref, g_variant_unref don't check for NULL.

Also, the _gtk_tree_data_list_alloc (in the same file) uses g_slice_new0 to allocate the thing. That implies that the initial value of v_pointer will be NULL.

[LordYuuma]

Fix pushed. For the record, your code contained a fair number of typos, so here is the updated and fixed test case:

guile-gi/test/gtk.scm

Lines 139 to 148 in b398733

	(test-equal "tree-store:append! (<GClosure> NULL)"
	#f
	(let ((tree-store (tree-store:new (vector <GClosure>)))
	(tree-view (tree-view:new))
	(iter (make <GtkTreeIter>))
	(value (make <GValue>)))
	(tree-view:set-model tree-view tree-store)
	(tree-store:append! tree-store iter #f)
	(tree-model:get-value! tree-store iter 0 value)
	(value)))