Skip to content

Bounds-check the BBList API: bad indices fail cleanly instead of via UB#73

Merged
CoreyRDean merged 2 commits into
developfrom
fix/bblist-bounds-checking
May 30, 2026
Merged

Bounds-check the BBList API: bad indices fail cleanly instead of via UB#73
CoreyRDean merged 2 commits into
developfrom
fix/bblist-bounds-checking

Conversation

@CoreyRDean
Copy link
Copy Markdown
Collaborator

Non-technical summary

BBList (the modern dynamic-list type, used heavily by rcce2) trusted whatever index you handed it. Pass an out-of-range or negative index — or call ListFirst/ListLast on an empty list — and the runtime hit undefined behaviour / heap corruption, or in the case of ListAt, killed the entire program with an opaque C++ error. This PR adds bounds checks so bad indices fail cleanly: a clear runtime error in debug builds, a safe no-op otherwise — never memory corruption. Advances INTENT value #6 ("Trust the runtime").

Technical summary

In src/blitzrc/bbruntime/basic.cpp, the _bbVector* functions did no validation:

  • ListInsert/ListRemove/ListReplacebegin()+idx with no range check (invalid iterator → UB).
  • ListFirst/ListLastfront()/back() on a possibly-empty vector (UB).
  • ListAtvec.at(idx), whose std::out_of_range escaped to the top-level bbruntime_run catch and terminated the whole program with an STL string.

Added two inline guards — listIndexOk() and listNotEmpty() — mirroring the established bank/stream idiom (bbbank.cpp debugBank): debug ? RTEX(clean message) : errorLog + safe default. Applied to _bbVectorAt (now [] after the check), _bbVectorBack/_bbVectorFirst, _bbVectorRemove/_bbVectorReplace, and _bbVectorInsert (which correctly allows the append position idx == size and rejects idx > size). Happy-path behaviour is byte-identical. BBList is a BlitzForge-specific API, so there's no legacy-Blitz3D compatibility risk.

Acceptance criteria + results

  • Out-of-range/negative ListAt returns Null and the program keeps running (pre-fix it crashed the whole run) — new testListAtOutOfRange.
  • ListFirst/ListLast on an empty list return Null, no UB — testListFirstLastEmpty.
  • Out-of-range ListRemove/ListReplace are no-ops, size + contents unchanged — testListRemoveReplaceOutOfRange.
  • ListInsert accepts idx == ListSize (append) and rejects idx > ListSize / negative — testListInsertBoundary.
  • All existing ListTest.bb happy-path blocks pass unmodified; full test.bat green; local build 0 errors; corpus sweep unaffected.

Note: the new tests run in -t mode where debug is false, so they exercise the soft-fail (no-op + safe default) path; the blocks completing is the regression guard, since the pre-fix ListAt out-of-range aborted the process. In a -d debug build the same inputs raise a clean Blitz RuntimeError.

Trade-offs, deferred follow-ups

  • Negative-index "count from end" is a feature, not a fix — out of scope.
  • _bbVectorClear arg-count/shadow quirk (basic.cpp:882: rtSym declares one arg, C fn takes two; inner for shadows the unused param) — harmless, pre-existing; flagged, not touched.
  • Deferred (confirmed, next candidates): the seTranslator missing-break fix (bbruntime_dll.cpp — mislabels every native crash; verification path now known) and human-readable compiler diagnostics (source line + caret, sanitize EOF byte).

🤖 Generated with Claude Code

CoreyRDean and others added 2 commits May 29, 2026 20:18
The BBList runtime (_bbVector* in basic.cpp, exposed as ListAt/ListInsert/
ListRemove/ListReplace/ListFirst/ListLast) did no index or emptiness validation
on caller-supplied indices. From ordinary Blitz code this was reachable UB /
memory corruption:
- ListInsert/Remove/Replace computed begin()+idx with no range check -> invalid
  iterator -> UB.
- ListFirst/ListLast called front()/back() on a possibly-empty vector -> UB.
- ListAt called vec.at(idx), whose std::out_of_range escaped to the top-level
  bbruntime_run catch and terminated the WHOLE program with an STL string.

Add listIndexOk()/listNotEmpty() guards mirroring the established bank/stream
debug-vs-errorLog idiom (bbbank.cpp debugBank): in a debug build a bad index
raises a clean Blitz RuntimeError; otherwise it is logged and the call no-ops /
returns a safe default. ListInsert allows the append position (idx == size) and
rejects idx > size. Happy-path behaviour is unchanged.

BBList is a modern BlitzForge API (no legacy-Blitz3D compat risk) and is used
heavily by downstream rcce2. Advances INTENT value #6 (trust the runtime).

tests/ListTest.bb: add bounds/emptiness blocks — out-of-range ListAt returns Null,
Remove/Replace are no-ops with size unchanged, First/Last on an empty list return
Null, and ListInsert at idx==size appends while idx>size no-ops. The blocks
complete (rather than aborting the run), which is the regression guard: pre-fix
the out-of-range ListAt crashed the whole process.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root cause, the bank/stream guard idiom mirrored, why -t exercises the soft-fail
path, and deferred follow-ups (seTranslator break fix; compiler diagnostic snippet).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CoreyRDean CoreyRDean requested a review from a team as a code owner May 30, 2026 01:19
@CoreyRDean CoreyRDean added the bug Something isn't working label May 30, 2026
@CoreyRDean CoreyRDean merged commit 3505dc9 into develop May 30, 2026
4 checks passed
@CoreyRDean CoreyRDean deleted the fix/bblist-bounds-checking branch May 30, 2026 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant