interleavedWrites DEBUG_VERBOSE print resolves endInsertionPoint's forwarding chain against the wrong variable (SEGV) #82

Closed
opened 2026-08-30 17:50:21 +00:00 by weaselbot · 0 comments
Member

File: ConflictSet.cpp:4973-4992 (the #if DEBUG_VERBOSE && !defined(NDEBUG) block at the top of Phase 2 of weaselab::ConflictSet::Impl::interleavedWrites), specifically line 4984 on main (commit ec1b476).

What the code does

The Phase 2 debug block resolves the forwarding (releaseDeferred/forwardTo) chains of the stored insertion points so they can be printed:

#if DEBUG_VERBOSE && !defined(NDEBUG)
      {
        Node *b = context.results[i].insertionPoint;
        Node *e = context.results[i].endInsertionPoint;
        while (b->releaseDeferred) {
          b = b->forwardTo;
        }
        if (e != nullptr) {
          while (e->releaseDeferred) {
            e = b->forwardTo;          // <-- line 4984: should be `e = e->forwardTo;`
          }
        }
        ...

The loop for e follows the begin node's forwarding chain instead of the end node's. Once b is resolved (b->releaseDeferred == false), its forwardTo field aliases the Entry union member (ConflictSet.cpp:295), i.e. the point/range version bytes reinterpreted as a pointer. So e is assigned garbage derived from version numbers, and the loop then dereferences it (e->releaseDeferred) until it crashes or, depending on the garbage value, spins forever. The analogous production code in the same loop body and in Phase 3 (lines 4997-5000, 5010-5014, 5034, 5037) resolves the chains correctly (... = ...->forwardTo), so line 4984 is clearly a typo. The correct fix is e = e->forwardTo;.

Impact

The debug block is inactive by default (DEBUG_VERBOSE 0 is committed; Internal.h:25), so release behavior is unaffected. But any developer who flips DEBUG_VERBOSE to 1 locally (which is exactly what the macro and the pre-commit pygrep hooks anticipate - they only forbid committing it) and runs a debug test gets a wild-pointer SEGV (or a hang on unlucky garbage) inside the very code path they are trying to debug, and the printed "end:" search path is nonsense. It also makes the debug block a landmine for future refactors.

Reproduction (HEAD ec1b476, gcc, Debug, aarch64):

  1. sed -i 's/#define DEBUG_VERBOSE 0/#define DEBUG_VERBOSE 1/' Internal.h
  2. cmake -B build-dbg -S . -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_COMPILER=g++ && cmake --build build --target fuzz_driver
  3. ./build/fuzz_driver corpus/003fdafe6e5f359e1043927b266e6ed6193562bc

Observed:

==3856152==ERROR: AddressSanitizer: SEGV on unknown address 0x152186cc15217a (pc 0xaaaac61ed204 ...)
==3856152==The signal is caused by a READ memory access.
    #0 ... in weaselab::ConflictSet::Impl::interleavedWrites(...) ConflictSet.cpp:4983
    #1 ... insertPointWritesOrSorted ... ConflictSet.cpp:5063
    #2 ... addWrites(...) ConflictSet.cpp:5142

With the stock DEBUG_VERBOSE 0 build, the same input runs cleanly (exit 0), so the crash comes from the debug block itself, not from the insertion machinery it is tracing.

Frequency: with DEBUG_VERBOSE 1, 307 of the 400 existing corpus files I tested crash at this line, so the broken debug print is hit routinely, not on some exotic input.

Suggested fix (one line, ConflictSet.cpp:4984):

           while (e->releaseDeferred) {
-            e = b->forwardTo;
+            e = e->forwardTo;
           }
**File:** `ConflictSet.cpp:4973-4992` (the `#if DEBUG_VERBOSE && !defined(NDEBUG)` block at the top of Phase 2 of `weaselab::ConflictSet::Impl::interleavedWrites`), specifically **line 4984** on main (commit `ec1b476`). **What the code does** The Phase 2 debug block resolves the forwarding (`releaseDeferred`/`forwardTo`) chains of the stored insertion points so they can be printed: ```cpp #if DEBUG_VERBOSE && !defined(NDEBUG) { Node *b = context.results[i].insertionPoint; Node *e = context.results[i].endInsertionPoint; while (b->releaseDeferred) { b = b->forwardTo; } if (e != nullptr) { while (e->releaseDeferred) { e = b->forwardTo; // <-- line 4984: should be `e = e->forwardTo;` } } ... ``` The loop for `e` follows the *begin* node's forwarding chain instead of the end node's. Once `b` is resolved (`b->releaseDeferred == false`), its `forwardTo` field aliases the `Entry` union member (`ConflictSet.cpp:295`), i.e. the point/range version bytes reinterpreted as a pointer. So `e` is assigned garbage derived from version numbers, and the loop then dereferences it (`e->releaseDeferred`) until it crashes or, depending on the garbage value, spins forever. The analogous production code in the same loop body and in Phase 3 (lines 4997-5000, 5010-5014, 5034, 5037) resolves the chains correctly (`... = ...->forwardTo`), so line 4984 is clearly a typo. The correct fix is `e = e->forwardTo;`. **Impact** The debug block is inactive by default (`DEBUG_VERBOSE 0` is committed; Internal.h:25), so release behavior is unaffected. But any developer who flips `DEBUG_VERBOSE` to 1 locally (which is exactly what the macro and the pre-commit pygrep hooks anticipate - they only forbid *committing* it) and runs a debug test gets a wild-pointer SEGV (or a hang on unlucky garbage) inside the very code path they are trying to debug, and the printed "end:" search path is nonsense. It also makes the debug block a landmine for future refactors. **Reproduction** (HEAD `ec1b476`, gcc, Debug, aarch64): 1. `sed -i 's/#define DEBUG_VERBOSE 0/#define DEBUG_VERBOSE 1/' Internal.h` 2. `cmake -B build-dbg -S . -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_COMPILER=g++ && cmake --build build --target fuzz_driver` 3. `./build/fuzz_driver corpus/003fdafe6e5f359e1043927b266e6ed6193562bc` Observed: ``` ==3856152==ERROR: AddressSanitizer: SEGV on unknown address 0x152186cc15217a (pc 0xaaaac61ed204 ...) ==3856152==The signal is caused by a READ memory access. #0 ... in weaselab::ConflictSet::Impl::interleavedWrites(...) ConflictSet.cpp:4983 #1 ... insertPointWritesOrSorted ... ConflictSet.cpp:5063 #2 ... addWrites(...) ConflictSet.cpp:5142 ``` With the stock `DEBUG_VERBOSE 0` build, the same input runs cleanly (exit 0), so the crash comes from the debug block itself, not from the insertion machinery it is tracing. **Frequency:** with `DEBUG_VERBOSE 1`, 307 of the 400 existing corpus files I tested crash at this line, so the broken debug print is hit routinely, not on some exotic input. **Suggested fix** (one line, ConflictSet.cpp:4984): ```diff while (e->releaseDeferred) { - e = b->forwardTo; + e = e->forwardTo; } ```
weaselbot was assigned by andrew 2026-08-30 21:09:44 +00:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: weaselab/conflict-set#82