feat(ssa): check purity contracts during SSA interpretation - #13518
feat(ssa): check purity contracts during SSA interpretation#13518asterite wants to merge 3 commits into
Conversation
Purity analysis classifies each function as Pure, PureWithPredicate, or Impure, and LICM, DIE, and constant-folding deduplication act on those results — but nothing verified that a function's runtime behavior matches its recorded purity. Several recent miscompilations were exactly this contract silently breaking (LICM-hoisted vector mutators, the vector- mutator wrapper purity bug, purities lost while cloning functions, Brillig callers of vector mutators considered pure). The SSA interpreter now enforces the contract on every interpreted call to a function with a recorded purity: - Pure and PureWithPredicate functions must not mutate caller-visible memory. On entry the storage identities reachable from the call's arguments are captured, and every in-place write (store through a reference, mutable array_set, vector intrinsic writing through its input vector) plus every global's storage is checked against the active scopes. Local allocations are not in any scope's set, so local mutation passes freely. - Pure functions must additionally not fail: execution failures escaping a Pure call (failed constraints, overflow, division by zero, out-of-bounds indexing, ...) are reported as violations, since passes may deduplicate or remove such calls. Interpretation artifacts (step budget, missing oracles, malformed SSA) are excluded. - Foreign function calls are forbidden inside any function with a recorded non-Impure purity. Violations surface as the new InterpreterError::PurityViolation, pointing at the offending function and its recorded purity. Since the interpreter runs in SSA pass unit tests (assert_pass_does_not_affect_execution), nargo interpret, and both fuzzers, drift between purity analysis and actual behavior now fails loudly instead of miscompiling silently. Purity is observed through the caller's runtime (DataFlowGraph::purity_of), matching what optimization passes see. When no purity analysis has run the checks are skipped. Adds Shared::as_ptr to noirc_frontend for storage-identity comparisons (Shared's PartialEq compares contents, not identity). Verified: full noirc_evaluator suite (1967 tests, which interpret SSA before/after every pass), nargo_cli interpret tests, and the AST fuzzer smoke test all pass with the checks active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ator intrinsics
The function-level purity checks validate the purity *map* against behavior,
but that map is computed from hardcoded per-intrinsic constants —
Intrinsic::mutates_array_operand_in_brillig in particular — which nothing
validated. An intrinsic that mutates its vector operand without being in
that list poisons the recorded purity of every function containing a call
to it, without any function-level contract being observably broken in the
function that uses the intrinsic directly (the LICM hoisted-mutator bug was
this shape).
The interpreter's single in-place vector mutation point now requires the
intrinsic on whose behalf it mutates to declare itself in
mutates_array_operand_in_brillig, reporting an IntrinsicPurityViolation
otherwise. Verified end-to-end by temporarily unlisting VectorPushBack:
the existing in-place push test immediately fails with
IntrinsicPurityViolation { intrinsic: VectorPushBack }.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y mutation check In a constrained context arrays have value semantics: the Mutable Array Set Optimizations pass marks an array_set mutable only when the old array value is dead, so the interpreter's in-place write merely reuses that value's backing store. It is not a caller-visible mutation, and purity analysis rightly keeps such functions predicate_pure — the check was flagging essentially every ACIR program interpreted after that pass. In Brillig the reference count governs genuine sharing, so the check still applies there. Also drops the intra-doc link from the public IntrinsicPurityViolation documentation to the pub(crate) mutates_array_operand_in_brillig, which failed cargo doc. The regression test reproduces the failure shape from the interpret execution tests (an ACIR predicate_pure function doing array_set mut on its parameter, classified by the real analysis); it was verified to fail against the unexempted check. All 3831 interpret_execution tests pass with the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CI diagnosis and fix (81cbae3): All 21 test-partition violations were one flavor — The Verified locally: the new regression test (ACIR 🤖 Generated with Claude Code |
Description
Problem
Purity analysis classifies every function as
Pure,PureWithPredicate, orImpure, and LICM, DIE, and constant-folding deduplication act on those results — but nothing verified that a function's actual behavior matches its recorded purity. This contract has silently broken four times recently, each one a miscompilation or near-miss:predicate_pureand deduplicated,All of these were found late, by a fuzzer roll or corpus luck, because the existing oracle (
pass_vs_prevdifferential testing) only fires on the consequence: a pass has to actually exploit the wrong purity and the divergence has to reach an output. For the LICM bug that meant aligning spare vector capacity, a specific loop shape, and no pre-loop use of the vector.Summary
The SSA interpreter now enforces the purity contract on every interpreted call — firing on the precondition (recorded purity contradicts observed behavior) instead, deterministically, on any program that exercises the misclassified function:
PureandPureWithPredicatefunctions must not mutate caller-visible memory. On call entry the storage identities reachable from the arguments are captured (via a newShared::as_ptr—Shared'sPartialEqcompares contents, not identity); every in-place write (store through a reference, mutablearray_set, vector intrinsics writing through their input) and every global's storage is checked against the active scopes. Local allocations are in no scope's set, so local mutation passes freely, matching purity analysis' "local mutations are not impure" rule.Purefunctions must not fail: execution failures escaping aPurecall (failed constraints, overflow, division by zero, out-of-bounds indexing, ...) are violations, since passes may deduplicate or remove such calls. Interpretation artifacts (step budget, missing oracles, malformed SSA) are excluded.Impurepurity.Intrinsic::mutates_array_operand_in_brillig. That list is an input to purity analysis, so an unlisted mutator poisons every containing function's recorded purity without any function-level contract observably breaking — exactly the LICM-bug shape, which the function-level checks alone cannot see.Violations surface as
InterpreterError::PurityViolation/IntrinsicPurityViolation, naming the offending function or intrinsic and its recorded contract. Purity is observed through the caller's runtime (DataFlowGraph::purity_of), matching what optimization passes see; when no purity analysis has run, the checks are skipped.Value
The interpreter runs inside every
assert_pass_does_not_affect_executionSSA pass unit test,nargo interpret(after every pass), and both fuzzers — so drift between purity analysis and actual behavior now fails loudly at every one of those points, with the cause named, instead of waiting for a pass to miscompile and a divergence to be bisected. Three of the four historical bugs above would have been caught directly at the misclassification stage (the wrapper bug, #13479, and the dangerous direction of the purity-cloning bug); the intrinsic-label check covers the remaining bare-intrinsic shape (the LICM bug).Verification
FunctionPurities, since the SSA parser validates hand-written annotations); the intrinsic-label wiring was verified end-to-end by temporarily unlistingVectorPushBack, which makes the existing in-place push test fail withIntrinsicPurityViolation.noirc_evaluatorsuite (1968 tests),nargo_cliinterpret test, AST fuzzer smoke test, and aztec-packages protocol circuits (private-kernel-inner, rollup-tx-base-private) undernargo interpret.nargo interpret --forceon those circuits shows no measurable overhead (private-kernel-inner 10.06s → 10.10s; rollup-tx-base-private within noise in both directions).🤖 Generated with Claude Code