Point writes are recorded via the 2-argument interleaved_insert::Result constructor, which leaves endRemaining (a TrivialSpan, whose default constructor deliberately leaves its members uninitialized) indeterminate. The Phase 2 debug print in interleavedWrites then unconditionally printed endRemaining, so any DEBUG_VERBOSE && !NDEBUG build read an indeterminate pointer/length pair. When stack garbage happened to be a span from a previous addWrites call, that produced a wild read of freed memory (heap-use-after-free under ASan, Invalid read under valgrind).
Change: only print the end search path / end span for range writes, mirroring the if (e != nullptr) check already used a few lines above for the forwardTo walk. Point writes now print just their begin line instead of a bogus search path: <end>, end: <garbage> line.
Production code was already correct (endRemaining is only otherwise read under endInsertionPoint != nullptr), so behaviour with the committed DEBUG_VERBOSE 0 is unchanged. Per review feedback, the Result constructors are left as-is rather than initializing the range-only members, which would only be dead stores.
Minimal two-call repro (range writes, then point writes reusing the same stackResults frame): valgrind 26 errors from 4 contexts before, 0 errors after; the bogus search path: <end>, end: <garbage> lines are gone.
corpus/003fdafe6e5f359e1043927b266e6ed6193562bc through FuzzTestDriver: valgrind clean after the fix.
Full ctest suite (-DCMAKE_BUILD_TYPE=Debug, 8598 tests incl. ASan/UBSan fuzz drivers and valgrind blackbox partitions) passes.
Note: the suite is not practical to run with DEBUG_VERBOSE 1 because the pre-existing verbosity (e.g. script_test_large_removal_buffer emits ~800 MB of stderr in 20s) blows the timeouts. That is unrelated to this change.
Closes #86.
Point writes are recorded via the 2-argument `interleaved_insert::Result` constructor, which leaves `endRemaining` (a `TrivialSpan`, whose default constructor deliberately leaves its members uninitialized) indeterminate. The Phase 2 debug print in `interleavedWrites` then unconditionally printed `endRemaining`, so any `DEBUG_VERBOSE && !NDEBUG` build read an indeterminate pointer/length pair. When stack garbage happened to be a span from a previous `addWrites` call, that produced a wild read of freed memory (`heap-use-after-free` under ASan, `Invalid read` under valgrind).
Change: only print the end search path / end span for range writes, mirroring the `if (e != nullptr)` check already used a few lines above for the `forwardTo` walk. Point writes now print just their begin line instead of a bogus `search path: <end>, end: <garbage>` line.
Production code was already correct (`endRemaining` is only otherwise read under `endInsertionPoint != nullptr`), so behaviour with the committed `DEBUG_VERBOSE 0` is unchanged. Per review feedback, the `Result` constructors are left as-is rather than initializing the range-only members, which would only be dead stores.
Verification (local, `DEBUG_VERBOSE 1` + assertions):
* Minimal two-call repro (range writes, then point writes reusing the same `stackResults` frame): valgrind `26 errors from 4 contexts` before, `0 errors` after; the bogus `search path: <end>, end: <garbage>` lines are gone.
* `corpus/003fdafe6e5f359e1043927b266e6ed6193562bc` through `FuzzTestDriver`: valgrind clean after the fix.
* Full `ctest` suite (`-DCMAKE_BUILD_TYPE=Debug`, 8598 tests incl. ASan/UBSan fuzz drivers and valgrind blackbox partitions) passes.
Note: the suite is not practical to run with `DEBUG_VERBOSE 1` because the pre-existing verbosity (e.g. `script_test_large_removal_buffer` emits ~800 MB of stderr in 20s) blows the timeouts. That is unrelated to this change.
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m46s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (pull_request) Successful in 4m5s
Point writes are recorded with the 2-argument
interleaved_insert::Result constructor, which leaves endRemaining
indeterminate, so the Phase 2 debug print in interleavedWrites read an
indeterminate span and, with stack garbage left over from an earlier
call, dereferenced a freed pointer. Confirmed with valgrind on a point
write following a range write: "Conditional jump or move depends on
uninitialised value(s)" and "Invalid read of size 1" at
ConflictSet.cpp:4984, and ASan heap-use-after-free in the same place.
Only print the end search path and spans for range writes, mirroring the
`if (e != nullptr)` check that already guards the endInsertionPoint
forwarding-chain fixup a few lines above. Production code already treats
endRemaining as range-write only, so nothing else changes.
weaselbot
changed title from Fix DEBUG_VERBOSE print of uninitialized Result::endRemaining (#86) to Skip DEBUG_VERBOSE end print for point writes (uninitialized Result::endRemaining)2026-10-27 17:07:16 +00:00
Pushed a revised commit: dropped the Result constructor initializations as requested (dead stores), so the diff is now only the if (e != nullptr) guard around the end-path debug print. Re-verified with valgrind: 0 errors on the minimal repro and on the corpus file with DEBUG_VERBOSE 1, and the full Debug ctest suite (8598 tests) passes.
Pushed a revised commit: dropped the `Result` constructor initializations as requested (dead stores), so the diff is now only the `if (e != nullptr)` guard around the end-path debug print. Re-verified with valgrind: 0 errors on the minimal repro and on the corpus file with `DEBUG_VERBOSE 1`, and the full Debug `ctest` suite (8598 tests) passes.
andrew
merged commit 20e2e332e4 into main2026-10-27 19:29:32 +00:00
andrew
deleted branch weaselbot/issue-862026-10-27 19:29:35 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
andrew
Closes #86.
Point writes are recorded via the 2-argument
interleaved_insert::Resultconstructor, which leavesendRemaining(aTrivialSpan, whose default constructor deliberately leaves its members uninitialized) indeterminate. The Phase 2 debug print ininterleavedWritesthen unconditionally printedendRemaining, so anyDEBUG_VERBOSE && !NDEBUGbuild read an indeterminate pointer/length pair. When stack garbage happened to be a span from a previousaddWritescall, that produced a wild read of freed memory (heap-use-after-freeunder ASan,Invalid readunder valgrind).Change: only print the end search path / end span for range writes, mirroring the
if (e != nullptr)check already used a few lines above for theforwardTowalk. Point writes now print just their begin line instead of a bogussearch path: <end>, end: <garbage>line.Production code was already correct (
endRemainingis only otherwise read underendInsertionPoint != nullptr), so behaviour with the committedDEBUG_VERBOSE 0is unchanged. Per review feedback, theResultconstructors are left as-is rather than initializing the range-only members, which would only be dead stores.Verification (local,
DEBUG_VERBOSE 1+ assertions):stackResultsframe): valgrind26 errors from 4 contextsbefore,0 errorsafter; the bogussearch path: <end>, end: <garbage>lines are gone.corpus/003fdafe6e5f359e1043927b266e6ed6193562bcthroughFuzzTestDriver: valgrind clean after the fix.ctestsuite (-DCMAKE_BUILD_TYPE=Debug, 8598 tests incl. ASan/UBSan fuzz drivers and valgrind blackbox partitions) passes.Note: the suite is not practical to run with
DEBUG_VERBOSE 1because the pre-existing verbosity (e.g.script_test_large_removal_bufferemits ~800 MB of stderr in 20s) blows the timeouts. That is unrelated to this change.@@ -3905,3 +3905,3 @@Result(Node *insertionPoint, TrivialSpan remaining): insertionPoint(insertionPoint), remaining(remaining),endInsertionPoint(nullptr) {}endInsertionPoint(nullptr), endRemaining{}, nextRangeWrite(nullptr) {}Drop the constructor initialization changes. They are just adding a bunch of dead stores
c45d3cba81toa792352387Fix DEBUG_VERBOSE print of uninitialized Result::endRemaining (#86)to Skip DEBUG_VERBOSE end print for point writes (uninitialized Result::endRemaining)Pushed a revised commit: dropped the
Resultconstructor initializations as requested (dead stores), so the diff is now only theif (e != nullptr)guard around the end-path debug print. Re-verified with valgrind: 0 errors on the minimal repro and on the corpus file withDEBUG_VERBOSE 1, and the full Debugctestsuite (8598 tests) passes.