Lua GC Problems

[chromatic]

I've been digging into the Lua on Parrot source code to try to find out why 
there are GC problems (partly to fix them, partly so I can profile Lua 
programs and speed up the system).  I ran into something suspicious in the 
LuaTable PMC.

It uses a custom function to mark all contained keys and values as live, if 
they're GC-able PObjs:

static void lua_mark_table(PARROT_INTERP, Hash *t, STRING *mode)
{
    Node *v = t->node;
    int i;
    int mark_key = 1;
    int mark_val = 1;
    if (mode) {
        mark_key = string_str_index(interp, mode, const_string(interp, "k"), 
0) < 0;
        mark_val = string_str_index(interp, mode, const_string(interp, "v"), 
0) < 0;
    }
    for (i = 0; i < t->size; i++) {
        if (v[i].val != NULL) {
            if (mark_key)
                pobject_lives(interp, (PObj *)v[i].key);
            if (mark_val)
                pobject_lives(interp, (PObj *)v[i].val);
        }
    }
}

However, the node member of a Hash isn't an array:

typedef struct Node {
    PMC *key;
    PMC *val;
    struct Node *next;  /* for chaining */
} Node;

typedef struct Hash {
    Node *node;
    int size;
    Node *firstfree;  /* this position is free; all positions after it are 
full */
    PMC *container;
} Hash;

Now it may indeed be that the linked list happens to be the right size that 
this pointer math might work... but I'm not convinced.  Instead, I expect the 
correct approach to look something more like:

static void lua_mark_table(PARROT_INTERP, Hash *t, STRING *mode)
{
    Node *v      = t->node;
    int mark_key = 1;
    int mark_val = 1;
    if (mode) {
        mark_key = string_str_index(interp, mode, const_string(interp, "k"), 
0) < 0;
        mark_val = string_str_index(interp, mode, const_string(interp, "v"), 
0) < 0;
    }
    
    while (v) { 
        if (v->val) {
            if (mark_key)
                pobject_lives(interp, (PObj *)v->key);
            if (mark_val)
                pobject_lives(interp, (PObj *)v->val);
        }
        v = v->next;
    }
}

Of course, that makes segfaults elsewhere, which may mean that it's wrong, or 
it may mean that there's more hash cleanup necessary in that PMC.  At least 
when I run individual Lua files without --no-gc, they no longer segfault in 
DOD after saying "Method 'setfenv' not found".

Patch (for experimentation, not commit) attached.

-- c

=== pmc/luatable.pmc
==================================================================
--- pmc/luatable.pmc	(revision 6305)
+++ pmc/luatable.pmc	(local)
@@ -270,21 +270,22 @@
 
 static void lua_mark_table(PARROT_INTERP, Hash *t, STRING *mode)
 {
-    Node *v = t->node;
-    int i;
+    Node *v      = t->node;
     int mark_key = 1;
     int mark_val = 1;
     if (mode) {
         mark_key = string_str_index(interp, mode, const_string(interp, "k"), 0) < 0;
         mark_val = string_str_index(interp, mode, const_string(interp, "v"), 0) < 0;
     }
-    for (i = 0; i < t->size; i++) {
-        if (v[i].val != NULL) {
+
+    while (v) {
+        if (v->val) {
             if (mark_key)
-                pobject_lives(interp, (PObj *)v[i].key);
+                pobject_lives(interp, (PObj *)v->key);
             if (mark_val)
-                pobject_lives(interp, (PObj *)v[i].val);
+                pobject_lives(interp, (PObj *)v->val);
         }
+        v = v->next;
     }
 }