add function aware to gc

2026-02-03 01:20:24 -06:00
parent e680439a9b
commit e4b7de46f6
2 changed files with 84 additions and 74 deletions
--- a/source/cell.c
+++ b/source/cell.c
@@ -126,7 +126,7 @@ void script_startup(cell_rt *prt)
  rt = JS_NewRuntime();

  JSContext *js = JS_NewContextRaw(rt);
-  JS_SetInterruptHandler(rt, actor_interrupt_cb, prt);
+  JS_SetInterruptHandler(rt, (JSInterruptHandler *)actor_interrupt_cb, prt);
  
  JS_AddIntrinsicBaseObjects(js);
  JS_AddIntrinsicEval(js);
--- a/source/quickjs.c
+++ b/source/quickjs.c
@@ -486,7 +486,9 @@ static inline objhdr_t *chase(JSValue v) {
  objhdr_t *oh = JS_VALUE_GET_PTR(v);
  if (objhdr_type(*oh) != OBJ_FORWARD) return oh;
  do {
-    oh = (objhdr_t*)objhdr_fwd_ptr(*oh);
+    objhdr_t *next = (objhdr_t*)objhdr_fwd_ptr(*oh);
+    if (!next) return oh; /* NULL forward pointer (stale reference bug) */
+    oh = next;
  } while (objhdr_type(*oh) == OBJ_FORWARD);
  return oh;
 }
@@ -2200,13 +2202,17 @@ static void buddy_destroy (BuddyAllocator *b) {

 /* Forward declarations for GC helpers */
 static int ctx_gc (JSContext *ctx);
-static JSValue gc_copy_value (JSContext *ctx, JSValue v, uint8_t *to_base, uint8_t **to_free, uint8_t *to_end);
-static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *to_base, uint8_t **to_free, uint8_t *to_end);
+static JSValue gc_copy_value (JSContext *ctx, JSValue v, uint8_t *from_base, uint8_t *from_end, uint8_t *to_base, uint8_t **to_free, uint8_t *to_end);
+static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *from_base, uint8_t *from_end, uint8_t *to_base, uint8_t **to_free, uint8_t *to_end);
 static size_t gc_object_size (void *ptr);

+/* Alignment for GC object sizes - must match max alignment requirement */
+#define GC_ALIGN 8
+static inline size_t gc_align_up (size_t n) { return (n + GC_ALIGN - 1) & ~(GC_ALIGN - 1); }
+
 /* Get size of a heap object based on its type */
 static size_t gc_object_size (void *ptr) {
-  objhdr_t *hdr = ptr;
+  objhdr_t hdr = *(objhdr_t *)ptr;
  uint8_t type = objhdr_type (hdr);
  uint64_t cap = objhdr_cap56 (hdr);

@@ -2214,97 +2220,75 @@ static size_t gc_object_size (void *ptr) {
  case OBJ_ARRAY: {
    /* JSArray + inline values array. Cap is element capacity. */
    size_t values_size = sizeof (JSValue) * cap;
-    return sizeof (JSArray) + values_size;
+    return gc_align_up (sizeof (JSArray) + values_size);
  }
  case OBJ_TEXT: {
    /* JSText: header + pad + hdr + length + packed chars */
    size_t word_count = (cap + 1) / 2;
-    return sizeof (JSText) + word_count * sizeof (uint64_t);
+    return gc_align_up (sizeof (JSText) + word_count * sizeof (uint64_t));
  }
  case OBJ_RECORD: {
    /* JSRecord + inline tab. Cap is mask, so tab size is mask+1 entries. */
    size_t tab_size = sizeof (JSRecordEntry) * (cap + 1);
-    return sizeof (JSRecord) + tab_size;
+    return gc_align_up (sizeof (JSRecord) + tab_size);
  }
  case OBJ_FUNCTION:
-  case OBJ_CODE:
-  case OBJ_FRAME:
-  case OBJ_BLOB:
+    return gc_align_up (sizeof (JSFunction));
  default:
-    /* Conservative estimate for unknown types */
-    return 64;
+    /* Unknown type - fatal error, heap is corrupt or scan desync'd */
+    fprintf (stderr, "gc_object_size: unknown object type %d at %p\n", type, ptr);
+    abort ();
  }
 }

 /* Copy a single value, returning the new value with updated pointer if needed.
-   Follows and collapses forward pointer chains. */
-static JSValue gc_copy_value (JSContext *ctx, JSValue v, uint8_t *to_base,
-                              uint8_t **to_free, uint8_t *to_end) {
-  if (!JS_IsPtr (v)) return v; /* Immediate value - no copy needed */
+   Simple Cheney: if already forwarded, return new address; else copy and forward.
+   from_base/from_end define the OLD heap bounds (where objects are being copied FROM). */
+static JSValue gc_copy_value (JSContext *ctx, JSValue v, uint8_t *from_base, uint8_t *from_end,
+                              uint8_t *to_base, uint8_t **to_free, uint8_t *to_end) {
+  if (!JS_IsPtr (v)) {
+    return v; /* Immediate value - no copy needed */
+  }

  void *ptr = JS_VALUE_GET_PTR (v);
  if (is_stone_ptr (ctx, ptr)) return v; /* Stone memory - don't copy */

-  /* Check if pointer is in current heap (not external allocation) */
-  if ((uint8_t *)ptr < ctx->heap_base || (uint8_t *)ptr >= ctx->heap_end) {
-    /* External allocation (using js_malloc_rt) - keep reference */
+  /* Check if pointer is in old heap (from-space) */
+  if ((uint8_t *)ptr < from_base || (uint8_t *)ptr >= from_end) {
+    /* External allocation - keep reference as-is */
    return v;
  }

  objhdr_t *hdr_ptr = ptr;
  objhdr_t hdr = *hdr_ptr;

-  /* Collect forward pointer chain */
-  objhdr_t *chain[64];
-  int chain_len = 0;
-
-  while (objhdr_type (hdr) == OBJ_FORWARD) {
+  /* Already forwarded? Return the new location. */
+  if (objhdr_type (hdr) == OBJ_FORWARD) {
    void *fwd_target = objhdr_fwd_ptr (hdr);
-
-    /* Stale pointer bug: forward target is NULL (should not happen with proper rooting) */
-    if (!fwd_target) {
-      return v; /* Return original value and let caller handle it */
-    }
-
-    /* Check if forward target is in new space */
-    if ((uint8_t *)fwd_target >= to_base && (uint8_t *)fwd_target < *to_free) {
-      /* Target is in new space - update chain and return */
-      for (int i = 0; i < chain_len; i++) {
-        *chain[i] = objhdr_make_fwd (fwd_target);
-      }
-      return JS_MKPTR (fwd_target);
-    }
-
-    if (chain_len < 64) chain[chain_len++] = hdr_ptr;
-    hdr_ptr = (objhdr_t *)fwd_target;
-    hdr = *hdr_ptr;
+    return JS_MKPTR (fwd_target);
  }

-  /* hdr_ptr points to real object in old space - copy it */
+  /* Copy object to to-space */
  size_t size = gc_object_size (hdr_ptr);
  if (*to_free + size > to_end) {
-    /* Should not happen if we sized new block correctly */
-    return v;
+    /* Out of space - this is a fatal error */
+    fprintf (stderr, "gc_copy_value: out of to-space, need %zu bytes\n", size);
+    abort ();
  }

  void *new_ptr = *to_free;
  memcpy (new_ptr, hdr_ptr, size);
  *to_free += size;

-  /* Install forward in old location */
+  /* Install forward pointer in old location */
  *hdr_ptr = objhdr_make_fwd (new_ptr);

-  /* Update ALL chain pointers to final location */
-  for (int i = 0; i < chain_len; i++) {
-    *chain[i] = objhdr_make_fwd (new_ptr);
-  }
-
  return JS_MKPTR (new_ptr);
 }

 /* Scan a copied object and update its internal references */
-static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *to_base,
-                            uint8_t **to_free, uint8_t *to_end) {
+static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *from_base, uint8_t *from_end,
+                            uint8_t *to_base, uint8_t **to_free, uint8_t *to_end) {
  objhdr_t hdr = *(objhdr_t *)ptr;
  uint8_t type = objhdr_type (hdr);

@@ -2312,7 +2296,7 @@ static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *to_base,
  case OBJ_ARRAY: {
    JSArray *arr = (JSArray *)ptr;
    for (uint32_t i = 0; i < arr->len; i++) {
-      arr->values[i] = gc_copy_value (ctx, arr->values[i], to_base, to_free, to_end);
+      arr->values[i] = gc_copy_value (ctx, arr->values[i], from_base, from_end, to_base, to_free, to_end);
    }
    break;
  }
@@ -2321,7 +2305,7 @@ static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *to_base,
    /* Copy prototype */
    if (rec->proto) {
      JSValue proto_val = JS_MKPTR (rec->proto);
-      proto_val = gc_copy_value (ctx, proto_val, to_base, to_free, to_end);
+      proto_val = gc_copy_value (ctx, proto_val, from_base, from_end, to_base, to_free, to_end);
      rec->proto = (JSRecord *)JS_VALUE_GET_PTR (proto_val);
    }
    /* Copy table entries */
@@ -2329,19 +2313,28 @@ static void gc_scan_object (JSContext *ctx, void *ptr, uint8_t *to_base,
    for (uint32_t i = 0; i <= mask; i++) {
      JSValue k = rec->slots[i].key;
      if (!rec_key_is_empty (k) && !rec_key_is_tomb (k)) {
-        rec->slots[i].key = gc_copy_value (ctx, k, to_base, to_free, to_end);
-        rec->slots[i].val = gc_copy_value (ctx, rec->slots[i].val, to_base, to_free, to_end);
+        rec->slots[i].key = gc_copy_value (ctx, k, from_base, from_end, to_base, to_free, to_end);
+        rec->slots[i].val = gc_copy_value (ctx, rec->slots[i].val, from_base, from_end, to_base, to_free, to_end);
      }
    }
    break;
  }
+  case OBJ_FUNCTION: {
+    JSFunction *fn = (JSFunction *)ptr;
+    /* Scan the function name */
+    fn->name = gc_copy_value (ctx, fn->name, from_base, from_end, to_base, to_free, to_end);
+    /* Note: function_bytecode and var_refs are allocated via js_malloc, not bump allocator */
+    break;
+  }
  case OBJ_TEXT:
  case OBJ_BLOB:
  case OBJ_CODE:
    /* No internal references to scan */
    break;
  default:
-    break;
+    /* Unknown type during scan - fatal error */
+    fprintf (stderr, "gc_scan_object: unknown object type %d at %p\n", type, ptr);
+    abort ();
  }
 }

@@ -2351,6 +2344,14 @@ static int ctx_gc (JSContext *ctx) {
  size_t old_used = ctx->heap_free - ctx->heap_base;
  size_t old_heap_size = ctx->current_block_size;

+  /* Save OLD heap bounds before allocating new block */
+  uint8_t *from_base = ctx->heap_base;
+  uint8_t *from_end = ctx->heap_end;
+
+#ifdef DUMP_GC
+  printf("ctx_gc: from_base=%p from_end=%p size=%zu\n", (void*)from_base, (void*)from_end, old_heap_size);
+#endif
+
  /* Request new block from runtime */
  size_t new_size = ctx->next_block_size;
  uint8_t *new_block = buddy_alloc (&rt->buddy, new_size);
@@ -2366,55 +2367,55 @@ static int ctx_gc (JSContext *ctx) {
  uint8_t *to_end = new_block + new_size;

  /* Copy roots: global object, class prototypes, exception, etc. */
-  ctx->global_obj = gc_copy_value (ctx, ctx->global_obj, to_base, &to_free, to_end);
-  ctx->global_var_obj = gc_copy_value (ctx, ctx->global_var_obj, to_base, &to_free, to_end);
-  ctx->regexp_ctor = gc_copy_value (ctx, ctx->regexp_ctor, to_base, &to_free, to_end);
-  ctx->throw_type_error = gc_copy_value (ctx, ctx->throw_type_error, to_base, &to_free, to_end);
-  ctx->eval_obj = gc_copy_value (ctx, ctx->eval_obj, to_base, &to_free, to_end);
+  ctx->global_obj = gc_copy_value (ctx, ctx->global_obj, from_base, from_end, to_base, &to_free, to_end);
+  ctx->global_var_obj = gc_copy_value (ctx, ctx->global_var_obj, from_base, from_end, to_base, &to_free, to_end);
+  ctx->regexp_ctor = gc_copy_value (ctx, ctx->regexp_ctor, from_base, from_end, to_base, &to_free, to_end);
+  ctx->throw_type_error = gc_copy_value (ctx, ctx->throw_type_error, from_base, from_end, to_base, &to_free, to_end);
+  ctx->eval_obj = gc_copy_value (ctx, ctx->eval_obj, from_base, from_end, to_base, &to_free, to_end);

  for (int i = 0; i < JS_NATIVE_ERROR_COUNT; i++) {
-    ctx->native_error_proto[i] = gc_copy_value (ctx, ctx->native_error_proto[i], to_base, &to_free, to_end);
+    ctx->native_error_proto[i] = gc_copy_value (ctx, ctx->native_error_proto[i], from_base, from_end, to_base, &to_free, to_end);
  }

  /* Copy class prototypes */
  for (int i = 0; i < rt->class_count; i++) {
-    ctx->class_proto[i] = gc_copy_value (ctx, ctx->class_proto[i], to_base, &to_free, to_end);
+    ctx->class_proto[i] = gc_copy_value (ctx, ctx->class_proto[i], from_base, from_end, to_base, &to_free, to_end);
  }

  /* Copy value stack */
  for (int i = 0; i < ctx->value_stack_top; i++) {
-    ctx->value_stack[i] = gc_copy_value (ctx, ctx->value_stack[i], to_base, &to_free, to_end);
+    ctx->value_stack[i] = gc_copy_value (ctx, ctx->value_stack[i], from_base, from_end, to_base, &to_free, to_end);
  }

  /* Copy frame stack references */
  for (int i = 0; i <= ctx->frame_stack_top; i++) {
    struct VMFrame *frame = &ctx->frame_stack[i];
-    frame->cur_func = gc_copy_value (ctx, frame->cur_func, to_base, &to_free, to_end);
-    frame->this_obj = gc_copy_value (ctx, frame->this_obj, to_base, &to_free, to_end);
+    frame->cur_func = gc_copy_value (ctx, frame->cur_func, from_base, from_end, to_base, &to_free, to_end);
+    frame->this_obj = gc_copy_value (ctx, frame->this_obj, from_base, from_end, to_base, &to_free, to_end);
  }

  /* Copy JS_PUSH_VALUE/JS_POP_VALUE roots */
  for (JSGCRef *ref = ctx->top_gc_ref; ref != NULL; ref = ref->prev) {
-    ref->val = gc_copy_value (ctx, ref->val, to_base, &to_free, to_end);
+    ref->val = gc_copy_value (ctx, ref->val, from_base, from_end, to_base, &to_free, to_end);
  }

  /* Copy JS_AddGCRef/JS_DeleteGCRef roots */
  for (JSGCRef *ref = ctx->last_gc_ref; ref != NULL; ref = ref->prev) {
-    ref->val = gc_copy_value (ctx, ref->val, to_base, &to_free, to_end);
+    ref->val = gc_copy_value (ctx, ref->val, from_base, from_end, to_base, &to_free, to_end);
  }

  /* Cheney scan: scan copied objects to find more references */
  uint8_t *scan = to_base;
  while (scan < to_free) {
-    gc_scan_object (ctx, scan, to_base, &to_free, to_end);
+    gc_scan_object (ctx, scan, from_base, from_end, to_base, &to_free, to_end);
    scan += gc_object_size (scan);
  }

  /* Return old block to buddy allocator */
 #ifdef POISON_HEAP
-  gc_poison_region(ctx->heap_base, ctx->current_block_size);
+  gc_poison_region(from_base, old_heap_size);
 #endif
-  buddy_free (&rt->buddy, ctx->heap_base, ctx->current_block_size);
+  buddy_free (&rt->buddy, from_base, old_heap_size);

  /* Update context with new block */
  size_t new_used = to_free - to_base;
@@ -2612,7 +2613,16 @@ JSContext *JS_NewContextRawWithHeapSize (JSRuntime *rt, size_t heap_size) {
  ctx->heap_free = ctx->heap_base;
  ctx->heap_end = ctx->heap_base + ctx->current_block_size;

+#ifdef DUMP_GC
+  printf("Context init: heap_base=%p heap_end=%p size=%zu\n", (void*)ctx->heap_base, (void*)ctx->heap_end, ctx->current_block_size);
+#endif
+
  JS_AddIntrinsicBasicObjects (ctx);
+
+#ifdef DUMP_GC
+  printf("After BasicObjects: heap_base=%p heap_end=%p heap_free=%p\n", (void*)ctx->heap_base, (void*)ctx->heap_end, (void*)ctx->heap_free);
+#endif
+
  return ctx;
 }