andrew/libakerror
1
0
Fork 0
Code Issues 14 Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

Move outstanding work from TODO.md into the issue tracker
Some checks failed
libakerror CI Build / cmake_build (push) Has been cancelled
Details
libakerror CI Build / coverage (push) Has been cancelled
Details
libakerror CI Build / thread_sanitizer (push) Has been cancelled
Details
libakerror CI Build / mutation_test (push) Has been cancelled
Details

Browse Source 
Fourteen issues on source.starfort.tech/andrew/libakerror, labelled by kind and
blast radius and milestoned by what they can land in: 2.0.x for anything that
breaks no ABI, 2.1.0 for additive surface, 3.0.0 for the handler-ladder rewrite.
Everything filed carries status::grooming.

Three of the fourteen were not in this file at all. They are defects in this
library that were written down in a consumer's TODO instead: IGNORE() leaking a
context (#14), the coverage target not being namespaced when embedded (#15),
and no akerrorConfigVersion.cmake being installed (#16). libakstdlib carries a
workaround for each. That is the finding worth keeping -- a library whose
consumers record its defects in their own files cannot see them, and the
workaround gets written once per consumer.

TODO.md keeps the reasoning: why the handler ladder is a major-version change,
why a copied akerr_ErrorContext is a trap in two specific fields, why
validating more inputs lowers branch coverage by construction, why the mutation
score is a floor, and why a registered name is returned by pointer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tachikoma
2026-08-02 19:00:22 -04:00
parent 5695061130
commit a932b976f7
3 changed files with 89 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
README.md
View File

@@ -33,7 +33,7 @@ This library has 6 guiding principles:
| [docs/thread-safety.md](docs/thread-safety.md) | What thread safety here covers, what it does not, and how to hand an error to another thread |
| [docs/building.md](docs/building.md) | Configure options, the generated header, and building without stdlib |
| [UPGRADING.md](UPGRADING.md) | What changed in 1.0.0, 2.0.0 and 2.0.1, and how to migrate |
| [TODO.md](TODO.md) | Known defects, ordered by blast radius |
| [TODO.md](TODO.md) | The reasoning behind decisions and measurements. **Outstanding work is in [the issue tracker](https://source.starfort.tech/andrew/libakerror/issues).** |
# Installation

230
TODO.md
View File

@@ -1,161 +1,103 @@
# TODO
# Record
Working notes for `libakerror`. Outstanding items only.
**Outstanding work is in the issue tracker, not in this file:**
<https://source.starfort.tech/andrew/libakerror/issues>
## 1. Only ThreadSanitizer is wired into CI, not ASan/UBSan
Issues are labelled by kind and blast radius, and milestoned by what they can land
in: `2.0.x` for anything that breaks no ABI, `2.1.0` for additive surface, `3.0.0`
for the handler-ladder rewrite and the contract changes that go with it. Everything
filed carries `status::grooming` until it has been through grooming.
`AKERR_SANITIZE` builds the library and the tests with any sanitizer list, and
CI runs `-DAKERR_SANITIZE=thread` through `scripts/thread_test.sh`. Nothing runs
`address,undefined` yet, and that is the one that covers the original
motivation: mutation testing caught an out-of-bounds probe in the status-name
hash table that the suite could not, because the failure mode was a write into
adjacent BSS, which does not crash. Sharpening one test closed that instance;
ASan would catch the whole class regardless of how sharp the assertions are.
What stays here is the reasoning a tracker has no place for.
The machinery is in place — this is one more job in
`.gitea/workflows/ci.yaml` running
`cmake -S . -B build/asan -DAKERR_SANITIZE=address,undefined`. Left separate
because ASan and TSan cannot be combined in one build.
## Three defects were recorded only downstream
## 2. `HANDLE`-level status aliasing is still undetectable
Found while moving this file and `libakstdlib`'s into their trackers. Each is a
defect **in this library**, each was written down in a *consumer's* TODO file, and
none had an entry here:
Two components can compile the same integer into a `case` label without ever
reserving a range or registering a name, and nothing sees it. Ownership
enforcement covers *naming*, which is the part the library mediates; the `case`
label never reaches it.
| Issue | What it is | What it costs downstream |
|---|---|---|
| #14 | `IGNORE()` logs a context and never releases it | `libakstdlib`'s `aksl_tree_iterate` open-codes log-then-release by hand; every `IGNORE()` in `libakgl`'s `CLEANUP` blocks is a leaked slot |
| #15 | The `coverage` target is not namespaced when embedded, though `mutation` is | `libakstdlib` shadows `add_custom_target` to embed this library at all |
| #16 | No `akerrorConfigVersion.cmake` is installed | No consumer can ask `find_dependency(akerror)` for a version floor |
Closing this needs the `if`/`else if` handler ladder — rewriting
`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` so status matching
is not restricted to integer constant expressions. That would also allow
matching on ranges or predicates, and would let a handler resolve a code through
its owner. It touches the most load-bearing code in the library and every
consumer's error handling at once, so it wants its own change.
**That is the finding worth keeping, more than the three defects.** A library whose
consumers record its defects in their own files has no way to see them: each
consumer knows one, nobody sees the pattern, and the workaround gets written three
times. The tracker is now the place, and a consumer filing upstream costs them one
issue instead of one workaround.
Note it would *not* by itself fix the "don't use `CATCH` or `FAIL_*_BREAK`
inside a loop" hazard: that comes from exiting via `break`, not from `switch`.
## Why the handler ladder is a `3.0.0` change and not a patch
## 3. No registry introspection
`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` compile to a `switch`,
which restricts status matching to integer constant expressions. That is what makes
`HANDLE`-level aliasing undetectable (#4): two components can compile the same
integer into a `case` label without reserving a range or registering a name, and
**ownership enforcement never sees it, because it covers naming and the `case` label
never reaches the library.**
There is no way to ask who owns a status, or to enumerate reservations. The
"coordinate ranges at the dependency-stack level" advice in UPGRADING.md
therefore has no tooling behind it.
Rewriting it as an `if`/`else if` ladder would allow matching on ranges or
predicates and let a handler resolve a code through its owner. It also touches **the
most load-bearing code in the library and every consumer's error handling at once.**
A read-only accessor plus a dump through `akerr_log_method` would let a startup
self-check or a CI job print the whole map for a linked stack. Cheap, additive,
and the natural next step for multi-component adoption.
One thing it would *not* fix, recorded so nobody expects it to: the "don't use
`CATCH` or `FAIL_*_BREAK` inside a loop" hazard comes from exiting via `break`, not
from `switch`.
## 4. No way to release a reservation
## Why a copied `akerr_ErrorContext` is a trap
A plugin host that `dlopen`s many distinct plugins over a process lifetime
accumulates ranges until the table fills. Reloading the *same* plugin is fine —
an identical repeat by the same owner is idempotent.
Recorded because the struct looks copyable and is not, and #11 exists only if a
consumer ever needs it.
## 5. Renaming a status is not safe against a concurrent lookup
- **`stacktracebufptr` is self-referential.** `akerr_ErrorContext c = *src;` leaves
the copy's cursor pointing into the source's buffer, so the copy logs correctly
and then **corrupts a slot it does not own** the first time anything appends.
- **`arrayid` is restored after the wipe** in `akerr_release_error()`
(`src/error.c:395-398`), so a copied id makes the destination **impersonate the
source's slot for the life of the process.**
If it is ever built, the shape takes the destination as a parameter rather than
allocating it: an allocating copy could fail on pool exhaustion, and **reporting
that failure needs a pool slot**, so it would have to abort — a third `exit()` site
in a library that deliberately has two.
## Why validating more inputs lowers branch coverage
Branch coverage on `src/error.c` sits just above its 50% gate, and the gate is set
where it is on purpose.
Every `FAIL_*` site carries about **six** branch outcomes of error-construction
machinery (`ENSURE_ERROR_READY`, `AKERR_STACKTRACE_APPEND`) that only run when that
specific failure fires. **Every `PASS` site around a call that cannot fail carries
about twenty-five.**
So adding a defensive check lowers the ratio by construction. **Before adding one,
expect to add a test that drives it**, as `tests/err_copy_string.c` does.
## Why the mutation score is a floor
`scripts/mutation_test.py` configures each mutant with the default CMake options,
so **a mutant that only breaks under concurrency is judged by a suite running
without ThreadSanitizer.**
Measured: deleting the pool's `akerr_mutex_lock()` call survives the run, **even
though it is a real race.** Rebuilt and run directly, that mutant fails
`tests/err_threads_pool.c` in 4 of 10 runs, and fails under `scripts/thread_test.sh`
in 5 of 5.
**81.2% is therefore a floor for that category, not a verdict.** #10 is the
passthrough that would fix the measurement, and it belongs behind a flag: a
TSan-instrumented suite per mutant costs roughly 6s instead of 0.4s.
## Why a registered name is returned by pointer
`akerr_name_for_status(status, NULL)` returns a pointer into the registry rather
than a copy, which is what makes it usable from inside `FAIL` — it needs no
buffer and no error context of its own. Registering a *second* name for a status
that already has one (`tests/err_name_ownership.c` covers that it is allowed)
overwrites that buffer in place, so a thread reading the name at that moment can
see a torn string. Every other registry operation is serialized; this one cannot
be, because the reader is outside the lock by the time it reads the characters.
than a copy, **which is what makes it usable from inside `FAIL`** — it needs no
buffer and no error context of its own.
Documented in docs/thread-safety.md and UPGRADING.md as "register names during
initialization". Closing it properly means making a registered name immutable —
either refusing a rename outright (a behavior change, and
`tests/err_name_ownership.c` asserts the current contract), or copying names
into a bump-allocated arena and publishing the pointer with a release store, so
a rename allocates new storage instead of rewriting live storage. The arena is
the better answer; it costs a second capacity limit and its exhaustion path.
## 6. Deprecate the two-argument name-registration path
`akerr_name_for_status(status, name)` cannot identify its caller, so it can only
check that *some* reservation covers the status, not that the caller owns it. It
exists for migration. Once consumers have moved to
`akerr_register_status_name()`, make the set path a no-op or remove it and leave
`akerr_name_for_status()` as pure lookup.
## 7. `akerr_init()`'s own reservation failure is untested
`tests/err_library_status_fatal.c` covers the terminal path in
`__akerr_name_library_status()` by naming a status the library does not own. The
band reservation in `akerr_init()` has no such handle: it can only fail in a
build whose tables are too small for the library's own entries, and both sizes
are `PRIVATE` to the library target, so a test executable cannot set them.
Closing it means a second library target built with tiny tables plus a
`WILL_FAIL` test linked against it. Nothing in the CMake does that yet: the
sanitizer and coverage options vary the *flags* of the one library target, not
its compile definitions.
Related: branch coverage on `src/error.c` now sits just above its 50% gate.
Every `FAIL_*` site carries about six branch outcomes of error-construction
machinery (`ENSURE_ERROR_READY`, `AKERR_STACKTRACE_APPEND`) that only run when
that specific failure fires, and every `PASS` site around a call that cannot
fail carries about twenty-five. Validating more inputs therefore lowers the
ratio by construction. Before adding defensive checks, expect to add a test that
drives them, as `tests/err_copy_string.c` does.
## 8. Mutation testing judges concurrency mutants without a sanitizer
`scripts/mutation_test.py` configures each mutant build with the default CMake
options, so a mutant that only breaks under concurrency is judged by a suite
running without ThreadSanitizer. Deleting the pool's `akerr_mutex_lock()` call
survives the run even though it is a real race: rebuilt and run directly, that
mutant fails `tests/err_threads_pool.c` in 4 of 10 runs, and fails under
`scripts/thread_test.sh` in 5 of 5. So 81.2% is a floor for that category, not a
verdict.
Closing it means a `--cmake-arg` passthrough on the harness so the mutant build
can be configured with `-DAKERR_SANITIZE=thread`. The whole run then costs a
TSan-instrumented suite per mutant (roughly 6s instead of 0.4s), so it belongs
behind a flag rather than in the default target or in CI.
## 9. No way to keep an error context and report it at the same time
A context can be handed to another thread and released there --
`docs/thread-safety.md` now documents that pattern and
`tests/err_threads_handoff.c` proves it -- but it is a *move*. A thread that
wants to both keep its error and report it upward has to read the fields out
into its own record, and it loses the stack trace doing so, because
`stacktracebuf` is the one thing that cannot be usefully summarized.
Copying the struct is not a workaround. `stacktracebufptr` is self-referential
(`include/akerror.tmpl.h`), so `akerr_ErrorContext c = *src;` leaves the copy's
cursor pointing into the source's buffer -- the copy logs correctly and then
corrupts a slot it does not own the first time anything appends to it. `arrayid`
is restored after the wipe in `akerr_release_error()` (`src/error.c:395-398`), so
a copied id makes the destination impersonate the source's slot for the life of
the process.
If this is ever worth an API, the shape is:
```c
akerr_ErrorContext AKERR_NOIGNORE *akerr_copy_error(akerr_ErrorContext *source,
akerr_ErrorContext *destination);
```
taking the destination as a parameter rather than allocating it. An allocating
copy could fail on pool exhaustion, and reporting *that* failure needs a pool
slot, so it would have to abort -- adding a third `exit()` site to a library that
deliberately has two. Caller-allocates puts the pool pressure where it can be
managed. The copy must repair four fields: `arrayid` (the destination's own),
`refcount` (set to 1, never inherited), `handled` (false -- a copy is a fresh
obligation, or `FINISH_NORETURN` on the receiving side drops it silently), and
`stacktracebufptr` (re-anchored to the destination's buffer at the *same offset*,
so a later append continues the trace instead of overwriting it).
Not worth building yet: no consumer needs it. The trigger is a consumer that
needs a worker's stack *trace*, not just its status and message, at the join
point -- `libakstdlib`'s planned `pthread_*` wrappers are the likely first.
## Unrelated pre-existing issues
- The `AKERR_USE_STDLIB=OFF` build does not compile at all: `bool`, `PATH_MAX`
and `NULL` are used unconditionally but only included under the stdlib branch.
`docs/building.md`'s dependency list states what a replacement must provide,
but the header still needs its includes untangled for that configuration to
work.
- `CMakeLists.txt` sets `main_lib_dest` from `MY_LIBRARY_VERSION`, which is never
defined and never read. Dead line.
The cost is that renaming a status is not safe against a concurrent lookup (#7):
every other registry operation is serialized, and this one cannot be, because **the
reader is outside the lock by the time it reads the characters.** Documented in
`docs/thread-safety.md` and `UPGRADING.md` as "register names during
initialization".

5
tests/MUTATION.md
View File

@@ -149,9 +149,8 @@ Findings surfaced by mutation testing:
* **Open:** the harness builds every mutant with the default CMake options, so a
mutant that only breaks under concurrency is judged by a suite running without
ThreadSanitizer. Mutating under `-DAKERR_SANITIZE=thread` would close that,
and needs a way to pass CMake options through to the mutant build. See
"Mutation testing judges concurrency mutants without a sanitizer" in
`TODO.md`.
and needs a way to pass CMake options through to the mutant build. That is
issue #10; `TODO.md` records why the score is a floor rather than a verdict.
* **Superseded:** status names now use a private sparse registry, so the old
public `AKERR_MAX_ERR_VALUE` ceiling and its consumer ABI mismatch no longer