Repository navigation
Slot-access API, rehearsal build and migration gate (#1665 arm D B0) - #1702
Conversation
Batch B0 of the arm D step-2 design (#1665 comment "Arm D step 2 design"). Storage stays Value**; no production call site changes. - src/container.h: borrowed slot reads, list_elem_type, list_get_ref, checked eigs_slot_num family, allocation-free stack views (EIGS_VIEW, caller-owned storage so two live borrows never alias; a cleanup guard aborts if a view's refcount moved), owned/return getters, num_guarded writes, take/move/sort, dict mirrors. - make slot-rehearsal (-DEIGS_SLOT_REHEARSAL): post-flip semantics on today's storage (numbers decode to immediates, owned/return copies are fresh) so each migrated file is proven flip-safe before the flip. - vm_borrow_scan aborts on a builtin result that is an escaped view (EIGS_BORROW_GUARD / rehearsal). - tools/container_migrated_files.txt + container_access_check.sh forbid the legacy borrow API in migrated files ([99ze] checks both verdicts). - tests/test_container_slots.c ([0ff]): every API function on numbers, strings, nested lists, null, bools; planted faults: forbidden borrow, escaped view abort, two simultaneous numeric views stay distinct. Builder gates (Codex Cloud): release 4217/4217, slot-rehearsal 4218/4218, ASan detect_leaks=1 4203/4203 (tally 0), jit-smoke, freestanding-check, werror_switch_check, container/num-read/shape gates. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0481c86caa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static inline EigsSlot dict_value_take_slot(Value *dict, int i) { | ||
| Value *v = dict->data.dict.vals[i]; | ||
| EigsSlot s = eigs_container_slot(v); | ||
| if (slot_is_num(s)) val_decref(v); | ||
| return s; |
There was a problem hiding this comment.
dict_value_take_slot returns the dictionary's only owned reference without removing or replacing the entry. If a caller follows the ownership implied by take and calls slot_decref—as the new test does—the dictionary retains a dangling pointer and later access or destruction can use or free it again; under slot rehearsal, numeric entries are freed inside this function and become dangling immediately. Replace the entry with a safe null/empty value before transferring the slot.
Useful? React with 👍 / 👎.
| while IFS= read -r hit; do | ||
| [[ -z $hit ]] && continue | ||
| migrated_hits=$((migrated_hits + 1)) | ||
| echo "forbidden migrated container access: $file:$hit" >&2 | ||
| migrated_violations=$((migrated_violations + 1)) | ||
| done < <(grep -n -E 'list_get_borrow[[:space:]]*[(]|dict_value_get_borrow[[:space:]]*[(]|(^|[^[:alnum:]_])dict_get(_hashed|_cached)?[[:space:]]*[(]|list_values_storage[[:space:]]*[(]' "$file" || true) |
There was a problem hiding this comment.
The migration gate only recognizes a subset of the pointer-based APIs, so an enrolled file can still call list_iter_borrow, dict_value_iter_borrow, list_get_num, dict_value_get_num, dict_values_storage, or either *_storage_ref helper and receive a clean violations=0 verdict. These helpers all read the current Value ** representation and will therefore leave supposedly migrated files unready for the slot-storage flip; include all legacy compatibility and raw-storage accessors in this scan.
Useful? React with 👍 / 👎.
| variant=release | ||
| if [ -e ../build/slot-rehearsal/eigenscript ] && [ ./eigenscript -ef ../build/slot-rehearsal/eigenscript ]; then | ||
| variant=slot-rehearsal | ||
| elif [ -e ../build/asan/eigenscript ] && [ ./eigenscript -ef ../build/asan/eigenscript ]; then | ||
| variant=asan |
There was a problem hiding this comment.
When the suite runs against any variant other than exactly asan or slot-rehearsal, this falls back to release; in particular, the checked ASan workflow builds asan-server and then runs run_all_tests.sh, so this section silently compiles and executes an uninstrumented release fixture. Consequently the new ownership and lifetime API receives no sanitizer coverage in the repository's actual ASan suite. Derive the hard-linked active variant (including asan-server and asan-pool-off) instead of defaulting those runs to release.
Useful? React with 👍 / 👎.
| static inline void list_sort(Value *list, int (*cmp)(const void *, const void *)) { | ||
| qsort(list->data.list.items, (size_t)list->data.list.count, sizeof(Value *), cmp); |
There was a problem hiding this comment.
list_sort exposes the underlying storage cells directly to its comparator, so the new rehearsal test receives Value ** cells and stays green even though the post-flip array will contain EigsSlot values. A comparator written for today's Value ** representation will dereference tagged slot bits after the flip, while a slot-aware comparator cannot run correctly in rehearsal, meaning sort call sites cannot be migrated safely before the representation change. Define the callback in terms of logical EigsSlot values and adapt the current boxed storage inside this helper.
Useful? React with 👍 / 👎.
2 participants
Part of #1665: batch B0 of the arm D step-2 design. Storage stays
Value**, and no production call site changes.What it adds
src/container.hslot API:list_elem_type;list_get_ref, which returns NULL for a number;eigs_slot_numfamily;num_guard, take/move/sort, and the dict mirrors.make slot-rehearsal: applies post-flip semantics on today's storage (numbers decode to immediates, owned and return copies are fresh), so batches B1–B9 can prove each migrated file safe before the flip.vm_borrow_scanaborts on a builtin result that is an escaped view.tools/container_migrated_files.txt.container_access_check.shforbids the old borrow API in any listed file, and[99ze]requires both verdict lines.tests/test_container_slots.c(section[0ff]) tests every API function on numbers, strings, nested lists, null and bools, with three planted faults:Gates
detect_leaks=1: 4203/4203, leak tally 0;jit-smoke,freestanding-checkandwerror_switch_check.examined=26 allowed=26 violations=0), num-read and shape contract (190 candidates, 0 errors) pass. The one shape hash re-pin is an exempt builtin whose helper closure includesvm_borrow_scan, which gained the escape guard.🤖 Generated with Claude Code
https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF