Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Garbage collection bugs with u8vector #11

Open
elfprince13 opened this issue Sep 3, 2024 · 1 comment
Open

Garbage collection bugs with u8vector #11

elfprince13 opened this issue Sep 3, 2024 · 1 comment

Comments

@elfprince13
Copy link

elfprince13 commented Sep 3, 2024

I've identified two fairly serious bugs with garbage collection as it relates to u8vectors. I expect I will have fixed these in my fork some time this week, but the fork has drifted far enough from this repo that PRing a fix is probably more work than I want to do.

First issue:

  • in the code path where no vectors have yet been allocated compact() places free_vec_pointer in an invalid state:

    picobit/vm/core/gc.c

    Lines 283 to 332 in ece8fde

    void compact ()
    {
    /*
    Move all used blocks to the left.
    This is done by scanning the heap, and moving any taken block to the
    left if there's free space before it.
    */
    obj cur = VEC_TO_RAM_OBJ(MIN_VEC_ENCODING);
    obj prev = 0;
    uint16 cur_size;
    while (cur < free_vec_pointer) {
    cur_size = ram_get_car(cur);
    if (prev && !ram_get_gc_tag0 (prev)) { // previous block is free
    if (!ram_get_gc_tag0 (cur)) { // current is free too, merge free spaces
    // advance cur, but prev stays in place
    cur += cur_size;
    } else { // prev is free, but not cur, move cur to start at prev
    // fix header in the object heap to point to the data's new
    // location
    ram_set_cdr(ram_get_cdr(cur), RAM_TO_VEC_OBJ(prev+1));
    while(cur_size--) { // copy cur's data, which includes header
    ram_set_field0(prev, ram_get_field0(cur));
    ram_set_field1(prev, ram_get_field1(cur));
    ram_set_field2(prev, ram_get_field2(cur));
    ram_set_field3(prev, ram_get_field3(cur));
    cur++;
    prev++;
    }
    // set up a free block where the end of cur's data was
    // (prev is already there from the iteration above)
    ram_set_gc_tag0(prev, 0);
    // at this point, cur is after the new free space, where the
    // next block is
    }
    } else {
    // Go to the next block, which is <size> away from cur.
    prev = cur;
    cur += cur_size;
    }
    }
    // free space is now all at the end
    free_vec_pointer = prev;
    }

    This follows from init_ram_heap() sets free_vec_pointer=VEC_TO_RAM_OBJ(MIN_VEC_ENCODING) (aka 8192). Then when no vectors are allocated, the while loop never executes, and prev is never changed from 0 (an invalid encoding), and free_vec_pointer is set to 0 (which in subsequent code causes underflows when VEC_TO_RAM_OBJ is used.
    Patch: wrap the final update in if (prev)

Second issue:

  • mark() does not seem to properly handle u8vectors. "1 field" objects are treated as though they always store an object reference in their "car"; however, u8vectors store an unencoded length in their car which gets pushed as visit (corollary: there doesn't appear to be any code path to mark the header of the actual vector storage as marked, btw edit: they get a permanent mark when vec cells are created until their owner is swept) , and then interpreted as an object reference. so, for example, in a program with no globals and a single u8vector, the length field gets pushed as visit and a length of 1280 (0x0500) will end up being a self-reference, and a length of 8096 (0x2004) references into the vector's own data bytes, letting you construct arbitrary object references to be traversed by the garbage collector. (edit: can't do this because limited to 13 bit lengths)
    Patch: this is more involved, still working on it, but u8vectors probably need a special code-path separate from 1-field objects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@elfprince13 and others