Make SHOW_MEMORY builds work: fix deferred node capacity assert and eraseTree use-after-free #91

Merged

andrew merged 4 commits from weaselbot/conflict-set:weaselbot/issue-88 into main 2026-10-27 19:32:44 +00:00

Conversation 2 Commits 4 Files Changed 4

+252 -15

4 Commits

Author	SHA1	Message	Date
weaselbot	80f084cc43	Compile eraseTree's search path length bookkeeping out of non-SHOW_MEMORY builds ... `eraseTree` only carries the search path lengths of queued nodes around so that the SHOW_MEMORY key accounting doesn't have to walk `parent` pointers of nodes that have already been released. Put the lengths on a parallel stack that only exists when SHOW_MEMORY is enabled, so that other builds keep the upstream worklist of bare `Node *` and don't compute anything extra.	2026-10-27 12:56:55 -04:00
weaselbot	c0bd10effa	Drop redundant coverage exclusion markers in the excluded tail region ... `getNodeSize`'s unreachable `default:` sits inside ConflictSet.cpp's `GCOVR_EXCL_START`/`GCOVR_EXCL_STOP` block, so the `GCOVR_EXCL_LINE` markers on those two lines are redundant: they cannot change the report, but they make gcovr print (WARNING) GCOVR_EXCL_LINE found on line 6002 in excluded region started on line 5535 once per line for each of the four reports the coverage job writes. Drop them, matching the rest of the excluded tail, which carries no exclusion markers. Context: the previous coverage job for this branch did not fail in any of its steps. Build, Test (5754/5754 passed), Coverage report (`gcovr --fail-under-line 100` reported 100% for ConflictSet.cpp, LongestCommonPrefix.h and Metrics.h), the upload and the test summary all reported success; the job was marked failed while tearing the job container down. The post-actions (actions/cache, actions/checkout) could no longer `docker exec` into the container and the runner's rootless podman socket was gone ("Cannot connect to the Docker daemon at unix:///run/user/992/podman/podman.sock", "Error while stop job container"); the actions/cache service on 192.168.0.110:41379 had been refusing connections for the whole run. Verified locally on this commit: Debug, gcc and `--coverage` builds of the coverage job's configuration; 5754/5754 tests pass; merged line coverage (union over the library, SHOW_MEMORY and fuzz-driver objects, honouring the GCOVR_EXCL_START/STOP regions) is 100% for all three measured files, and no GCOVR_EXCL_LINE markers remain inside an excluded region.	2026-10-26 21:47:41 -04:00
weaselbot	df986810ff	Give the SHOW_MEMORY instrumentation a single definition per function ... The coverage job compiles ConflictSet.cpp twice: once without SHOW_MEMORY, like the rest of the build, and once with it for the SHOW_MEMORY test. Each config previously saw a *different* definition of the accounting helpers: no-op `constexpr` stubs near the top of the file when SHOW_MEMORY was off, and the real definitions at the bottom when it was on. gcovr reads the .gcda files for a source file and merges them, and its default (strict) function merge mode asserts that a function name always maps to one line. Since `addNode`, `removeNode`, `addKey`, both `removeKey` overloads and `getSearchPathLength` were reported on two different lines, report generation aborted with AssertionError: Got function getSearchPathLength(Node*) on multiple lines: 99, 6002. Define those functions in exactly one place instead, and compile the accounting out from within the bodies. Then every configuration reports each function on the same line and the coverage data can be merged. The empty calls are inlined away: the Release shared library contains no calls to them. With the merge fixed, the accounting lines that only exist in the SHOW_MEMORY build show up in the report, so the coverage job's 100% line requirement covers them. Extend the SHOW_MEMORY test to grow a node through every node type (so `getNodeSize` covers every case) and to erase entries one at a time via `setOldestVersion` (so `removeKey` and the `internal_setOldestVersion` cross-check are covered). The remaining `abort()`s in the accounting are defensive checks that can never be taken on a passing run; mark them GCOVR_EXCL_LINE like the rest of the unreachable code in this file.	2026-10-26 20:34:45 -04:00
weaselbot	bba328f79f	Fix SHOW_MEMORY crashes on valid input ... `Node::getCapacity()` asserted `!releaseDeferred`, but releasing a deferred node requires its capacity, so any node replacement aborted an assertion- enabled `SHOW_MEMORY` build. `partialKeyCapacity` is not repurposed by `deferRelease`, so there is no reason to reject deferred nodes. Drop the assertion. `eraseTree` released a node before visiting its children, so the `parent` pointers walked by `removeKey` pointed to freed memory (and `deferRelease` repurposes `parent` as a list link). Carry the search path length of each node in the worklist, computed while its parent is still alive, and account for it with the new `removeKey` overload. Add a test covering the SHOW_MEMORY instrumentation, which the rest of the test suite doesn't compile with.	2026-10-26 18:32:03 -04:00