Repository navigation
builtins.c onto the slot API (#1665 arm D B3) - #1712
Conversation
…onst pool, desc_isolate_const, set_at/insert/copy_into); sort_by owned-arg leak Rehearsal [104] lib/supervise SIGSEGV was a numeric EigsView facade retained in a chunk constant pool by vm_build_chunk_desc_body (stack-use-after-return under rehearsal+ASan). Audit of every get_ref / view site in builtins.c fixed the same class at 10 sites. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
…eturn) + 4 same-class sites Read-only blind review of the migrated files (B0-B3) against the retained-view / ownership class: - get_at (1-D and 2-D) returned list_get_return for a GRANDCHILD of its argument list; vm_borrow_scan only compensates direct children, so a string/list/dict element came back unowned (heap-use-after-free under ASan at teardown). Now list_get_owned. - builtin_dispatch passed a numeric stack view to the inner builtin; one that returns its argument (num) handed back a pointer into dispatch's dead frame (garbage + SIGSEGV in release; stack-use-after-return under ASan). A number argument now goes in as a fresh owned box. - classify: a BUILTIN_LIST_VIEW compound literal died at its if-block but was read after it; the view now has function-scope storage. - chan_clone_rec / values_equal_impl: the depth-64 cap now applies to containers only; scalars are copied / compared by value at any depth. - vm.c CASE(DISPATCH): two exits after fn_view was filled lacked eigs_view_check. tests/test_slot_escape_regress.eigs ([0fh]): red on the unfixed tree under ASan (both bugs), green with the fixes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
…get_owned; no policy change) 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: fa018aa63a
ℹ️ 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".
| slot_decref(list_slot(list, idx)); | ||
| list_set_owned(list, idx, val); |
There was a problem hiding this comment.
When built with EIGS_SLOT_REHEARSAL, list_slot(list, idx) converts the stored boxed number into an immediate slot, and slot_decref is therefore a no-op; list_set_owned then overwrites the actual items[idx] pointer without releasing its box. Repeated numeric set_at calls consequently leak memory unboundedly (the same release pattern also appears in the newly migrated copy_into, truncate, remove, and insert paths), so the storage-owning element must be released rather than the normalized read slot.
Useful? React with 👍 / 👎.
Part of #1665: batch B3 of the arm D step-2 design. All of
src/builtins.cmoves onto the slot API and joins the migrated-files gate. Storage staysValue**.Contents
list_get_return, copies uselist_append_slot, and moves and sorts uselist_move/list_sort.[104]lib/supervise. The cause was a numeric view kept in a chunk's constant pool (stack-use-after-returnunder rehearsal + ASan). The same class was fixed at 10 more sites, plus asort_byowned-argument leak.get_atuse-after-free. It returned a string/list/dict element throughlist_get_return, but that element is a grandchild of the argument list, whichvm_borrow_scannever compensates. It now useslist_get_owned. The suite only ever calledget_aton numbers.dispatchreturned a pointer into its own dead frame: a numeric view passed to a builtin that returns its argument (num). Release printed garbage and then hit SIGSEGV. A number argument now goes in as a fresh owned box.classify: view storage that died with itsifblock;chan_clone_recandvalues_equal_impl: the depth-64 cap treated scalars by pointer;CASE(DISPATCH)exits that skippedeigs_view_check.[0fh]tests/test_slot_escape_regress.eigsis red on the unfixed tree under ASan:heap-use-after-freeforget_atandstack-use-after-returnfordispatch. It is green with the fixes.Gates (run locally)
jit-smokejit_diffdetect_leaks=1[0fh]), leak tally 0[99d](#1708); the shape pin myget_atfix changed is re-pinned, and[99s]re-passescoalescedouble free B2 left in rehearsal is gone; the rest is[99d]and the pre-existing[70d](#1709)Also filed from the review: #1710 (a
list_insert_atoverflow, already on main) and #1711 (BUILTIN_*_VIEWuses have no escape check).🤖 Generated with Claude Code
https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF