Fix: cap absvector size so a huge request is a catchable error, not a heap-exhaustion abort

[pyrex41]

Summary

(absvector HUGE) handed make-array a size far larger than the heap. On SBCL this is an uncatchable Heap exhausted abort -- trap-error cannot recover from it, so a single bad size takes down the whole image:

$ shen eval -e "(trap-error (absvector 100000000000) (lambda E (str E)))"
Heap exhausted during allocation: 1028063232 bytes available, 800000000016 requested.
... (process aborts despite the trap-error wrapper)

Fix

src/primitives.lsp: validate the requested size in |absvector|. A non-negative integer up to a sanity cap of 2^24 (16,777,216) slots allocates exactly as before; anything larger, negative, or non-integer raises a catchable Shen error via simple-error instead of attempting the allocation.

The cap is over 800x the largest vector the kernel itself ever allocates (the 20000-slot property dictionary used by (dict 20000)), so it cannot break any legitimate kernel or program use. It is portable CL and so protects every impl, while the heap-exhaustion abort it prevents is the SBCL failure mode from the issue.

Test

tests/primitives-tests.shen gains issue #3 regressions, including the exact form requested in the issue:

(assert-true "absvector huge size is catchable (issue #3)"
             (trap-error (absvector 100000000000) (lambda E true)))   \\ => true

plus over-cap and negative-size catchability, and a large-but-legal (absvector 1000000) that still allocates.

Results

Rebuilt the SBCL image.

• Canonical SBCL kernel cert (make test-sbcl): 134/134, 100%
• Compiler golden tests (make test-compiler): pass
• Port runtime suite (make test-port): 130/130

Fixes #3

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

[pyrex41]

Independent agent review (ratatoskr + ShenSpec deep-dive, post-load native override discipline, 134/134 cert preserved)

Code Review: PR #5

PR: #5
Title: Fix: cap absvector size so a huge request is a catchable error, not a heap-exhaustion abort
URL: #5
Review file: /tmp/grok-reviews/f84b81e0-REVIEW.md
Reviewer mode: Independent verification agent (PR mode)
Inputs read:

