Repository navigation
Run the verifier under ASan and UBSan every night; give the certifier fresh stacks - #133
Merged
Merged
Conversation
…t from reusing a precompiled header.
…me fails in a partly instrumented build.
…NITIZER's comments.
…f of its stack is used.
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.
Plan item T3 (sanitizer builds). Nothing ran the verifier under AddressSanitizer or UndefinedBehaviorSanitizer, though it shares a thread pool, a proof cache, and racing solvers between threads, and parses untrusted archives.
Change
-DCPPVERIFY_SANITIZER="address;undefined"buildsclangVerifyandcpp-verifywith those sanitizers, beside an uninstrumented LLVM, Clang, and Z3 (-fno-sanitize=vptr,function, since the verifier has no RTTI), asCPPVERIFY_ENABLE_COVERAGEinstrumentsclangVerifyalone. A fully sanitized LLVM and Clang take hours to build on a CI runner and slow the suite until its timeouts change verdicts; the code we write is what the sanitizers should check. The sanitized targets do not reuse the libraries' precompiled header, which is built without the sanitizers (DISABLE_PCH_REUSE)..github/workflows/cppverify-sanitize.sh BUILDruns every Verify test program through such a build, with--jobs=2, and fails on any sanitizer report. It does not compare verdicts, which lit checks. It setsallow_user_poisoning=0: LLVM's allocators poison memory in the instrumented code's copies of their inline functions, and the uninstrumented libraries then write it (use-after-poisoninPreprocessor::RegisterBuiltinMacro)..github/workflows/cppverify-sanitizers.ymlbuilds it with Clang 18 every night and runs the script; a pull request that changes the setup runs it, so this PR does.What the first CI run found: the certifier's stack
The first CI run (40836155635) failed 11 of 465 programs with ASan reports deep inside the counterexample checker's evaluation (
Evaluator::eval/evalNode/definition, recursing):spec_proof_functions,clause_proofs,counterexample_confirmation,induction_mutual_recursion,induction_schemes,interrupt,spec_automation,spec_fuel_counterexamples,spec_induction,spec_termination,time_report. With ASan's fake stack off the same programs still crashed, on the real stack: a stack overflow.The checker runs on a thread of its own with a fixed 256 MiB stack and allows 50,000 nested evaluations (
CertifyLimits::Frames), oneevaland oneevalNodeframe each. That fits only while a level is small, and its size depends on the build (-fstack-usage, and ASan's__asan_stack_malloc_Nclass for the instrumented frame):Certify.cppevalevalNode-O3-O3-O0-O3+ ASanASan gives every local of
evalNode, a large switch, its own guarded slot, so 50,000 levels need up to 0.8 GiB. The script'sulimit -scould not help: it sizes the pool's threads, not the checker's.The fix makes the nesting limit independent of frame sizes, as Clang's
runWithSufficientStackSpaceand rustc'sensure_sufficient_stackdo: the evaluator records where its stack begins, and once half of it is used it continues the evaluation on a fresh stack of the same size (llvm::runOnNewStack), returning to the old one afterwards (Evaluator::evalNodeWithStackSpace). The stack size is nowCertifyLimits::StackBytes(256 MiB, as before), so a test can choose it. An optimized build uses at most about 63 MiB for 50,000 levels and never switches, so release behavior is unchanged; an ASan build switches every 128 MiB and reaches the same nesting limit, so its verdicts are the release build's. The evaluation's state is in theEvaluatorobject, the certifier already ran on a thread of its own, and nothing it uses is thread-local (NodeBudgetis not used there).Testing
CertifyTest.DeepNestingContinuesOnFreshStacksevaluates a sum nested 10,000 deep with a 1 MiB stack and checks the value. Mutant (never switch stacks): the test crashes with SIGSEGV.no reports in 466 programsin 299 s on 64 cores. The 11 programs above now reach the nesting limit (the evaluation nesting limit was reached while evaluating odd, as in a release build) instead of crashing. One program dies by design (crash_report_url.cpp,#pragma clang __debug crash).clause_proofsandspec_termination, with ASan's fake stack on or off). The script no longer setsulimit -s 1048576, which never reached the checker's thread.Why there is no ThreadSanitizer job
This PR first had a TSan job too. In this setup, with only the verifier instrumented, TSan's runtime failed:
suite/spec_automation.cppwith--jobs=4hung for 3 hours. Under gdb, 66 of 69 runs of it failed inside the runtime: 58 with SIGSEGV in TSan's trace lists (tsan_ilist.hPush, fromTraceSwitchPartImpl; and in the deadlock detector'sremoveLockwhile Z3 unlocks its global parameter mutex), and 8 hung with every worker waiting on TSan's trace locks (TracePartAlloc).So a TSan run could not tell a race from its runtime's failure. #137 found the cause (the checker's nesting overflows TSan's 64 Ki-frame shadow stack) and adds the job; ARCHITECTURE.md here records only what was observed.
Docs
CPPVERIFY_ENABLE_COVERAGE): the option, the script, and why poisoning is off;