Route container element access through src/container.h (#1665 arm D step 1) by InauguralPhysicist · Pull Request #1701 · InauguralSystems/EigenScript

InauguralPhysicist · 2026-10-10T07:09:54Z

Part of #1665 (arm D) and #1644.

A pure refactor with no layout or behaviour change. Storage stays Value**. Every runtime list element and dict value access now goes through ownership-explicit accessors in src/container.h:

borrowed and owned getters;
owned and borrowed setters;
numeric probes and iteration helpers;
narrow raw-storage helpers for bulk realloc, memmove, qsort and free.

Step 2 then switches the storage to NaN-boxed EigsSlot in one place. That is the change that closes the 94-versus-16.8 bytes-per-element gap against Lua measured on #1644.

Boundary gate

tools/container_access_check.sh (suite [99ze]) fails any raw data.list.items / data.dict.vals access outside container.h. Its verdict line carries the population: examined=26 allowed=26 violations=0. A planted raw access in state.c turns it red.

Allowlisted:

the AST's own dict.vals array (parser, compiler, lint, LSP);
two step-2 sites:
- the JIT's offsetof(Value, data.dict.vals) inline dict probe;
- eigs_bool_gate's argument scan in eigenscript.h, which comes before the container.h include.

Measured

benchmark	before Ir	after Ir	Δ
bench_dmg_shape	1,271,224,709	1,271,224,796	+0.00%
bench_idxset	95,739,425	95,544,531	−0.20%
bench_perf	301,112,416	301,117,550	+0.00%

Gates (run locally)

jit-smoke and jit_diff (256 programs × {jit, osr}) pass.
freestanding-check passes, and the http, gfx, net and zlib builds succeed.
The full release suite first showed 7 failures, all in the new gate pieces:
- the [99ze] section called check_eigs_suite (a runner for .eigs programs) on a shell script, and its check needed rg;
- num_read_check needed allowlist rows for the two accessor numeric probes;
- the child-exit ledger counted those two as untrustworthy child exits.
All are fixed. Sections 0fb and 99ze pass 15/15, and child_exit_check passes.
CI runs the full suite, the ASan shards (leak tally) and TSan.

Step-2 design note (flagged in review)

container.h plans to materialize borrowed numbers into a per-thread scratch Value. Two live borrows at once (for example a[i] + a[j] in a builtin) would alias the same scratch. Step 2 needs owned materialization, or at least two alternating scratch slots with a proof of the maximum number of live borrows.

Review

Built by Codex Cloud. It did not run the gates; I ran them here, fixed the gate pieces, and reviewed the diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF

chatgpt-codex-connector · 2026-10-10T07:10:04Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T07:15:32.730947Z	`a70871e`	PR opened
🔒 Security Review	✅ Completed 2026-10-10T07:18:28.672274Z	`a70871e`	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

github-advanced-security

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

codspeed · 2026-10-10T07:14:34Z

Merging this PR will not alter performance

✅ 13 untouched benchmarks

_{Comparing arm-d-1665 (3b13c5f) with main (9de86d2)}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a70871e60c

ℹ️ 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".

chatgpt-codex-connector · 2026-10-10T07:15:29Z

+        val_incref(list_get_borrow(list, idx));
+        return list_get_borrow(list, idx);


Use an owned getter when returning get_at elements

When the planned slot representation materializes numbers into borrowed scratch storage, this borrow-then-val_incref sequence does not create a stable owned value; the returned pointer can be overwritten by a later getter after the argument list is released. Fetch and return the element once via list_get_owned here and in the analogous two-dimensional path rather than manually retaining a borrow.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-10T07:15:29Z

        return NULL;
    }
-    Value **d = desc->data.list.items + off;
+    Value **d = list_values_storage(desc) + off;


Keep descriptor reads behind element accessors

When list storage changes from Value ** to EigsSlot, treating this semantic element range as Value ** bypasses all decoding and means the conversion cannot be confined to container.h. This is not a bulk allocation, movement, sort, or destruction operation; read desc elements through the indexed accessors instead. The same raw-storage escape should also be audited in the byte-list readers, dict value scans, and VM borrow scan.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-10T07:15:29Z

+    case "$file" in
+        src/parser.c|src/compiler.c|src/lint.c|src/eigenlsp.c|src/lint_host.c)
+            # ASTNode owns its own data.dict.vals array; not a Value container.
+            allowed=$((allowed + 1)); continue ;;


Narrow the AST exemption to AST-specific accesses

Any matching access anywhere in these translation units is accepted before its expression is inspected, although files such as compiler.c and lint_host.c also manipulate runtime Value objects. A future value->data.dict.vals[...] added there therefore produces violations=0, defeating the boundary gate precisely where it is meant to catch a missed step-2 conversion; whitelist the existing AST-shaped expressions rather than entire files.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-10T07:15:29Z

