From e48dde8a00232ba7a83a486c9a9f308ffe5329cd Mon Sep 17 00:00:00 2001 From: joyful-ii-V-I Date: Sat, 12 Sep 2026 11:09:52 -0400 Subject: [PATCH 1/2] diag(noalias): VERIFY_NO_ALIAS at the entry of 15 functions where self-aliasing is UB or a wrong answer Applies the lane D subset of a src/-wide audit for VERIFY_NO_ALIAS / VERIFY_NO_ALIAS3 candidates: 15 one-line insertions across 13 files, each immediately after the opening brace of the function body and before the first statement it protects. Every guard is debug-only (compiles to nothing under NDEBUG) and asserts what the audit measured: no release codegen change, no signature change, no publicness change. Also fixes test/columnarcommacheck.sh, whose standalone probe build of columnar.h did not link src/infra/diagnostics.cpp. That was fine while buildPathTable never referenced VERIFY; the new VERIFY_NO_ALIAS3 call needs Diagnostics::ConsoleLog::handleAssert, so the gate now links diagnostics.cpp the same way every other standalone harness in test/ already does. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 7 +++++++ src/columnar.h | 1 + src/commentcoherence.h | 1 + src/contextratio.h | 1 + src/editpreview.h | 1 + src/gitoracle.h | 1 + src/ingest_cache.h | 1 + src/ingest_parsepool.h | 1 + src/mcpindex.h | 1 + src/mention.h | 3 +++ src/packtask.h | 1 + src/search.h | 1 + src/testmap.h | 1 + src/verbs_navigate.h | 1 + test/columnarcommacheck.sh | 4 +++- 15 files changed, 25 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0302c6f1f..5e7553edb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,13 @@ not published here — see `docs/EVALS.md` for the instruments behind the headli ## [Unreleased] +### Added — `VERIFY_NO_ALIAS` guards at 15 call sites where self-aliasing was a silent wrong answer or UB + +Debug-only correctness checks (`VERIFY_NO_ALIAS` / `VERIFY_NO_ALIAS3`, zero release codegen change) +at the top of 15 functions whose two-or-more same-element-type out-parameters would silently +mis-compute or invalidate an iterator if a caller ever passed the same object twice. No performance +claim; this is a debug-build guard, not an optimizer hint. + ## [0.6.0] — 2026-09-11 **Languages and integrations from outside the project, much faster on the largest trees, and answers that say where diff --git a/src/columnar.h b/src/columnar.h index a1ef457f5..4de80e047 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -78,6 +78,7 @@ inline constexpr const char* kColumnarLegend = inline void buildPathTable( const std::vector& rowFileIds, std::vector& outUniqueFiles, std::vector& outRowPathIdx ) { + VERIFY_NO_ALIAS3( rowFileIds, outUniqueFiles, outRowPathIdx ); outUniqueFiles.clear(); outRowPathIdx.clear(); outRowPathIdx.reserve( rowFileIds.size() ); diff --git a/src/commentcoherence.h b/src/commentcoherence.h index 7f159b14b..89e91ba3b 100644 --- a/src/commentcoherence.h +++ b/src/commentcoherence.h @@ -102,6 +102,7 @@ inline bool isCommentStopword( std::string_view w ) noexcept // splitIdentifier(...) lowercased, one call site — the ONE tokenizer this whole lens uses (header note). inline void lowerSplitInto( std::string_view text, std::vector& scratch, std::vector& out ) { + VERIFY_NO_ALIAS( scratch, out ); naminglens::splitIdentifier( text, scratch ); out.reserve( out.size() + scratch.size() ); for( const std::string& tok : scratch ) diff --git a/src/contextratio.h b/src/contextratio.h index ec6dd16c8..4a22cc74a 100644 --- a/src/contextratio.h +++ b/src/contextratio.h @@ -292,6 +292,7 @@ inline Facts collectFacts( const IngestResult& ing, const NameDefs& byName, { // name interning, so a "distinct NAME" fact is a u32 and the ext=/amb= folds are the same sort as the // entity fold. Same rule as byName: lookup only, never iterated. + VERIFY_NO_ALIAS( symRows, fileRows ); HashMap nameIndex; nameIndex.reserve( byName.size() ); diff --git a/src/editpreview.h b/src/editpreview.h index 8156541b8..39f7825d8 100644 --- a/src/editpreview.h +++ b/src/editpreview.h @@ -66,6 +66,7 @@ inline Outcome refuse( std::string message ) // kBinaryPayloadRefusal makes is exactly the condition that would drop the file from the index. inline bool readPayload( std::string_view spec, std::size_t maxFileBytes, std::string& out, std::string& err ) { + VERIFY_NO_ALIAS( out, err ); out.clear(); if( spec == "-" ) { diff --git a/src/gitoracle.h b/src/gitoracle.h index 3b0db1e06..12da011d4 100644 --- a/src/gitoracle.h +++ b/src/gitoracle.h @@ -255,6 +255,7 @@ inline void putStr( std::string& b, const std::string& s ) { // Every field written here is a git-controlled identifier / sha / date / path, all far inside 64 KiB; a // pathological one is CLAMPED rather than allowed to wrap the length field (G1 runs -fsanitize=integer). + VERIFY_NO_ALIAS( b, s ); const std::uint16_t n = std::uint16_t( std::min( s.size(), 0xffffu ) ); qsnapPut( b, n ); b.append( s.data(), n ); diff --git a/src/ingest_cache.h b/src/ingest_cache.h index 54999061b..afaf959bb 100644 --- a/src/ingest_cache.h +++ b/src/ingest_cache.h @@ -2122,6 +2122,7 @@ inline std::vector buildCacheWritePlan( const std::vector& prevEntries, std::vector& carryOut ) { + VERIFY_NO_ALIAS( prevEntries, carryOut ); carryOut.clear(); carryOut.reserve( prevEntries.size() ); { diff --git a/src/ingest_parsepool.h b/src/ingest_parsepool.h index 11ac62e65..59ce3e499 100644 --- a/src/ingest_parsepool.h +++ b/src/ingest_parsepool.h @@ -264,6 +264,7 @@ inline WarmHitTotals markCacheHits( const std::vector& files, const HashMap& cache, std::vector& candidates, std::vector& hits ) { + VERIFY_NO_ALIAS( candidates, hits ); WarmHitTotals tot; for( std::size_t fileId = 0; fileId < files.size(); ++fileId ) { diff --git a/src/mcpindex.h b/src/mcpindex.h index 397274700..10ac804a4 100644 --- a/src/mcpindex.h +++ b/src/mcpindex.h @@ -1274,6 +1274,7 @@ inline const McpIndex& getIndex( const std::string& root ) // identity and there is nothing to strip. inline void handleIdentity( const McpIndex& ix, NodeId id, std::string& canonOut, std::string& pathOut ) { + VERIFY_NO_ALIAS( canonOut, pathOut ); const Symbol& s = ix.ing.symbols[ id ]; const std::string_view rootArg = ix.ing.realPaths.empty() ? std::string_view( ix.root ) : std::string_view(); canonOut = ( id < ix.g.canonId.size() ) ? canonicalIdForEmit( ix.ing, s, rootArg ) : s.name; diff --git a/src/mention.h b/src/mention.h index 12c95c840..002713ef3 100644 --- a/src/mention.h +++ b/src/mention.h @@ -112,6 +112,7 @@ inline std::string capDisclosureNote( const CapDisclosure& disc ) // Every one of the three is "" unless a cap actually bit, so a bundle that lost nothing still pays nothing. inline void absorbCapDisclosure( const CapDisclosure& disc, std::string& note, std::string& xmlAttrs, std::string& jsonKeys ) { + VERIFY_NO_ALIAS3( note, xmlAttrs, jsonKeys ); note += capDisclosureNote( disc ); xmlAttrs += disc.xml; jsonKeys += disc.json; @@ -403,6 +404,7 @@ inline bool definesScopeName( const IngestResult& ing, const std::string& scope, inline void mentionUnkeptFiles( const IngestResult& ing, const RawMention& m, const std::vector& kept, std::vector& out ) { + VERIFY_NO_ALIAS( kept, out ); const std::size_t fileCount = ing.files.size(); for( std::size_t suffixLen = m.segments.size(); suffixLen >= 1; --suffixLen ) { @@ -801,6 +803,7 @@ struct DocMentionBoostInfo inline void collectRefusedDocLifts( const Graph& g, const std::vector& lensRank, const std::vector& order, std::size_t from, std::size_t to, std::vector& out ) { + VERIFY_NO_ALIAS( order, out ); for( std::size_t k = from; k < to; ++k ) { const NodeId anchor = order[k]; diff --git a/src/packtask.h b/src/packtask.h index daf4299cd..8e151471d 100644 --- a/src/packtask.h +++ b/src/packtask.h @@ -719,6 +719,7 @@ inline void partitionByEligibility( const std::vector& topRanked, const const std::vector& d1Mark, std::vector& eligibleIds, std::vector& d2plusIds ) { + VERIFY_NO_ALIAS3( topRanked, eligibleIds, d2plusIds ); for( NodeId id : topRanked ) { ( ( id < d0Mark.size() && d0Mark[id] ) || ( id < d1Mark.size() && d1Mark[id] ) ? eligibleIds : d2plusIds ).push_back( id ); diff --git a/src/search.h b/src/search.h index 833f1ffd4..7db2b3862 100644 --- a/src/search.h +++ b/src/search.h @@ -283,6 +283,7 @@ inline bool matchesOnlyEmpty( const RegexInfo& r ) // (e.g. `(ab|cd)(ef|gh)` → abef, abgh, cdef, cdgh, whose trigrams the AND can then require). inline bool crossProduct( const std::vector& a, const std::vector& b, std::vector& out ) { + VERIFY_NO_ALIAS3( a, b, out ); if( a.size() * b.size() > kMaxExactSet ) { return false; diff --git a/src/testmap.h b/src/testmap.h index c5efa5383..316f39785 100644 --- a/src/testmap.h +++ b/src/testmap.h @@ -891,6 +891,7 @@ inline bool dependenciesMapCorpus( const IngestResult& ing, const std::vector in ; do` in the token stream, one token of lookahead state. inline void appendForListStems( const std::vector& tokens, std::vector& stems ) { + VERIFY_NO_ALIAS( tokens, stems ); enum class Loop : std::uint8_t { Scan, Var, ExpectIn, List }; Loop state = Loop::Scan; for( const std::string& token : tokens ) diff --git a/src/verbs_navigate.h b/src/verbs_navigate.h index d80ab8c83..ac91343e3 100644 --- a/src/verbs_navigate.h +++ b/src/verbs_navigate.h @@ -1082,6 +1082,7 @@ inline std::optional sliceSincePrepare( const MainDispatch& d, std::string_ const ::TSLanguage* grammar, const rw::slicev::SliceScan& scan, const std::string& src, std::string& legendOut, std::string& bodyOut, rw::slicev::SliceEmitOpts& emit ) { + VERIFY_NO_ALIAS( legendOut, bodyOut ); const rw::Config& cfg = d.cfg; if( cfg.since.empty() ) { diff --git a/test/columnarcommacheck.sh b/test/columnarcommacheck.sh index f3db9d45a..e50835b9f 100755 --- a/test/columnarcommacheck.sh +++ b/test/columnarcommacheck.sh @@ -45,7 +45,9 @@ command -v "$CXX" >/dev/null 2>&1 || CXX=g++ # (PR #1, run 30732976779). Rationale + the CMake mapping this mirrors: scripts/cxxstd.sh. . "$ROOT/scripts/cxxstd.sh" CXXSTD="$( ripwire_cxx_std_flag "$CXX" )" -"$CXX" "$CXXSTD" -I "$ROOT/src" -I "$ROOT/src/infra" -I "$ROOT/third_party" "$SRC" -o "$TMP/t" 2>"$TMP/build.err" +# diagnostics.cpp supplies Diagnostics::ConsoleLog::handleAssert — link it exactly as every other +# standalone harness in test/ does, now that buildPathTable carries a VERIFY_NO_ALIAS3 guard. +"$CXX" "$CXXSTD" -I "$ROOT/src" -I "$ROOT/src/infra" -I "$ROOT/third_party" "$SRC" "$ROOT/src/infra/diagnostics.cpp" -o "$TMP/t" 2>"$TMP/build.err" if [ -x "$TMP/t" ]; then ok "standalone gate binary built" else From 2aab3a8f274d31a8cc58626765f2f116509d5b9a Mon Sep 17 00:00:00 2001 From: joyful-ii-V-I Date: Sat, 12 Sep 2026 17:15:52 -0400 Subject: [PATCH 2/2] diag(noalias): crossProduct checks each input against the output only; the CHANGELOG entry states what the macro is in release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on #201: (1) VERIFY_NO_ALIAS3( a, b, out ) also asserted &a != &b, but a and b are only read, so a self-product crossProduct( v, v, out ) is valid and would have VERIFY-failed in debug — now VERIFY_NO_ALIAS( a, out ) and VERIFY_NO_ALIAS( b, out ). (2) "zero release codegen change / not an optimizer hint" predates #200: in release the macro leaves the separate_storage promise, consumed on clang 18+ by default, LLVM 17/AppleClang 16 with the CMake flag for scalar accesses, never on GCC or clang < 17; for these 15 object-form sites it measured no codegen change, so the entry keeps "no performance claim" with the reason. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 12 ++++++++---- src/search.h | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37cb33dc0..f4068ef58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,10 +124,14 @@ regenerated (2026-09-11). cached CMake probe. ### Added — `VERIFY_NO_ALIAS` guards at 15 call sites where self-aliasing was a silent wrong answer or UB -Debug-only correctness checks (`VERIFY_NO_ALIAS` / `VERIFY_NO_ALIAS3`, zero release codegen change) -at the top of 15 functions whose two-or-more same-element-type out-parameters would silently -mis-compute or invalidate an iterator if a caller ever passed the same object twice. No performance -claim; this is a debug-build guard, not an optimizer hint. +`VERIFY_NO_ALIAS` / `VERIFY_NO_ALIAS3` at the top of 15 functions whose two-or-more same-element-type +out-parameters would silently mis-compute or invalidate an iterator if a caller ever passed the same +object twice. The check runs in debug builds; in release the macro leaves only the +`__builtin_assume_separate_storage` promise on the two objects, which the optimizer reads on clang 18+ +by default, on LLVM 17 / AppleClang 16 only with the CMake-added `-mllvm -basic-aa-separate-storage` +and there for scalar accesses, and not at all on GCC or clang before 17. For these 15 functions the +promise measured no codegen change (the object form says nothing about a container's heap buffer), so +there is no performance claim here: these are correctness contracts. ## [0.6.0] — 2026-09-11 diff --git a/src/search.h b/src/search.h index 7db2b3862..3eddffc12 100644 --- a/src/search.h +++ b/src/search.h @@ -283,7 +283,8 @@ inline bool matchesOnlyEmpty( const RegexInfo& r ) // (e.g. `(ab|cd)(ef|gh)` → abef, abgh, cdef, cdgh, whose trigrams the AND can then require). inline bool crossProduct( const std::vector& a, const std::vector& b, std::vector& out ) { - VERIFY_NO_ALIAS3( a, b, out ); + VERIFY_NO_ALIAS( a, out ); // inputs are only read: a self-product crossProduct( v, v, out ) is valid, + VERIFY_NO_ALIAS( b, out ); // so the contract is each input against the output, never a against b if( a.size() * b.size() > kMaxExactSet ) { return false;