• Unified diff: /tmp/grok-reviews/f84b81e0.diff (RIGHT-side lines cited below)
• Changed files list: /tmp/grok-reviews/f84b81e0.files.txt
• shen-cl/src/primitives.lsp (pre-patch state in workspace; post-patch semantics per diff)
• shen-cl/tests/primitives-tests.shen (pre-patch; additions per diff)
• ratatoskr/README.md (context on kernel structures + ratatoskr role)
• ratatoskr/KLambda/*.kl (full searches for absvector, dict, vector, prolog memory, property-vector, tuples, pvars, etc.; key files: dict.kl, init.kl, sys.kl, macros.kl, prolog.kl, stlib.kl, declarations.kl)
• shen-cl/src/overwrite.lsp, src/native.lsp (allocation patterns, overrides, |vector| vs |absvector|)
• shen-cl/tests/test-harness.shen (assert-caught, trap-error, assert-no-crash semantics)
• shen-cl/AGENT.md, AGENT-more.md, README.md (port conventions)
• shen-cl/src/primitives.lsp definitions for |simple-error|, |trap-error|, |absvector?|, |fail| sentinel logic, |shen-cl.<-address/or| etc.
• Cross-PR context notes on "minimal port change + executable spec" discipline and "no canonical sources edited"

Process followed: Read diff first, then all instructed sources + surrounding allocation/sentinel/error paths. Broad grep for absvector + size constants across ratatoskr/KLambda and shen-cl/src, followed by targeted file reads. Verified 2^24 rationale, predicate tightness, error surfacing, success-path identity, and absence of >16M kernel needs. RIGHT-side diff lines used for citations of proposed code.

---

Summary

Overall verdict: APPROVE (with minor observations). The change is correct, minimal, and directly solves the reported problem (Shen issue #3). A huge (absvector N) (e.g. 1e11 from the issue) previously passed an arbitrarily large size to make-array, producing an uncatchable SBCL "Heap exhausted" abort that trap-error (and thus Shen trap-error) could not recover from, taking down the entire image. The fix adds a portable pre-check in the CL primitives layer.

Correctness (primary):

• Predicate is tight: (and (integerp n) (>= n 0) (<= n |shen-cl.max-absvector-size|)) (diff RIGHT: primitives.lsp:25). Rejects non-integers (floats, strings, etc.), negatives, and oversize before any allocation attempt. Short-circuit and prevents type errors on non-numbers in the comparisons.
• Error path: delegates to |simple-error| (which does (error "~A" string)) for a normal, catchable CL condition. |trap-error| is (handler-case ... (error (condition) ...)) (primitives.lsp:87-88), so Shen-level trap-error + harness assert-caught both work.
• Success path identical to before for valid sizes: same (make-array n :initial-element (|fail|)) (diff RIGHT: primitives.lsp:26). |fail| sentinel (for <-vector "not found") and |absvector?| (arrayp + not stringp) are untouched.
• Edge cases covered: n=0 (legal empty), n=2^24 (max, legal), n=2^24+1 and bignum 1e11 (error), negative int, non-int. Tests in diff exercise exactly these (including the issue's 100000000000 form inside trap-error returning true).

Rationale + scale verification (ratatoskr angle):
Searches + reads of ratatoskr/KLambda confirm the "800x headroom" claim holds against real canonical kernel usage (no canonical KLambda or vendored kernels were edited, per cross-PR notes):

• Largest: shen.dict 20000 in init.kl → absvector (+ 3 V4162) in dict.kl (portable dict impl: ~20003 slots). Property-vector in shen-cl is overridden to native hash-table in overwrite.lsp:208 (|shen.dict|), but the portable KLambda still documents/uses the 20000 figure and the absvector-based dict constructor for other contexts (shaken kernels, ratatoskr builds, other ports).
• Prolog: set shen.*prolog-memory* 1000, then (prolog-memory 10000) in init.kl; shen.prolog-vector in macros.kl does absvector (value shen.*prolog-memory*). (See also declarations.kl:9 and ratatoskr/Primitives/CL/prolog-memory.lsp.)
• Common small: @p/tuple/absvector 3 (sys.kl, t-star.kl, stlib.kl), pvar 2 (prolog.kl), fn-print 2, rational/complex 3/4, programmable-pattern stack 1, vector(N) wrapper does absvector(+ N 1) (sys.kl:28), populate etc. at user sizes (stlib).
• No KLambda site requests anywhere near 2^24 (~16.7M). 16M / 20003 ≈ 839×. The cap is safe for all kernel structures and legitimate programs. (See ratatoskr/README.md for kernel scale and ratatoskr's role in producing minimal portable KL slices.)

Tests / ShenSpec angle (primitives-tests.shen):
The additions (diff RIGHT: tests/primitives-tests.shen:41-54) are placed in the port's executable spec (after absvector out-of-range tests, before logic; loaded via scripts/run-port-tests.shen after test-harness). They explicitly pin the new contract:

• "An excessive size must raise a CATCHABLE Shen error, NOT an uncatchable heap-exhaustion abort" (comment).
• Exact huge form from the issue inside trap-error → true.
• assert-caught (which thaws under trap-error expecting the 'caught symbol; see test-harness.shen:44-47) for over-cap and negative.
• Legal large (absvector 1000000) still succeeds via absvector?.
  This is the "non-hush" safety fix among the six; it strengthens the port spec in a way other ports should consider for parity on the "never abort on bad primitive size" contract. Results claimed (134/134 cert, golden compiler, port suite 130/130) are consistent with adding only guarded paths + positive tests.

Error handling / primitives layer:

• |fail| only on success (post-kernel); error path uses |simple-error| (defined early, primitives.lsp:84-85).
• Surrounding: |<-address| / |address->| (svref, no creation check), |shen-cl.<-address/or| etc. use length; kernel already guards indices for user vectors. Creation is the new choke point.
• Portable across CLs (SBCL was the reported abort case; others may have had OOM instead). No handler-bind / restart complexity needed — simple guard + existing error path suffices.

Other notes (no high-severity findings):

• Diff only touches port product code (src/primitives.lsp + tests/primitives-tests.shen) — ratatoskr/KLambda, vendored kernels (cl-source/, shen-s41.1/, etc.), and canonical sources untouched, matching cross-PR discipline.
• No performance impact on hot path (the guard is cheap integer arithmetic; valid allocations unchanged).
• Constant is defconstant with clear name in shen package style (|shen-cl.max-absvector-size|). Comment block (diff RIGHT: primitives.lsp:9-16) is excellent.
• No unwrap/clone/lock issues (this is CL, no Rust/Go equivalents). No races (absvector allocation is not concurrent in normal Shen model).

Related observation (not a defect in this PR): The user-level |vector| override (overwrite.lsp:314-317) does an analogous unchecked (make-array (1+ n) ...) (bypassing |absvector|). A huge (vector HUGE) could still hit SBCL abort. However: (a) the reported issue + title + tests were narrowly about absvector; (b) in the portable KLambda path, Shen vector goes through kernel vector → absvector(+1), which will now be capped; (c) shen-cl's override is a deliberate native path for the high-level vector abstraction. If consistency is desired in a follow-up, a similar (or higher) cap could be added, or vector sizes could be left uncapped with the understanding that raw absvector is the low-level one being hardened. No evidence of legitimate kernel or common user need for >16M absvector slots.

The fix is a clean, well-tested safety bound that turns a fatal abort into a catchable Shen error while preserving all prior behavior for valid inputs. 134/134 + port suite green is the expected outcome.

---

Issues

Severity: Low (documentation/scope observation, not a correctness defect)
File:line: shen-cl/src/overwrite.lsp:314 (post-PR context; related allocation site; primitives change itself is at src/primitives.lsp:23-29 per diff RIGHT)
Description: |vector| (the Shen high-level vector constructor) performs an unchecked make-array of size (1+ n). While absvector (the low-level primitive used by kernel for tuples/@p/pvar/dict/prolog/freshterm/etc. and directly by users) is now guarded, the vector path is not. A crafted (vector 100000000000) would still attempt a huge allocation and could produce the uncatchable heap-exhaustion abort on SBCL (or equivalent on other CLs). In portable KLambda (sys.kl:28), vector(N) does (absvector (+ V3779 1)), so the cap would be hit there on non-shen-cl ports; shen-cl's override bypasses it for speed/idiom. The PR title, body, and new tests correctly target only absvector (the form from issue #3). No kernel code (ratatoskr searches) ever requests sizes near the cap via either path.
Suggestion: No action required for this PR (scope is absvector; "all PRs touch only their port's product code"). Consider a follow-up or note in INTEROP.md/README if hardening the native |vector| path (or documenting that raw absvector is the hardened one) is desired for full symmetry. The 2^24 cap (or a separate larger one) would be equally safe here given the verified headroom.
Status: open (observation only; does not affect approval of the absvector fix)

Severity: Info (style / future-proofing)
File:line: src/primitives.lsp:17 (diff RIGHT side; the new constant definition)
Description: |shen-cl.max-absvector-size| is a compile-time defconstant with no provision for runtime override or per-image tuning (e.g. via a global that defaults to 2^24). This is fine and matches the "portable CL, protects every impl" goal, but on very memory-constrained CL images a smaller effective cap might be preferable, or on 64-bit systems with huge heaps a user might legitimately want >16M for application data structures built on raw absvector. (Note: absvector is intentionally low-level; most users should prefer vector, dict, or lists.) Kernel max remains ~20k even in portable dict form.
Suggestion: Leave as-is for this safety fix. If needed later, the constant + guard can be made dynamic (e.g. defvar with a setter that only increases) without changing the error-message or predicate shape. Document the cap value and rationale (already well done in the added comment block) so other ports can adopt analogous limits.
Status: closed (no change needed; design choice is appropriate)

Severity: Info (test completeness / edge)
File:line: tests/primitives-tests.shen:53 (diff RIGHT side; the "large-but-legal" test)
Description: The regression uses (absvector 1000000) + absvector? check. This is < 2^24 and exercises the success path + predicate >=0 && <= max. Good, but does not explicitly test the exact boundary (absvector |shen-cl.max-absvector-size|) succeeding or (1+ max) failing (beyond the huge 1e11 example). Negative zero or other numeric edge forms are irrelevant in practice. The harness assert-caught + direct trap-error forms are both used, which is strong.
Suggestion: Optional: add two more one-liners for the boundary if the executable spec wants to lock it more tightly in future (e.g. (assert-true "absvector at exact cap" (absvector? (absvector 16777216))) and a caught form for 16777217). Not required for this PR; the existing cases (including the literal huge from the issue report) are sufficient to prevent regression of the abort behavior.
Status: closed (tests are adequate and targeted)

Severity: None (positive finding; no issue)
File:line: src/primitives.lsp:25 (diff RIGHT) + tests/primitives-tests.shen:45 (diff RIGHT)
Description: Predicate + error surfacing + test coverage for the "must be catchable" contract is excellent. The exact form from the issue (trap-error (absvector 100000000000) (lambda E true)) is asserted true; assert-caught (freeze + thaw under trap) is used for the over/under cases; legal large allocation remains possible. This strengthens primitives-tests.shen as an executable ShenSpec for the port (other ports should consider equivalent). No gaps in the checked conditions (integerp + range) or in |simple-error| / |trap-error| integration.
Suggestion: N/A — keep.
Status: N/A (commendation)

Severity: None (positive; verification)
File:line: ratatoskr/KLambda/dict.kl:1, init.kl: (initialise-environment and prolog-memory 10000), sys.kl:28, macros.kl:56, and multiple stlib/t-star/prolog sites (all via grep absvector)
Description: Confirmed zero legitimate kernel allocations approach the cap. The 20000-slot dict figure cited in the PR comment (and 10000 prolog) is accurate to the sources; the +3/ +1 wrappers do not change the 800x+ math. shen-cl-specific overrides (overwrite.lsp for dict and vector) were also reviewed; they do not invalidate the cap rationale for the remaining absvector call sites (tuples, pvars, prolog vectors in macros, stlib rationals, etc.).
Suggestion: N/A.
Status: N/A (rationale holds)

No other issues found (no error-handling gaps, no race conditions, no incorrect sentinel logic, no changes to |absvector?| or address ops, no impact on certification paths or ratatoskr builders). The patch is ready to land.

---

End of review. Review artifacts written to the path above. The change meets the "correctness first" bar with clear, testable hardening of a foot-gun primitive while preserving the success path and adding executable-spec coverage.

---

Posted automatically by Grok reviewer subagents. See also the PENDING review (if any) for inline comments on the Files tab.

[pyrex41]

From independent reviewer subagent (ratatoskr/KLambda scale verification for the 800x claim, primitives layer, test-harness integration, and portable KL vs. native override paths):

Low (documentation/scope observation)

• File: shen-cl/src/overwrite.lsp:314 (related |vector| site; the primitives change is at src/primitives.lsp)
• Description: |vector| (high-level) does an unchecked (make-array (1+ n) ...). A huge (vector HUGE) could still abort on SBCL. The reported issue + tests targeted raw absvector. In portable KLambda, vector goes through absvector(+1) which is now guarded.
• Suggestion: No action for this PR (scope was absvector). Consider follow-up or note in docs for symmetry if desired. The cap (or a separate larger one) would be safe here too.

Info (style / future-proofing)

• File: src/primitives.lsp:17 (the new defconstant)
• Description: |shen-cl.max-absvector-size| is a compile-time defconstant with no runtime override. Fine for the safety goal, but on very constrained images or for apps wanting >16M raw absvectors a user might want tuning.
• Suggestion: Leave as-is for this fix. If needed later, can be made dynamic without changing predicate/error shape. Document the value (already well done in the comment block).

Info (test completeness / edge)

• File: tests/primitives-tests.shen:53
• Description: Uses (absvector 1000000) + absvector? for the large-but-legal case. Good, but does not explicitly test the exact boundary (max succeeds, max+1 fails) beyond the huge 1e11 example.
• Suggestion: Optional one-liners for the exact cap value if the spec wants to lock the boundary tighter later.

Strong positives: Predicate tight (integerp + range, short-circuit and), error via |simple-error| (catchable), success path identical for valid sizes, |fail| sentinel untouched, ratatoskr/KLambda searches confirm max kernel usage ~20k (dict/prolog etc.), tests pin the exact contract from the issue using both trap-error and assert-caught. No changes to |absvector?| or address ops. Full ratatoskr verification in the previous top-level comment. APPROVE per subagent (with minor observations).

[pyrex41]

Superseded by #6 (merged into kernel-41.2), which combines the absvector cap (#3) and the CLISP/ECL hush fix (#2) in one branch. Closing as redundant.