+    list_set_owned(list, list->data.list.count++, item);
    val_incref(item);


Store borrowed append values with the borrowed setter

list_append receives a borrowed item, but this calls the ownership-consuming setter before acquiring the container's reference. The current Value ** implementation masks the contract violation; once the setter encodes immediate numeric slots, it must either consume/release a reference the caller does not own or leak the temporary, and the following val_incref may be too late. Use list_set_borrow, which retains before storing, and remove the trailing increment; the new-dictionary insertion path has the same reversed ownership sequence.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-10T07:15:29Z

+        val_decref(list_get_borrow(list, idx));
+        list_set_owned(list, idx, val);


Release displaced slots without decrefing a borrow

When the destination currently contains an immediate number, the planned list_get_borrow implementation returns scratch storage that explicitly must not be decref'd, so this replacement path will release the scratch object rather than the slot's ownership before writing the new value. Introduce a slot-aware replacement/release operation in container.h and use it here and in the analogous VM, dict, and tensor replacement paths instead of manually decrefing a borrowed getter result.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-10T07:15:29Z


        for (int c = 0; c < cols && c < row->data.list.count; c++) {
-            Value *cell = row->data.list.items[c];
+            Value *cell = list_get_borrow(row, c);


Mutate tensor cells through a slot-aware setter

With immediate numeric container slots, cell is the scratch value produced by the borrowed getter, so the subsequent VAL_NUM_RAW(cell) perturbations modify only scratch storage and never change the matrix passed to numerical_loss. The plus/minus evaluations can consequently become identical and silently produce zero or incorrect gradients; this path needs a numeric slot write accessor that updates the actual container cell.

Useful? React with 👍 / 👎.

…tep 1) Pure refactor, no layout or behaviour change: storage stays Value**. List elements and dict values go through ownership-explicit accessors (borrow/owned getters, owned/borrowed setters, numeric probes, iteration, narrow raw-storage helpers for bulk realloc/memmove/qsort/free) so step 2 can switch the storage to NaN-boxed EigsSlot in one place. tools/container_access_check.sh (suite [99ze]) fails any raw data.list.items / data.dict.vals access outside container.h. Allowlisted: the AST's own dict.vals array, and two step-2 sites (the JIT's offsetof(Value, data.dict.vals) inline dict probe, and eigs_bool_gate's arg scan in eigenscript.h, which precedes the container.h include). Ir before -> after: bench_dmg_shape 1,271,224,709 -> 1,271,224,796 (+0.00%), bench_idxset 95,739,425 -> 95,544,531 (-0.20%), bench_perf 301,112,416 -> 301,117,550 (+0.00%). jit-smoke, jit_diff (256 programs), freestanding-check, http/gfx/net/zlib builds pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF

…es (#1665) The step-1 refactor rewrote list access in 48 exempt builtins (and their helper closures) from arg->data.list.items[i] to the container.h accessors, which changes the source hashes the contract pins for review. The changes are mechanical access rewrites; each exemption's reasoning is unchanged. Re-pinned with the checker's own functions/ exemption_sources; only hash lines change. Candidate discovery keyed on the literal `data.list`; it now also matches the accessors (list_get_*/list_set_*/list_iter_*/list_values_*) so step 2 cannot shrink the audited population silently. Population is unchanged: candidates=190 guarded=139 exempt=51 errors=0; selftest 20/20. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF

github-advanced-security Bot found potential problems Oct 10, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 10, 2026

View reviewed changes

xxxxxxxxxxxxx and others added 2 commits October 10, 2026 03:35

InauguralPhysicist force-pushed the arm-d-1665 branch from a70871e to 3b13c5f Compare October 10, 2026 08:38

github-advanced-security Bot found potential problems Oct 10, 2026

View reviewed changes

InauguralPhysicist added this pull request to the merge queue Oct 10, 2026

InauguralPhysicist mentioned this pull request Oct 10, 2026

Memory oracle has blind spots: freelists, arena, NaN-boxed slots and JIT code are invisible to ASan #1665

Open

6 tasks

Merged via the queue into main with commit 5b6e442 Oct 10, 2026
32 checks passed

InauguralPhysicist deleted the arm-d-1665 branch October 10, 2026 11:53

		val_incref(list_get_borrow(list, idx));
		return list_get_borrow(list, idx);

		list_set_owned(list, list->data.list.count++, item);
		val_incref(item);

		val_decref(list_get_borrow(list, idx));
		list_set_owned(list, idx, val);

Conversation

InauguralPhysicist commented Oct 10, 2026

Boundary gate

Measured

Gates (run locally)

Step-2 design note (flagged in review)

Review

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

github-advanced-security AI left a comment

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading

codspeed Bot commented Oct 10, 2026 •

edited

Loading