Rongsong Shen
d26ff8870d
The buglog was one 27K-line JSON file whose fields covered only symptom,
root cause and fix, as free text: 2% of entries named the code version a
bug was found on, 12% the build profile, 36% a way to reproduce, and 58% of
the file was OpenWolf hook auto-detections ("Missing guard clause",
"Function not marked async" on .md files).
- .claude/skills/buglog/bug-record.md: the record standard. Eight
sections (Summary as the title, Description, Environment, Reproduction;
Root cause, Fix, Verification, References once fixed), each with the
content it must contain -- e.g. Environment needs the OS, the libc the
failing binary loads (from its ELF interpreter: glibc 2.42 in the nix
shell vs 2.40 on the host), toolchain, build profile and commit.
- .wolf/bugs/<id>.md: 500 records migrated from buglog.json; every field
verified to round-trip. They carry legacy: true and an explicit "not
recorded" where the old log had no such field; component derived from
file paths, severity from keywords (else unknown), fix_commits from
hashes in status_note/fix that git resolves (91 of 463 fixed).
- .wolf/buglog-archive.json: the 710 hook auto-detections, verbatim.
- .wolf/buglog.json is now an index generated from the records, keeping
the fields OpenWolf's hooks and `openwolf bug search` read.
- scripts/buglog.py rewritten: new (template + Environment collected from
the machine and --binary), check, status (refuses a status the record
doesn't support; fixed needs Root cause, Fix, Verification and
--commit hashes git resolves), note, recur, set, show, list, triage,
env, index. 29 unittest cases in scripts/tests/test_buglog.py.
- scripts/git-hooks/pre-commit runs `check --staged`: incomplete staged
records, a legacy record hand-changed to fixed without the fix-time
sections, a new record claiming legacy: true, or a stale index refuse
the commit.
- scripts/gitea-sync.py reads the records: issue body = the record's
sections plus linked fix commits (origin remote, GITEA_COMMIT_REPO to
override), and a body hash so an issue's text is refreshed when the
record changes.
- bug-1866 rewritten to the full standard as the worked example.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
38 lines
5.1 KiB
Markdown
38 lines
5.1 KiB
Markdown
---
|
|
id: "bug-1824"
|
|
status: "open"
|
|
status_updated: "2026-09-24"
|
|
status_note: "repl_info.c bounds check uses zarray count instead of caps"
|
|
component: "scheduler"
|
|
severity: "crash"
|
|
timestamp: "2026-09-17T02:45:00Z"
|
|
last_seen: "2026-09-19T09:00:00Z"
|
|
occurrences: 1
|
|
files: ["src/scheduler/src/repl_info.c"]
|
|
tags: ["scheduler", "repl", "zarray", "bounds-check", "out-of-bounds-read", "latent", "discovered-during-scheduler-sim-diagnostics"]
|
|
related: []
|
|
legacy: true
|
|
---
|
|
|
|
# bug-1824: out-of-scope latent bug discovered while implementing scheduler-sim-diagnostics task 3.1 (new…
|
|
|
|
## Description
|
|
NOT A CRASH -- pre-existing, out-of-scope latent bug discovered while implementing scheduler-sim-diagnostics task 3.1 (new repl_info_limit() live-usage display). repl_info_resource_one() (src/scheduler/src/repl_info.c ~line 996) bounds-checks a resource's zarray index via `(r->idx < 0) || (r->idx >= sim->core.sharedInst.count)` before calling zarray_get() -- but zarray_t.count tracks the number of POPULATED slots (incremented only when a previously-NULL slot becomes non-NULL via zarray_put_ex/zarray_store), not the array's allocated capacity (.caps, grown by zarray_ensure_items). A sparsely-populated zarray (e.g. only a high index ever zarray_put() -- realistic if resources are registered out of index order, or an index is skipped) can have count=1 while a LOWER, unpopulated index is itself < count and would incorrectly pass this check and be read as though populated, or a HIGHER populated index can exceed .count and be incorrectly rejected as absent even though zarray_get()'s own .caps-based check would have safely returned it (or correctly returned NULL for a genuinely-unpopulated-but-in-capacity slot, since zarray_ensure_items() NULL-initializes newly-grown slots). Net effect: repl_info_resource_one() can report a resource as having no value ('value=(none)') when a real, populated value exists at that index but the array is sparse, OR (less likely) skip a real bounds-safety net zarray_get_ex() would otherwise provide.
|
|
|
|
## Environment
|
|
_Not recorded (migrated from buglog.json)._
|
|
|
|
## Reproduction
|
|
_Not recorded (migrated from buglog.json)._
|
|
|
|
## Root cause
|
|
repl_info_resource_one()'s pre-check compares r->idx against sim->core.sharedInst.count (populated-slot count) instead of relying on zarray_get_ex()'s own bounds check, which is against .caps (allocated capacity) -- these two counters diverge for any sparsely-populated zarray. Confirmed by direct reading of src/lib/zarray.c: zarray_get_ex() (~line 341-362) checks only `idx >= array->caps` before dereferencing array->data[idx]; zarray_put_ex()/zarray_store() are the only writers of .count, incrementing it exclusively when filling a previously-NULL slot -- so .count is a 'high-water mark of contiguous-from-zero fill,' not a true capacity bound. Separately, and more seriously: neither the .count-based check NOR zarray_get_ex()'s own .caps check safely rejects a NEGATIVE idx -- `idx >= caps` is false for any negative idx when caps >= 0, so a negative idx falls through to `array->data[idx]`, a genuine out-of-bounds read. A negative idx is reachable in this codebase: add_vres() (src/lib/xcelimit.c ~407-435) unconditionally sets a newly-allocated xce_resource_t's ->idx to -1 before key_add() runs, only overwriting it with the real slot index on success -- a caller that reaches a resource-value read before/without that overwrite succeeding would pass a negative idx through. repl_info_resource_one()'s existing `r->idx < 0` half of its check does happen to guard this specific negative case correctly (by luck of ordering, not by understanding zarray's real contract) -- the count-vs-caps confusion is the part that is a genuine, distinct latent bug.
|
|
|
|
## Fix
|
|
NOT FIXED -- discovered and characterized during scheduler-sim-diagnostics task 3.1 (limit-usage display), out of that task's scope (task 3.1 only touches its OWN new code path, repl_print_limit_usage(), which was written from scratch to avoid this exact mistake: it omits the imprecise .count pre-check entirely, relies on zarray_get_ex()'s own .caps bound for the upper end, and adds an explicit idx<0 guard of its own -- verified correct and reviewed independently, see this session's git history for scheduler-sim-diagnostics task 3.1). repl_info_resource_one() itself was deliberately left untouched to keep task 3.1 in its declared boundary. Recommended fix when picked up: replace the `r->idx >= sim->core.sharedInst.count` half of the pre-check with reliance on zarray_get()'s own .caps-based bound (mirroring task 3.1's new repl_print_limit_usage() pattern), keeping the `r->idx < 0` guard. Likely benign in practice today only because resource registration order in this codebase's current production paths tends to be dense/contiguous from index 0, so .count and .caps rarely diverge for sim->core.sharedInst specifically -- but this is an incidental property of current callers, not a guarantee, and worth a regression test with a deliberately sparse fixture (zarray_put() at a high index with no lower indices ever populated) before relying on it.
|
|
|
|
## Verification
|
|
_Not recorded (migrated from buglog.json)._
|
|
|
|
## References
|
|
_Not recorded (migrated from buglog.json)._
|