diff --git a/changes/internal/1665-vm-slot-api.md b/changes/internal/1665-vm-slot-api.md new file mode 100644 index 00000000..ac406af1 --- /dev/null +++ b/changes/internal/1665-vm-slot-api.md @@ -0,0 +1 @@ +- Migrate VM container element access and builtin argument assembly to the slot API. diff --git a/src/container.h b/src/container.h index 577c9d1d..1cdc2cb6 100644 --- a/src/container.h +++ b/src/container.h @@ -15,6 +15,29 @@ static inline EigsSlot eigs_container_slot(Value *v) { return slot_from_heap(v); } +/* A borrowed read for VALUE use (the VM stack, arithmetic): a boxed number + * becomes an immediate. list_slot stays storage-faithful before the flip + * (a boxed number is a heap slot) because ownership walkers (free_value, GC + * clear, vm_borrow_scan) must still see and release the box; the VM stack + * expects immediates, so its read sites normalise here. Identity after the + * flip, where numbers are already immediate. */ +static inline EigsSlot slot_read_value(EigsSlot s) { + if (slot_is_heap(s) && slot_as_ptr(s)->type == VAL_NUM) + return slot_from_num(VAL_NUM_RAW(slot_as_ptr(s))); + return s; +} +/* slot_read_value + slot_incref in one branch, for the VM hot read sites: a + * boxed number becomes an immediate (no ref taken), any other heap value is + * incref'd, immediates pass through. The returned slot is OWNED. */ +static inline EigsSlot slot_read_owned(EigsSlot s) { + if (slot_is_heap(s)) { + Value *v = slot_as_ptr(s); + if (v->type == VAL_NUM) return slot_from_num(VAL_NUM_RAW(v)); + val_incref(v); + } + return s; +} + /* A slot read is borrowed: it owns no reference and remains valid only until * the container is mutated. In rehearsal a number is an immediate. */ static inline EigsSlot list_slot(const Value *list, int i) { @@ -24,6 +47,26 @@ static inline EigsSlot dict_value_slot(const Value *dict, int i) { return eigs_container_slot(dict->data.dict.vals[i]); } +/* Precondition: the selected element's logical type is VAL_NUM. These raw + * readers intentionally bypass the slot encode/decode round trip for hot + * paths which have already established that contract. They remain valid + * only until the container is mutated, just like the borrowed slot readers. */ +/* Rewrite an already boxed, exclusive numeric element. The caller must pass + * a number already normalized by the VM slot boundary; this deliberately + * avoids a redundant num_guard on the assignment hot path. */ +static inline int dict_value_rewrite_num(const Value *dict, int i, double d) { + Value *v = dict->data.dict.vals[i]; + if (!v || v->type != VAL_NUM || v->refcount != 1) return 0; + VAL_NUM_RAW(v) = d; + return 1; +} +static inline int list_rewrite_num(const Value *list, int i, double d) { + Value *v = list->data.list.items[i]; + if (!v || v->type != VAL_NUM || v->refcount != 1) return 0; + VAL_NUM_RAW(v) = d; + return 1; +} + /* Return the logical type of a borrowed slot without allocating. */ static inline ValType slot_type(EigsSlot s) { if (slot_is_num(s)) return VAL_NUM; @@ -110,6 +153,10 @@ static inline void eigs_view_check(EigsView *view) { #else #define EIGS_VIEW(name) EigsView name = {{0}} #endif +/* Computed goto does not run cleanup attributes (and clang rejects jumping + * out of their protected scope). Dispatch loops use this form and call + * eigs_view_check explicitly before every exit from the view's scope. */ +#define EIGS_VIEW_NOCHECK(name) EigsView name = {{0}} /* Owned/return reads make numeric storage independent. Reference values are * increfed by the owned form; return preserves the builtin direct-child @@ -159,7 +206,8 @@ static inline Value *eigs_slot_box(EigsSlot s) { if (slot_is_num(s)) return make_num(num_guard(SLOT_NUM_RAW(s))); Value *v = eigs_slot_ref(s); val_incref(v); return v; } -static inline void list_set_slot_owned(Value *list, int i, EigsSlot s) { +static inline __attribute__((always_inline)) void +list_set_slot_owned(Value *list, int i, EigsSlot s) { Value *old = list->data.list.items[i]; Value *v = slot_is_num(s) ? make_num(num_guard(SLOT_NUM_RAW(s))) : eigs_slot_ref(s); list->data.list.items[i] = v; @@ -174,7 +222,8 @@ static inline void dict_value_set_slot_owned(Value *dict, int i, EigsSlot s) { static inline void list_append_slot(Value *list, EigsSlot s) { Value *v = eigs_slot_box(s); list_append_owned(list, v); } -static inline void list_set_num(Value *list, int i, double d) { +static inline __attribute__((always_inline)) void +list_set_num(Value *list, int i, double d) { d = num_guard(d); #ifndef EIGS_SLOT_REHEARSAL Value *old = list->data.list.items[i]; diff --git a/src/vm.c b/src/vm.c index 8fa552c5..5865cabe 100644 --- a/src/vm.c +++ b/src/vm.c @@ -642,25 +642,29 @@ static inline void dict_cache_insert(Value *dict, uint32_t h, int idx) { ce[1].dict = dict; ce[1].hash = h; ce[1].index = idx; } -static inline Value *dict_get_cached(Value *dict, const char *key, uint32_t h) { +static inline __attribute__((always_inline)) EigsSlot +vm_dict_slot_cached(Value *dict, const char *key, uint32_t h) { /* #1057: a module namespace is a LIVE VIEW of the module env, so its * own slots are only a mirror — the inline cache must not answer from * them. Route to dict_get_hashed, which projects the current binding. * The JIT's inline probe carries the same guard (emit_dict_cache_probe). */ if (__builtin_expect(dict->module_ns != 0, 0)) - return dict_get_hashed(dict, key, h); + { + Value *v = dict_get_hashed_ref_legacy(dict, key, h); + return v ? eigs_container_slot(v) : slot_null(); + } DictCacheEntry *ce = dict_cache_probe(dict, h); if (ce && ce->index < dict->data.dict.count) { const char *stored = dict->data.dict.keys[ce->index]; if (stored == key || strcmp(stored, key) == 0) - return dict_value_get_borrow(dict, ce->index); + return dict_value_slot(dict, ce->index); } int idx = env_hash_find_dict(dict, key, h); if (idx >= 0) { dict_cache_insert(dict, h, idx); - return dict_value_get_borrow(dict, idx); + return dict_value_slot(dict, idx); } - return NULL; + return slot_null(); } static inline void dict_set_cached(Value *dict, const char *key, uint32_t h, Value *val) { @@ -674,8 +678,7 @@ static inline void dict_set_cached(Value *dict, const char *key, uint32_t h, Val if (stored == key || strcmp(stored, key) == 0) { /* Cache hit — update in place */ val_incref(val); - val_decref(dict_value_get_borrow(dict, ce->index)); - dict_value_set_owned(dict, ce->index, val); + dict_value_set_slot_owned(dict, ce->index, slot_from_heap(val)); return; } } @@ -688,17 +691,19 @@ static inline void dict_set_cached(Value *dict, const char *key, uint32_t h, Val * mutate data.num in place — no allocation, no refcount churn. Returns * 1 if the in-place fast path fired; 0 means the caller must materialize * and call dict_set_cached. */ -static inline int dict_set_cached_immediate(Value *dict, const char *key, uint32_t h, double num) { +static inline __attribute__((always_inline)) int +dict_set_cached_immediate(Value *dict, const char *key, uint32_t h, double num) { if (__builtin_expect(dict->module_ns != 0, 0)) return 0; /* #1057: never in-place on a mirror */ DictCacheEntry *ce = dict_cache_probe(dict, h); if (ce && ce->index < dict->data.dict.count) { const char *stored = dict->data.dict.keys[ce->index]; if (stored == key || strcmp(stored, key) == 0) { - Value *existing = dict_value_get_borrow(dict, ce->index); - if (existing && existing->type == VAL_NUM && - existing->refcount == 1) { - VAL_NUM_RAW(existing) = num; + /* dict_value_get_ref is NULL for a number by contract, so the + * number test goes through the slot. dict_value_set_num keeps the + * rc==1 in-place rewrite on Value** storage and becomes a plain + * slot store after the flip. */ + if (dict_value_rewrite_num(dict, ce->index, num)) { return 1; } } @@ -1360,13 +1365,8 @@ int jit_helper_local_idx_get(int slot, int idx) { } if (target->type == VAL_LIST) { if (i < target->data.list.count) { - Value *item = list_get_borrow(target, i); - if (item && item->type == VAL_NUM) { - vm_push_slot(slot_from_num(VAL_NUM_RAW(item))); - } else { - val_incref(item); - vm_push(item); - } + EigsSlot item = slot_read_owned(list_slot(target, i)); + vm_push_slot(item); } else { rt_error(EK_INDEX, g_vm.current_line, "index %d out of range (list length %d)", @@ -1415,17 +1415,8 @@ int jit_helper_local_dot_get(EigsChunk *chunk, int slot, int name_idx) { h = env_hash_name(key); if (chunk->const_hashes) chunk->const_hashes[name_idx] = h; } - Value *v = dict_get_cached(target, key, h); - if (v) { - if (v->type == VAL_NUM) { - vm_push_slot(slot_from_num(VAL_NUM_RAW(v))); - } else { - val_incref(v); - vm_push(v); - } - } else { - vm_push_slot(slot_null()); - } + EigsSlot v = slot_read_owned(vm_dict_slot_cached(target, key, h)); + vm_push_slot(v); return g_has_error; } if (target) { @@ -1459,7 +1450,8 @@ int jit_helper_local_idx_dot_get(EigsChunk *chunk, int slot, int i = list_idx; if (target && target->type == VAL_LIST) { if (i < target->data.list.count) { - Value *dict = list_get_borrow(target, i); + EIGS_VIEW(dict_view); + Value *dict = list_get_view(target, i, &dict_view); if (dict && dict->type == VAL_DICT) { const char *key = chunk->const_interns[name_idx]; uint32_t h = chunk->const_hashes @@ -1469,14 +1461,10 @@ int jit_helper_local_idx_dot_get(EigsChunk *chunk, int slot, if (chunk->const_hashes) chunk->const_hashes[name_idx] = h; } - Value *v = dict_get_cached(dict, key, h); - if (v) { - if (v->type == VAL_NUM) { - vm_push_slot(slot_from_num(VAL_NUM_RAW(v))); - } else { - val_incref(v); - vm_push(v); - } + EigsSlot v = slot_read_value(vm_dict_slot_cached(dict, key, h)); + if (!slot_is_null(v)) { + slot_incref(v); + vm_push_slot(v); return g_has_error; } } else if (dict) { @@ -1560,17 +1548,11 @@ void jit_helper_dot_get(EigsChunk *chunk, int name_idx) { } Value *target = vm_pop(); if (target->type == VAL_DICT) { - Value *v = dict_get_cached(target, key, h); - if (v) { - if (v->type == VAL_NUM) { - double n = VAL_NUM_RAW(v); - val_decref(target); - vm_push_slot(slot_from_num(n)); - return; - } - val_incref(v); + EigsSlot v = slot_read_value(vm_dict_slot_cached(target, key, h)); + if (!slot_is_null(v)) { + slot_incref(v); val_decref(target); - vm_push(v); + vm_push_slot(v); return; } } else { @@ -1789,11 +1771,11 @@ int jit_helper_iter_next(void) { Value *state = slot_as_ptr(state_slot); if (!state || state->type != VAL_LIST || state->data.list.count < 2) return 1; - Value *iterable = list_get_borrow(state, 0); + EIGS_VIEW(iterable_view); + Value *iterable = list_get_view(state, 0, &iterable_view); if (!iterable) return 1; - Value *idx_v = list_get_borrow(state, 1); - if (!idx_v || idx_v->type != VAL_NUM) return 1; - int idx = (int)VAL_NUM_RAW(idx_v); + if (list_elem_type(state, 1) != VAL_NUM) return 1; + int idx = (int)eigs_slot_num(list_slot(state, 1), "for"); int len = 0; if (iterable->type == VAL_LIST) len = iterable->data.list.count; else if (iterable->type == VAL_BUFFER) len = iterable->data.buffer.count; @@ -1801,31 +1783,24 @@ int jit_helper_iter_next(void) { /* #491: cap by the entry-time snapshot (items[2]) — see make_iter_state. * Mirrors CASE(ITER_NEXT). */ if (state->data.list.count >= 3) { - Value *snap_v = list_get_borrow(state, 2); + EIGS_VIEW(snap_v_view); + Value *snap_v = list_get_view(state, 2, &snap_v_view); if (snap_v && snap_v->type == VAL_NUM) { int snap = (int)VAL_NUM_RAW(snap_v); if (snap < len) len = snap; } } if (idx >= len) return 1; - Value *elem; + EigsSlot elem; if (iterable->type == VAL_BUFFER) { - elem = make_num(buffer_read_num(iterable, idx)); + elem = slot_from_num(num_guard(buffer_read_num(iterable, idx))); } else { - elem = list_get_borrow(iterable, idx); - val_incref(elem); + elem = slot_read_owned(list_slot(iterable, idx)); } /* In-place idx bump when the slot is an exclusive plain VAL_NUM * (matches the interpreter's NUM_REUSE fast path). */ - if (idx_v->type == VAL_NUM && idx_v->refcount == 1) { - VAL_NUM_RAW(idx_v) = (double)(idx + 1); - } else { - val_decref(idx_v); - /* #873: heap-force when the state list must outlive an active - * loop state. */ - list_set_owned(state, 1, make_num(idx + 1)); - } - vm_push(elem); + list_set_num(state, 1, (double)(idx + 1)); + vm_push_slot(elem); return g_has_error ? 2 : 0; } @@ -1921,16 +1896,14 @@ static int vm_leaf_accessor_exec(EigsChunk *c, int argc) { h = env_hash_name(key); c->const_hashes[name_idx] = h; } - Value *v = dict_get_cached(target, key, h); - if (!v || v->type == VAL_NULL) { + EigsSlot v = slot_read_value(vm_dict_slot_cached(target, key, h)); + if (slot_is_null(v)) { /* Missing field is null in the interpreter too. NOTE: * slot_from_value would TAKE OWNERSHIP (decref) of a * null/num input — these are borrows, so wrap by hand. */ mini[msp++] = slot_null(); - } else if (v->type == VAL_NUM) { - mini[msp++] = slot_from_num(VAL_NUM_RAW(v)); } else { - mini[msp++] = slot_from_heap(v); /* borrow, no incref */ + mini[msp++] = v; /* borrow, no incref */ } break; } @@ -1946,14 +1919,7 @@ static int vm_leaf_accessor_exec(EigsChunk *c, int argc) { if (!vm_index_is_int(idx_d, &i)) return 0; if (target->type == VAL_LIST) { if (!vm_index_resolve(&i, target->data.list.count)) return 0; - Value *r = list_get_borrow(target, i); - if (!r) return 0; - if (r->type == VAL_NUM) - mini[msp++] = slot_from_num(VAL_NUM_RAW(r)); - else if (r->type == VAL_NULL) - mini[msp++] = slot_null(); - else - mini[msp++] = slot_from_heap(r); /* borrow, no incref */ + mini[msp++] = slot_read_value(list_slot(target, i)); /* borrow, no incref */ } else if (target->type == VAL_BUFFER) { if (!vm_index_resolve(&i, target->data.buffer.count)) return 0; /* #1417: this speculative evaluator has no callee frame and @@ -2225,14 +2191,8 @@ void jit_helper_index_set(void) { } if (target->type == VAL_LIST && slot_is_num(val_s)) { if (_ok && vm_index_resolve(&i, target->data.list.count)) { - Value *existing = list_get_borrow(target, i); - if (existing && existing->type == VAL_NUM && - existing->refcount == 1) { - VAL_NUM_RAW(existing) = SLOT_NUM_RAW(val_s); - } else { - val_decref(existing); - list_set_owned(target, i, make_num(SLOT_NUM_RAW(val_s))); - } + double n = SLOT_NUM_RAW(val_s); /* already guarded in a slot */ + if (!list_rewrite_num(target, i, n)) list_set_num(target, i, n); } else { if (!_ok) rt_error(EK_VALUE, g_vm.current_line, "index must be an integer, got %g", SLOT_NUM_RAW(idx_s)); else rt_error(EK_INDEX, g_vm.current_line, "index %d out of range (list length %d)", @@ -2252,8 +2212,7 @@ void jit_helper_index_set(void) { rt_error(EK_VALUE, g_vm.current_line, "index must be an integer, got %g", VAL_NUM_RAW(idx)); } else if (vm_index_resolve(&i, target->data.list.count)) { val_incref(val); - val_decref(list_get_borrow(target, i)); - list_set_owned(target, i, val); + list_set_slot_owned(target, i, slot_from_heap(val)); } else { rt_error(EK_INDEX, g_vm.current_line, "index %d out of range (list length %d)", i, target->data.list.count); } @@ -2288,19 +2247,9 @@ int jit_helper_index_get(void) { int i, _ok = vm_index_is_int(SLOT_NUM_RAW(idx_s), &i); if (target->type == VAL_LIST) { if (_ok && vm_index_resolve(&i, target->data.list.count)) { - Value *r = list_get_borrow(target, i); - if (r && r->type == VAL_NUM) { - /* See CASE(INDEX_GET) for the use-after-free story — - * snapshot VAL_NUM_RAW(r) before slot_decref(tgt_s) - * potentially frees r via the num freelist. */ - double rv = VAL_NUM_RAW(r); - slot_decref(tgt_s); - vm_push_slot(slot_from_num(rv)); - } else { - val_incref(r); - slot_decref(tgt_s); - vm_push(r); - } + EigsSlot r = slot_read_owned(list_slot(target, i)); + slot_decref(tgt_s); + vm_push_slot(r); } else { if (!_ok) rt_error(EK_VALUE, g_vm.current_line, "index must be an integer, got %g", SLOT_NUM_RAW(idx_s)); else rt_error(EK_INDEX, g_vm.current_line, @@ -2338,16 +2287,16 @@ int jit_helper_index_get(void) { if (!vm_index_is_int(VAL_NUM_RAW(idx), &i)) { rt_error(EK_VALUE, g_vm.current_line, "index must be an integer, got %g", VAL_NUM_RAW(idx)); } else if (vm_index_resolve(&i, target->data.list.count)) { - result = list_get_borrow(target, i); - val_incref(result); + result = list_get_owned(target, i); } else { rt_error(EK_INDEX, g_vm.current_line, "index %d out of range (list length %d)", i, target->data.list.count); } } else if (target->type == VAL_DICT && idx->type == VAL_STR) { - Value *v = dict_get(target, idx->data.str); - if (v) { result = v; val_incref(result); } + EigsSlot v = vm_dict_slot_cached(target, idx->data.str, + env_hash_name(idx->data.str)); + if (!slot_is_null(v)) result = eigs_slot_box(v); } else if (target->type == VAL_STR && idx->type == VAL_NUM) { int i; if (!vm_index_is_int(VAL_NUM_RAW(idx), &i)) { @@ -2382,7 +2331,7 @@ int jit_helper_index_get(void) { /* Direct-borrow heuristic, shared by the three builtin call sites * (CASE(CALL), jit_helper_call, OP_DISPATCH): if a builtin's result is * one of arg's top-level items (coalesce/append/dict_set return - * list_get_borrow(arg, 0)), incref it so it survives the arg-decref + * list_get_view(arg, 0, &view)), incref it so it survives the arg-decref * that follows. Builtins returning a fresh allocation never match, so * no spurious ref is added; nested borrows (get_at: items[0][idx]) * must still incref locally — only direct children are scanned. @@ -2438,9 +2387,11 @@ static inline void vm_borrow_scan(Value *arg, Value *result, * borrow beyond the effective cap without exposing a builtin. */ if (getenv("EIGS_BORROW_GUARD_PLANT")) n = 0; #endif - Value **items = list_values_storage(arg); for (int i = 0; i < n; i++) { - if (items[i] == result) { val_incref(result); return; } + EigsSlot item = list_slot(arg, i); + if (slot_is_heap(item) && slot_as_ptr(item) == result) { + val_incref(result); return; + } } #if EIGS_BORROW_GUARD /* #548: the capped scan found no borrow — prove there is none @@ -2448,7 +2399,8 @@ static inline void vm_borrow_scan(Value *arg, Value *result, * incref: the VM would push a result it doesn't own, and the * over-release becomes a heisenbug UAF. Fail loudly instead. */ for (int i = n; i < arg->data.list.count; i++) { - if (items[i] == result) { + EigsSlot item = list_slot(arg, i); + if (slot_is_heap(item) && slot_as_ptr(item) == result) { fprintf(stderr, "EigenScript FATAL: borrow-scan guard (#548): builtin " "'%s' returned a borrowed direct child at arg index %d, " @@ -2615,7 +2567,7 @@ int jit_helper_call(EigsChunk *caller_chunk, int argc, int resume_off) { } else { Value *arg_list = make_list_heap(argc); /* #873: bound as a param slot — heap so it cannot dangle (and no deep-promote cost) */ for (int i = 0; i < argc; i++) - list_append(arg_list, STK_AS_VAL(g_vm.sp - argc + i)); + list_append_slot(arg_list, g_vm.stack[g_vm.sp - argc + i]); env_rebind_param_slot(call_env, 0, slot_from_heap(arg_list)); val_decref(arg_list); @@ -2645,7 +2597,7 @@ int jit_helper_call(EigsChunk *caller_chunk, int argc, int resume_off) { } else { Value *arg_list = make_list_heap(argc); /* #873: bound as a param slot — heap so it cannot dangle (and no deep-promote cost) */ for (int i = 0; i < argc; i++) - list_append(arg_list, STK_AS_VAL(g_vm.sp - argc + i)); + list_append_slot(arg_list, g_vm.stack[g_vm.sp - argc + i]); vm_bind_fresh_param(call_env, 0, fn_val->data.fn.params[0], fn_val->data.fn.param_intern_tbl, ph, slot_from_heap(arg_list)); @@ -2717,7 +2669,7 @@ int jit_helper_call(EigsChunk *caller_chunk, int argc, int resume_off) { * below releases the list_append increfs on its items. */ arg = make_list_heap(argc); for (int i = 0; i < argc; i++) { - list_append(arg, STK_AS_VAL(g_vm.sp - argc + i)); + list_append_slot(arg, g_vm.stack[g_vm.sp - argc + i]); } } for (int i = 0; i < argc; i++) @@ -4197,7 +4149,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, /* Heap-forced: see jit_helper_call wrapper rationale. */ arg = make_list_heap(argc); for (int i = 0; i < argc; i++) { - list_append(arg, STK_AS_VAL(g_vm.sp - argc + i)); + list_append_slot(arg, g_vm.stack[g_vm.sp - argc + i]); } } /* Pop args + fn from stack (slot-direct). */ @@ -4299,7 +4251,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, } else { Value *arg_list = make_list_heap(argc); /* #873: bound as a param slot — heap so it cannot dangle (and no deep-promote cost) */ for (int i = 0; i < argc; i++) - list_append(arg_list, STK_AS_VAL(g_vm.sp - argc + i)); + list_append_slot(arg_list, g_vm.stack[g_vm.sp - argc + i]); env_rebind_param_slot(call_env, 0, slot_from_heap(arg_list)); val_decref(arg_list); @@ -4331,7 +4283,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, } else { Value *arg_list = make_list_heap(argc); /* #873: bound as a param slot — heap so it cannot dangle (and no deep-promote cost) */ for (int i = 0; i < argc; i++) - list_append(arg_list, STK_AS_VAL(g_vm.sp - argc + i)); + list_append_slot(arg_list, g_vm.stack[g_vm.sp - argc + i]); vm_bind_fresh_param(call_env, 0, fn_val->data.fn.params[0], fn_val->data.fn.param_intern_tbl, ph, slot_from_heap(arg_list)); @@ -4547,7 +4499,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, Value *list = make_list(count); for (int i = 0; i < count; i++) { if (base + i < g_vm.sp) - list_append(list, STK_AS_VAL(base + i)); + list_append_slot(list, g_vm.stack[base + i]); } /* Pop the elements */ for (int i = 0; i < count && g_vm.sp > base; i++) @@ -4592,26 +4544,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, int i, _ok = vm_index_is_int(SLOT_NUM_RAW(idx_s), &i); if (target->type == VAL_LIST) { if (_ok && vm_index_resolve(&i, target->data.list.count)) { - Value *r = list_get_borrow(target, i); - if (r && r->type == VAL_NUM) { - /* Snapshot the double BEFORE decref'ing the list: - * if the stack slot was the sole owner, slot_decref - * frees the list and cascades into free_value(r), - * which memcpy's the num-freelist next pointer over - * r->data. Reading VAL_NUM_RAW(r) after that yields - * freelist garbage (denormal pointer-as-double, or - * the prior head — often 0 — for the first freed - * item). Surfaced by `(call())[i]` inline in an - * arg list; bound-to-local form hides the bug by - * keeping refcount > 1. */ - double rv = VAL_NUM_RAW(r); - slot_decref(tgt_s); - vm_push_slot(slot_from_num(rv)); - } else { - val_incref(r); - slot_decref(tgt_s); - vm_push(r); - } + EigsSlot r = slot_read_owned(list_slot(target, i)); + slot_decref(tgt_s); + vm_push_slot(r); } else { if (!_ok) rt_error(EK_VALUE, current_line, "index must be an integer, got %g", SLOT_NUM_RAW(idx_s)); else rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)", @@ -4647,14 +4582,14 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, if (!vm_index_is_int(VAL_NUM_RAW(idx), &i)) { rt_error(EK_VALUE, current_line, "index must be an integer, got %g", VAL_NUM_RAW(idx)); } else if (vm_index_resolve(&i, target->data.list.count)) { - result = list_get_borrow(target, i); - val_incref(result); + result = list_get_owned(target, i); } else { rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)", i, target->data.list.count); } } else if (target->type == VAL_DICT && idx->type == VAL_STR) { - Value *v = dict_get(target, idx->data.str); - if (v) { result = v; val_incref(result); } + EigsSlot v = vm_dict_slot_cached(target, idx->data.str, + env_hash_name(idx->data.str)); + if (!slot_is_null(v)) result = eigs_slot_box(v); } else if (target->type == VAL_STR && idx->type == VAL_NUM) { int i; if (!vm_index_is_int(VAL_NUM_RAW(idx), &i)) { @@ -4706,15 +4641,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, } if (target->type == VAL_LIST && slot_is_num(val_s)) { if (_ok && vm_index_resolve(&i, target->data.list.count)) { - Value *existing = list_get_borrow(target, i); - /* In-place mutate when slot is an exclusive untracked VAL_NUM. */ - if (existing && existing->type == VAL_NUM && - existing->refcount == 1) { - VAL_NUM_RAW(existing) = SLOT_NUM_RAW(val_s); - } else { - val_decref(existing); - list_set_owned(target, i, make_num(SLOT_NUM_RAW(val_s))); - } + list_set_num(target, i, SLOT_NUM_RAW(val_s)); } else { if (!_ok) rt_error(EK_VALUE, current_line, "index must be an integer, got %g", SLOT_NUM_RAW(idx_s)); else rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)", @@ -4734,8 +4661,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, rt_error(EK_VALUE, current_line, "index must be an integer, got %g", VAL_NUM_RAW(idx)); } else if (vm_index_resolve(&i, target->data.list.count)) { val_incref(val); - val_decref(list_get_borrow(target, i)); - list_set_owned(target, i, val); + list_set_slot_owned(target, i, slot_from_heap(val)); } else { rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)", i, target->data.list.count); } @@ -4775,17 +4701,11 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, if (h == 0) { h = env_hash_name(key); if (chunk->const_hashes) chunk->const_hashes[idx] = h; } Value *target = vm_pop(); if (target->type == VAL_DICT) { - Value *v = dict_get_cached(target, key, h); - if (v) { - if (v->type == VAL_NUM) { - double n = VAL_NUM_RAW(v); - val_decref(target); - vm_push_slot(slot_from_num(n)); - DISPATCH(); - } - val_incref(v); + EigsSlot v = slot_read_value(vm_dict_slot_cached(target, key, h)); + if (!slot_is_null(v)) { + slot_incref(v); val_decref(target); - vm_push(v); + vm_push_slot(v); DISPATCH(); } } else { @@ -4844,19 +4764,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, const char *key = chunk->const_interns[name_idx]; uint32_t h = chunk->const_hashes ? chunk->const_hashes[name_idx] : 0; if (h == 0) { h = env_hash_name(key); if (chunk->const_hashes) chunk->const_hashes[name_idx] = h; } - Value *v = dict_get_cached(target, key, h); - if (v) { - /* Hot DMG path: untracked VAL_NUM field -> push immediate, - * skipping the incref/decref pair on every register read. */ - if (v->type == VAL_NUM) { - vm_push_slot(slot_from_num(VAL_NUM_RAW(v))); - } else { - val_incref(v); - vm_push(v); - } - } else { - vm_push_slot(slot_null()); - } + EigsSlot v = slot_read_owned(vm_dict_slot_cached(target, key, h)); + vm_push_slot(v); } else if (target) { const char *key = chunk->const_interns[name_idx]; rt_error(EK_TYPE, current_line, "cannot access field '%s' on %s", @@ -4920,14 +4829,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, } if (target->type == VAL_LIST) { if (i < target->data.list.count) { - Value *item = list_get_borrow(target, i); - /* Plain VAL_NUM -> immediate; skip incref/decref pair. */ - if (item && item->type == VAL_NUM) { - vm_push_slot(slot_from_num(VAL_NUM_RAW(item))); - } else { - val_incref(item); - vm_push(item); - } + EigsSlot item = slot_read_owned(list_slot(target, i)); + vm_push_slot(item); } else { rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)", i, target->data.list.count); @@ -4964,23 +4867,22 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, Env *e = frame->fn_env; Value *target = vm_local_lift(e, slot); int i = (int)list_idx; + EIGS_VIEW_NOCHECK(dict_view); if (target && target->type == VAL_LIST) { if (i < target->data.list.count) { - Value *dict = list_get_borrow(target, i); + Value *dict = list_get_view(target, i, &dict_view); +#if EIGS_BORROW_GUARD + if (getenv("EIGS_VM_VIEW_GUARD_PLANT")) dict_view.v.refcount++; +#endif if (dict && dict->type == VAL_DICT) { const char *key = chunk->const_interns[name_idx]; uint32_t h = chunk->const_hashes ? chunk->const_hashes[name_idx] : 0; if (h == 0) { h = env_hash_name(key); if (chunk->const_hashes) chunk->const_hashes[name_idx] = h; } - Value *v = dict_get_cached(dict, key, h); - if (v) { - /* Hot DMG path: register dict[field] returning a - * plain VAL_NUM -> push immediate. */ - if (v->type == VAL_NUM) { - vm_push_slot(slot_from_num(VAL_NUM_RAW(v))); - } else { - val_incref(v); - vm_push(v); - } + EigsSlot v = slot_read_value(vm_dict_slot_cached(dict, key, h)); + if (!slot_is_null(v)) { + slot_incref(v); + vm_push_slot(v); + eigs_view_check(&dict_view); DISPATCH(); } } else if (dict) { @@ -4998,6 +4900,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, rt_error(EK_TYPE, current_line, "cannot index null"); } vm_push_slot(slot_null()); + eigs_view_check(&dict_view); DISPATCH(); } @@ -5010,9 +4913,10 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, Env *e = frame->fn_env; Value *target = vm_local_lift(e, slot); int i = (int)list_idx; + EIGS_VIEW_NOCHECK(dict_view); if (target && target->type == VAL_LIST) { if (i < target->data.list.count) { - Value *dict = list_get_borrow(target, i); + Value *dict = list_get_view(target, i, &dict_view); if (dict && dict->type == VAL_DICT) { const char *key = chunk->const_interns[name_idx]; uint32_t h = chunk->const_hashes ? chunk->const_hashes[name_idx] : 0; @@ -5023,6 +4927,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, EigsSlot tos = g_vm.stack[g_vm.sp - 1]; if (slot_is_num(tos) && dict_set_cached_immediate(dict, key, h, SLOT_NUM_RAW(tos))) { + eigs_view_check(&dict_view); DISPATCH(); } Value *val = vm_slot_lift(g_vm.sp - 1); @@ -5032,6 +4937,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, rt_error(EK_TYPE, current_line, "cannot assign field '%s' on %s", key, val_type_name(dict->type)); } + eigs_view_check(&dict_view); } else { rt_error(EK_INDEX, current_line, "index %d out of range (list length %d)", i, target->data.list.count); @@ -5065,13 +4971,13 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, * bare ITER_NEXT with any list on TOS, and reading VAL_NUM_RAW() off a * VAL_STR is a type confusion whose result then indexes items[] (#721). */ if (!state || state->type != VAL_LIST || state->data.list.count < 2 || - !list_get_borrow(state, 1) || - list_get_borrow(state, 1)->type != VAL_NUM) { + list_elem_type(state, 1) != VAL_NUM) { ip += exit_offset; DISPATCH(); } - Value *iterable = list_get_borrow(state, 0); - int idx = (int)VAL_NUM_RAW(list_get_borrow(state, 1)); + EIGS_VIEW_NOCHECK(iterable_view); + Value *iterable = list_get_view(state, 0, &iterable_view); + int idx = (int)eigs_slot_num(list_slot(state, 1), "for"); int len = 0; if (iterable->type == VAL_LIST) len = iterable->data.list.count; else if (iterable->type == VAL_BUFFER) len = iterable->data.buffer.count; @@ -5079,8 +4985,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, * appends can't extend the loop; min() with the live length keeps a * remove from reading past the end. See make_iter_state. */ if (state->data.list.count >= 3 && - list_get_borrow(state, 2)->type == VAL_NUM) { - int snap = (int)VAL_NUM_RAW(list_get_borrow(state, 2)); + list_elem_type(state, 2) == VAL_NUM) { + int snap = (int)eigs_slot_num(list_slot(state, 2), "for"); if (snap < len) len = snap; } @@ -5093,23 +4999,17 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, } else { /* Update index — mutate in place when the existing slot is * an exclusive plain VAL_NUM (avoids per-iter make_num/free). */ - Value *idx_v = list_get_borrow(state, 1); - if (NUM_REUSE(idx_v)) { - VAL_NUM_RAW(idx_v) = (double)(idx + 1); - } else { - val_decref(idx_v); - list_set_owned(state, 1, make_num(idx + 1)); - } + list_set_num(state, 1, (double)(idx + 1)); if (iterable->type == VAL_BUFFER) { /* Push number immediate directly — skip make_num + immediate- * promote round-trip through vm_push. */ vm_push_slot(slot_from_num(buffer_read_num(iterable, idx))); } else { - Value *elem = list_get_borrow(iterable, idx); - val_incref(elem); - vm_push(elem); + EigsSlot elem = slot_read_owned(list_slot(iterable, idx)); + vm_push_slot(elem); } } + eigs_view_check(&iterable_view); DISPATCH(); } @@ -5753,9 +5653,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, * so each SET op consumes elements in declaration order. Incref each * element so the list's release below doesn't free them prematurely. */ for (int i = n - 1; i >= 0; i--) { - Value *e = list_get_borrow(lst, i); - val_incref(e); - vm_push(e); + EigsSlot e = slot_read_owned(list_slot(lst, i)); + vm_push_slot(e); } slot_decref(tgt_s); DISPATCH(); @@ -5877,9 +5776,8 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, if (target->type == VAL_LIST) { result = make_list(n); for (int i = 0; i < n; i++) { - Value *e = list_get_borrow(target, start + i); - val_incref(e); - list_set_owned(result, i, e); + EigsSlot e = slot_read_owned(list_slot(target, start + i)); + list_set_slot_owned(result, i, e); } result->data.list.count = n; } else if (target->type == VAL_STR) { @@ -6469,7 +6367,9 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, DISPATCH(); } - Value *fn = list_get_borrow(table, key); + EIGS_VIEW_NOCHECK(fn_view); + + Value *fn = list_get_view(table, key, &fn_view); if (fn && fn->type == VAL_BUILTIN) { Value *arg = slot_to_value(arg_s); @@ -6491,6 +6391,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, /* A blocking builtin can return because a peer requested exit. */ POLL_STATE_EXIT(); CHECK_ERROR(); + eigs_view_check(&fn_view); DISPATCH(); } @@ -6543,6 +6444,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, rt_error(EK_LIMIT, current_line, "call stack overflow"); env_decref(call_env); vm_push(make_null()); + eigs_view_check(&fn_view); DISPATCH(); } frame = &g_vm.frames[g_vm.frame_count++]; @@ -6568,6 +6470,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, chunk_decref(fn_chunk); if (g_vm.frame_count <= base_frame) { EigsSlot result_s = g_vm.stack[--g_vm.sp]; + eigs_view_check(&fn_view); if (slot_is_num(result_s)) return make_num(SLOT_NUM_RAW(result_s)); if (slot_is_null(result_s)) return make_null(); if (slot_is_bool(result_s)) return make_bool(slot_as_bool(result_s)); @@ -6577,6 +6480,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, ip = frame->ip; chunk = frame->chunk; current_line = g_vm.current_line; + eigs_view_check(&fn_view); DISPATCH(); } if (fn_chunk->jit_advance == -2) { @@ -6585,6 +6489,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, ip = frame->ip; chunk = frame->chunk; current_line = g_vm.current_line; + eigs_view_check(&fn_view); DISPATCH(); } frame->ip += fn_chunk->jit_advance; @@ -6593,6 +6498,7 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume, ip = frame->ip; chunk = fn_chunk; + eigs_view_check(&fn_view); DISPATCH(); } diff --git a/tests/test_borrow_guard.sh b/tests/test_borrow_guard.sh index 8cb5866f..1cf54cab 100755 --- a/tests/test_borrow_guard.sh +++ b/tests/test_borrow_guard.sh @@ -113,6 +113,25 @@ else fail "borrow plant opt-in control" "rc=$rc got: $out" fi +# VM dispatch uses explicit checks because GCC computed goto skips cleanup +# attributes (and clang rejects those protected scopes outright). Plant a +# moved numeric view in the fused local[idx].field handler and prove its +# explicit pre-DISPATCH check fires in borrow-guard builds. +printf 'define vm_view_plant() as:\n xs is [7]\n return xs[0].a\nprint of vm_view_plant of []\n' \ + > "$TMPDIR/vm_view.eigs" +# `xs[0].a` on a number raises in every build, so a nonzero rc alone does not +# say the plant ran: compare against the same program without the plant. +base=$("$EIGS" "$TMPDIR/vm_view.eigs" 2>&1); base_rc=$? +out=$(EIGS_VM_VIEW_GUARD_PLANT=1 "$EIGS" "$TMPDIR/vm_view.eigs" 2>&1) +rc=$? +if [ "$rc" -eq "$base_rc" ] && [ "$out" = "$base" ]; then + echo " SKIP: non-borrow-guard build — VM view plant compiled out" +elif echo "$out" | grep -q "numeric view refcount moved"; then + ok "planted VM numeric-view retention aborts before dispatch (rc=$rc)" +else + fail "planted VM numeric-view guard diagnostic" "rc=$rc got: $(echo "$out" | head -1)" +fi + echo "" echo "borrow-guard: $PASS passed, $FAIL failed" [ "$FAIL" -eq 0 ] || exit 1 diff --git a/tools/container_migrated_files.txt b/tools/container_migrated_files.txt index d8e7967b..b14d2cb9 100644 --- a/tools/container_migrated_files.txt +++ b/tools/container_migrated_files.txt @@ -2,3 +2,4 @@ src/eigenscript.h src/eigenscript.c src/trace.c +src/vm.c diff --git a/tools/num_read_allowlist.txt b/tools/num_read_allowlist.txt index dca6d9b8..d1e60b90 100644 --- a/tools/num_read_allowlist.txt +++ b/tools/num_read_allowlist.txt @@ -10,6 +10,7 @@ builtins_tensor.c|builtin_tensor_matmul|1|writes the NaN-boxed null sentinel int container.h|dict_value_get_num|1|accessor numeric fast path: reads the number only behind its own type == VAL_NUM test (#1665 arm D) container.h|dict_value_get_owned|1|owned numeric read copies a proven VAL_NUM box for flip-safe ownership container.h|dict_value_get_return|1|builtin-return numeric read copies a proven VAL_NUM box +container.h|dict_value_rewrite_num|1|hot-path rewrite writes only after VAL_NUM and exclusive-ref checks container.h|dict_value_set_num|1|exclusive VAL_NUM box reuse writes only after type and refcount checks container.h|dict_value_set_slot_owned|1|slot write boxes an immediate only after slot_is_num container.h|eigs_container_slot|1|rehearsal decoder reads a box only after type == VAL_NUM @@ -18,8 +19,11 @@ container.h|eigs_slot_elem_num|1|element numeric accessor reads only after slot_ container.h|eigs_slot_num|1|required numeric accessor reads only after slot_is_num container.h|eigs_slot_opt_num|1|optional numeric accessor reads only after slot_is_num container.h|list_get_num|1|accessor numeric fast path: reads the number only behind its own type == VAL_NUM test (#1665 arm D) +container.h|slot_read_owned|1|VM read site: reads the number only behind its own heap + type == VAL_NUM test, to push an immediate (#1665 arm D B2) +container.h|slot_read_value|1|VM read site: reads the number only behind its own heap + type == VAL_NUM test, to push an immediate (#1665 arm D B2) container.h|list_get_owned|1|owned numeric read copies a proven VAL_NUM box for flip-safe ownership container.h|list_get_return|1|builtin-return numeric read copies a proven VAL_NUM box +container.h|list_rewrite_num|1|hot-path rewrite writes only after VAL_NUM and exclusive-ref checks container.h|list_set_num|1|exclusive VAL_NUM box reuse writes only after type and refcount checks container.h|list_set_slot_owned|1|slot write boxes an immediate only after slot_is_num container.h|slot_view|2|numeric view copies a proven immediate into caller storage @@ -66,16 +70,11 @@ value_slot.h|slot_truthy_immediate|1|truthiness after slot_is_num vm.c|#define ARITH_FAST|2|arithmetic fast path after both operands tested VAL_NUM; NUM_REUSE writes vm.c|#define INT_BINOP_R|2|integer op fast path after both slots slot_is_num vm.c|#define NUM_BINOP|6|arithmetic after a->type == VAL_NUM && b->type == VAL_NUM; NUM_REUSE writes -vm.c|dict_set_cached_immediate|1|in-place dict value update, writes a VAL_NUM it exclusively owns -vm.c|jit_helper_dot_get|1|JIT helper: pushes a field value after its VAL_NUM test vm.c|jit_helper_dot_set|1|JIT helper: number store after slot_is_num -vm.c|jit_helper_index_get|10|JIT helper: index read after slot_is_num / VAL_NUM tests on index and element -vm.c|jit_helper_index_set|12|JIT helper: index store after slot_is_num / VAL_NUM tests on index and value -vm.c|jit_helper_iter_next|3|JIT helper: loop state list [iterable, idx, snap] built by the VM, each read behind a VAL_NUM test -vm.c|jit_helper_local_dot_get|1|JIT helper: field value after its VAL_NUM test +vm.c|jit_helper_index_get|9|JIT helper: index read after slot_is_num / VAL_NUM tests on index and element +vm.c|jit_helper_index_set|10|JIT helper: index store after slot_is_num / VAL_NUM tests on index and value +vm.c|jit_helper_iter_next|1|JIT helper: loop state list [iterable, idx, snap] built by the VM, each read behind a VAL_NUM test vm.c|jit_helper_local_dot_set|1|JIT helper: number store after slot_is_num -vm.c|jit_helper_local_idx_dot_get|1|JIT helper: field value after its VAL_NUM test -vm.c|jit_helper_local_idx_get|1|JIT helper: element after its VAL_NUM test vm.c|jit_helper_observe_assign_local|2|JIT helper: observer update after slot_is_num vm.c|jit_helper_observe_name_post|2|JIT helper: observer update after slot_is_num vm.c|loop_iter_store|1|writes the loop counter slot after slot_is_num @@ -84,7 +83,7 @@ vm.c|obs_dump_value|2|SIGUSR1 observer dump after slot_is_num / case VAL_NUM vm.c|slot_as_double|2|VM numeric operand read: returns 0 (not a number) unless slot_is_num or VAL_NUM vm.c|slot_bridge_unwrap|1|NaN-boxing boundary after slot_is_num vm.c|slot_truthy|1|truthiness after slot_is_num -vm.c|vm_leaf_accessor_exec|3|leaf-accessor fast path: constants the scanner admitted only as VAL_NUM (chunk_scan_leaf_accessor) and values behind VAL_NUM tests -vm.c|vm_run_ex|67|the interpreter loop: every opcode reads numbers behind slot_is_num / type == VAL_NUM dispatch; NUM_REUSE in-place writes; the iterator state list is VM-built +vm.c|vm_leaf_accessor_exec|1|leaf-accessor fast path: constants the scanner admitted only as VAL_NUM (chunk_scan_leaf_accessor) and values behind VAL_NUM tests +vm.c|vm_run_ex|57|the interpreter loop: every opcode reads numbers behind slot_is_num / type == VAL_NUM dispatch; NUM_REUSE in-place writes; the iterator state list is VM-built vm.c|vm_slot_lift|1|NaN-boxing boundary after slot_is_num vm.c|vm_store_local_slot|2|in-place local number store after slot_is_num diff --git a/tools/strict_shape_contracts.json b/tools/strict_shape_contracts.json index ff3737cd..5d15e484 100644 --- a/tools/strict_shape_contracts.json +++ b/tools/strict_shape_contracts.json @@ -1464,7 +1464,7 @@ "cab31d774010bcc6b59ae3d4de9f380592fae037878071700a560932bb4f80d3" ], "vm_borrow_scan": [ - "f3944b1ed1b44bb9d331580dac563f62c5964d3fba2d039b36d9c1dab22cadb2" + "ca39b51e6a4ed3f73e7a8cce649ce7fb015114bb312b11a8d67da1d6c7ebcdf4" ] } },