Repository navigation
Lock every source lookup made while functions are verified at once - #136
Merged
Merged
Conversation
…ons are verified at once.
…notes look up sources together.
This was referenced Oct 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With more than one job, functions are verified as tasks on a pool, and each annotates its obligations with source locations through Clang's
SourceManager, which caches its last lookups (getLineNumber,getFileID). The tasks took a lock for that annotation, but not every lookup did:failureNotes) unroll the loop and annotate each bound's module: that call bound to the memberannotateObligationSources, declared before the locking wrapper, so it ran unlocked; and itsathelper looked up the loop's line unlocked;placeFailure,relateClause) fall back tosourceOfwhen no source was recorded, unlocked.ThreadSanitizer reported the race in
SourceManager::getLineNumber(both sides insourceOfunderannotateObligationSources, one from the failure notes) oncalls_in_expressions.cpp,inductive_counterexamples.cpp,pointer_origins.cpp, andrange_for.cpp.Change
The lock is now
Verifier::SourceLock, a recursive mutex taken insidesourceOf,annotateObligationSources, and the failure notes'at, so every caller is covered. The local mutex and the wrapper lambda are gone.Audit
Every member function reachable from the two lambdas that run on the pool (
verifyFunctionandconfirm): 55 functions. The source manager or theASTContextis used only in those three;confirmmakes no lookup. VCR nodes hold source locations, not AST pointers, and no other file ofclang/lib/Verifybut the (serial) converter reaches the source manager.Test
suite/parallel_failure_notes.cppfails sixteen functions at once through loops whose invariants are too weak, so each failure's notes unroll to 9 iterations, and checks every failure's place and notes. Under ThreadSanitizer (-DLLVM_USE_SANITIZER=Thread, this branch): main reported the race in 12 of 12 runs of it (8209 reports), the fix in 0 of 12. In a release build the race rarely changes the output, so the lit test itself passes on main; the TSan sweep of the nightly job (separate PR) is what fails there. It takes 3 s in a release build.Release notes: "Source lookups from parallel failures"; ARCHITECTURE.md says which lookups take the lock.
Not covered: verification during a compilation
clang++ -fverify-contractsruns the whole verifier on astd::asyncthread while the main thread runs CodeGen'sHandleTranslationUniton the sameASTContextandSourceManager(BackendConsumer::HandleTranslationUnit), so this lock cannot order those. A TSanclangcompiling a file with contracts and deferred template code (-g -c) reported races in 4 of 4 runs, with this fix: CodeGen's debug locations againstVerifier::sourceOfingetLineNumber; linkage caching (setLinkageForGVagainstASTConverter::functionIdentity); andASTContext::getTypeInfo's map, which CodeGen inserts into whileVType::fromQualTypelooks up. That needs verification ordered against CodeGen and is left for its own change.