Skip to content

Validate cc_dynarray indices and keep reads side-effect free - #816

Open
Dikluwe wants to merge 3 commits into
coin3d:masterfrom
Dikluwe:codex/pr/cc-dynarray-bounds
Open

Dikluwe wants to merge 3 commits into
coin3d:masterfrom
Dikluwe:codex/pr/cc-dynarray-bounds

Conversation

@Dikluwe

@Dikluwe Dikluwe commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

get() uses SbPList::operator[], which can expand a const array during a read. Unsigned indices outside the backend's signed domain also reach unchecked list operations. Removing a missing item can forward an invalid index.

Change

Check indices before converting to the backend's int domain. Use a non-growing lookup; invalid reads return NULL. Invalid mutations and removal of a missing item are no-ops, insertion accepts the end position, and truncation only shrinks. Document the contract in the internal header. On Windows shared builds, compile the private dynarray implementation directly into CoinTests, following the existing dictionary support pattern; its symbols are not exported from the DLL.

Validation

  • 3 embedded tests, 21 checks: non-growing reads, UINT_MAX, invalid mutations, empty arrays, insertion at the end/middle, valid replacement, ordered/fast removal and truncation.
  • Rebuilt on the current master: 85/85 CTests passed.
  • Separate integration lab: 125/125 CTests passed.

Integration and platform coverage

All nine reviewed candidates were tested together in a separate integration worktree: 129/129 CTests, 9 embedded tests/92 checks, and the dedicated regression executables. Candidate builds used Ninja/Debug, a shared library, tests and thread safety enabled, with the full suite under Xvfb/GLX and software rendering. Builds were kept in RAM.

Native Windows was not run locally; platform CI remains part of the PR validation.

Publication base: ca247c1321bce07199ec0f034493977761b3218e.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant