fix(ssa): disallow flattening pure brillig calls with constant args

compiler/noirc_evaluator/src/ssa/mod.rs

@@ -308,6 +308,10 @@ pub fn primary_passes(options: &SsaEvaluatorOptions) -> Vec<SsaPass<'_>> {
         SsaPass::new(Ssa::remove_redundant_params, "Remove Redundant Parameters"),
         // Removing redundant block parameters can reveal new CFG structures that can be simplified further.
         SsaPass::new(Ssa::simplify_cfg, "Simplifying"),
+        SsaPass::new(
+            |ssa| ssa.fold_constants(options.constant_folding_max_iter),
+            "Constant Folding",
+        ),
         SsaPass::new(Ssa::flatten_cfg, "Flattening").and_then_validate(|#[allow(unused)] ssa| {
             #[cfg(debug_assertions)]
             validation::flatten_post_check::verify_side_effect_predicates(ssa)?;

compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs

@@ -179,6 +179,9 @@
         let no_predicates: HashSet<FunctionId> =
             self.functions.values().filter(|f| f.is_no_predicates()).map(|f| f.id()).collect();
 
+        #[cfg(debug_assertions)]
+        assert_no_pure_constant_brillig_calls(&self);
+
         // ACIR functions which are neither entry points (`Fold`) nor deferred to the
         // post-flattening inlining pass (`NoPredicates`) must already have been inlined
         // into their callers, so no calls to them may remain.
@@ -236,6 +239,101 @@
     });
 }
 
+/// Asserts that no ACIR function reaching flattening still calls a foldable brillig function
+/// with all-constant arguments.
+///
+/// Such a call has no side effects to preserve and a constant result, so constant folding
+/// should have replaced it with that result before flattening; otherwise the result is
+/// multiplied by a predicate during flattening, turning the constant into a witness and
+/// blocking further simplification.
+///
+/// A brillig call from ACIR is always at most `PureWithPredicate` (ACIRgen returns bogus
+/// values under a disabled predicate), so the stored purity can never be used directly.
+/// Instead [`is_foldable_pure`] asks whether the callee would be entirely pure if it were not
+/// for that floor — transitively side-effect free *and* guaranteed to terminate (no loops, no
+/// recursion). Anything else (an `assert`/out-of-bounds hint, or a non-terminating one like
+/// `regression_9006`) is left in place, matching what constant folding can actually fold.
+///
+/// The foldability check walks only the candidate callee's call-subgraph (memoized), so it
+/// stays cheap on large programs and off the fuzzer's hot path.
+#[cfg(debug_assertions)]
+fn assert_no_pure_constant_brillig_calls(ssa: &Ssa) {
+    let mut foldable_cache: HashMap<FunctionId, bool> = HashMap::default();
+
+    for function in ssa.functions.values() {
+        if function.runtime().is_brillig() {
+            continue;
+        }
+        let dfg = &function.dfg;
+        for block in function.reachable_blocks() {
+            for inst in dfg[block].instructions() {
+                let Instruction::Call { func, arguments } = &dfg[*inst] else { continue };
+                let Value::Function(callee_id) = &dfg[*func] else { continue };
+                if !ssa.functions.get(callee_id).is_some_and(|f| f.runtime().is_brillig()) {
+                    continue;
+                }
+                if arguments.is_empty() || !arguments.iter().all(|arg| dfg.is_constant(*arg)) {
+                    continue;
+                }
+
+                let mut on_stack = HashSet::default();
+                assert!(
+                    !is_foldable_pure(ssa, *callee_id, &mut foldable_cache, &mut on_stack),
+                    "Call to pure brillig function {callee_id:?} with all-constant arguments \
+                     ({} args) should have been interpreted by constant folding before flattening.",
+                    arguments.len(),
+                );
+            }
+        }
+    }
+}
+
+/// Returns true if `id` and everything it transitively calls is side-effect free and
+/// guaranteed to terminate (no loops, no recursion) — i.e. a constant-argument call to it is
+/// safe to fold to a constant. Results are memoized in `cache`; `on_stack` detects recursion,
+/// which is treated as not foldable (it may not terminate).
+#[cfg(debug_assertions)]
+fn is_foldable_pure(
+    ssa: &Ssa,
+    id: FunctionId,
+    cache: &mut HashMap<FunctionId, bool>,
+    on_stack: &mut HashSet<FunctionId>,
+) -> bool {
+    use crate::ssa::opt::pure::Purity;
+
+    if let Some(&result) = cache.get(&id) {
+        return result;
+    }
+    if !on_stack.insert(id) {
+        // `id` is already on the current DFS path, so it is (mutually) recursive and may not
+        // terminate. Don't memoize here; its own frame records the final result.
+        return false;
+    }
+
+    let function = &ssa.functions[&id];
+    let mut foldable =
+        function.body_purity(Purity::Pure) == Purity::Pure && !function.contains_loop();
+
+    if foldable {
+        let dfg = &function.dfg;
+        'outer: for block in function.reachable_blocks() {
+            for inst in dfg[block].instructions() {
+                if let Instruction::Call { func, .. } = &dfg[*inst]
+                    && let Value::Function(callee) = &dfg[*func]
+                    && !is_foldable_pure(ssa, *callee, cache, on_stack)
+                {
+                    foldable = false;
+                    break 'outer;
+                }
+            }
+        }
+    }
+
+    on_stack.remove(&id);
+    cache.insert(id, foldable);
+    foldable
+}
+
 /// Post-check condition for [`Ssa::flatten_cfg`].
 ///
 /// Panics if the ACIR function contains more than one block. The caller already
@@ -2790,7 +2888,7 @@
     }
 
     #[test]
     fn store_optimization_arrayset_zeroed_by_remove_unreachable() {
         // Bug: The Store optimization emits `array_set` under the branch's
         // `enable_side_effects`. Since `ArraySet` has `requires_acir_gen_predicate = true`,
         // `remove_unreachable_instructions` replaces it with a zeroed array when the
@@ -3922,4 +4020,75 @@
         }
         ");
     }
+
+    #[test]
+    #[should_panic(expected = "should have been interpreted by constant folding before flattening")]
+    fn flatten_pre_check_flags_side_effect_free_constant_brillig_call() {
+        // `f1` has no side effects, so it would be entirely pure if it were not for the
+        // brillig `PureWithPredicate` floor. A constant-argument call to it has a constant
+        // result and nothing to preserve, so it should have been folded before flattening;
+        // with constant folding skipped the surviving call must trip the pre-check.
+        let src = "
+            acir(inline) fn main f0 {
+              b0():
+                v1 = call f1(u32 2) -> u32
+                return v1
+            }
+            brillig(inline) fn f1 f1 {
+              b0(v0: u32):
+                v1 = unchecked_add v0, u32 1
+                return v1
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+        let _ = ssa.flatten_cfg();
+    }
+
+    #[test]
+    fn flatten_pre_check_preserves_side_effecting_constant_brillig_call() {
+        // `f1` contains a `constrain`, so it is only `PureWithPredicate` even ignoring the
+        // brillig floor. Folding it away would drop the constraint, so the pre-check must let
+        // the surviving constant-argument call through rather than panic.
+        let src = "
+            acir(inline) fn main f0 {
+              b0():
+                v1 = call f1(u32 0) -> u32
+                return v1
+            }
+            brillig(inline) fn f1 f1 {
+              b0(v0: u32):
+                constrain v0 == u32 1
+                return v0
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+        // Must not panic: the callee has a side effect to preserve.
+        let _ = ssa.flatten_cfg();
+    }
+
+    #[test]
+    fn flatten_pre_check_preserves_non_terminating_constant_brillig_call() {
+        // `f1` is side-effect free but loops forever for this argument, so constant folding
+        // cannot fold it (it gives up at its step limit) and the call legitimately reaches
+        // flattening. Because the callee contains a loop it is not considered foldable-pure,
+        // so the pre-check must not panic. Mirrors `compile_success_no_bug/regression_9006`.
+        let src = "
+            acir(inline) fn main f0 {
+              b0():
+                call f1(u1 0)
+                return
+            }
+            brillig(inline) fn f1 f1 {
+              b0(v0: u1):
+                jmp b1()
+              b1():
+                jmpif v0 then: b2(), else: b1()
+              b2():
+                return
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+        // Must not panic: a looping callee may not terminate, so it is not foldable-pure.
+        let _ = ssa.flatten_cfg();
+    }
 }

compiler/noirc_evaluator/src/ssa/opt/pure.rs

@@ -25,6 +25,9 @@ use crate::ssa::{
     ssa_gen::Ssa,
 };
 
+#[cfg(debug_assertions)]
+use crate::ssa::ir::basic_block::BasicBlockId;
+
 impl Ssa {
     /// Analyzes the purity of each function and tag each function call with that function's purity.
     /// This is purely an analysis pass on its own but can help future optimizations.
@@ -119,7 +122,12 @@ impl std::fmt::Display for Purity {
 }
 
 impl Function {
-    pub(crate) fn is_pure(&self) -> Purity {
+    /// Computes the purity of this function's body starting from `start` and only ever
+    /// lowering it (`Pure` → `PureWithPredicate` → `Impure`) as side effects are found.
+    /// Callers choose `start` to encode the floor that applies to the function's runtime.
+    /// Calls to other functions are treated as neutral here; callers that need transitive
+    /// purity must follow the call graph themselves.
+    pub(crate) fn body_purity(&self, start: Purity) -> Purity {
         let contains_reference = |value_id: &ValueId| {
             let typ = self.dfg.type_of_value(*value_id);
             typ.contains_reference()
@@ -153,13 +161,7 @@ impl Function {
         // that have nested arrays.
         let mut brillig_array_input_was_moved = false;
 
-        let mut result = if self.runtime().is_acir() {
-            Purity::Pure
-        } else {
-            // Because we return bogus values when a brillig function is called from acir
-            // in a disabled predicate, brillig functions can never be truly pure unfortunately.
-            Purity::PureWithPredicate
-        };
+        let mut result = start;
 
         for block in self.reachable_blocks() {
             for instruction in self.dfg[block].instructions() {
@@ -319,6 +321,55 @@ impl Function {
 
         result
     }
+
+    pub(crate) fn is_pure(&self) -> Purity {
+        let start = if self.runtime().is_acir() {
+            Purity::Pure
+        } else {
+            // Because we return bogus values when a brillig function is called from acir
+            // in a disabled predicate, brillig functions can never be truly pure unfortunately.
+            Purity::PureWithPredicate
+        };
+        self.body_purity(start)
+    }
+
+    /// Returns true if the function's control-flow graph contains a back-edge (a loop).
+    ///
+    /// This is a cheap depth-first search over block successors. Unlike the loop-finding pass it
+    /// does not build a dominator tree, so it is suitable for running on candidate callees during
+    /// the flatten pre-check without weighing down the fuzzer's hot path.
+    #[cfg(debug_assertions)]
+    pub(crate) fn contains_loop(&self) -> bool {
+        #[derive(Clone, Copy, PartialEq)]
+        enum Color {
+            Gray,
+            Black,
+        }
+
+        let dfg = &self.dfg;
+        let entry = self.entry_block();
+        let mut color: HashMap<BasicBlockId, Color> = HashMap::default();
+        color.insert(entry, Color::Gray);
+        let mut stack = vec![(entry, dfg[entry].successors().collect::<Vec<_>>())];
+        while let Some((_, successors)) = stack.last_mut() {
+            if let Some(successor) = successors.pop() {
+                match color.get(&successor) {
+                    // An edge back to a block on the current DFS path is a back-edge: a loop.
+                    Some(Color::Gray) => return true,
+                    Some(Color::Black) => {}
+                    None => {
+                        color.insert(successor, Color::Gray);
+                        let next = dfg[successor].successors().collect::<Vec<_>>();
+                        stack.push((successor, next));
+                    }
+                }
+            } else {
+                let (node, _) = stack.pop().expect("stack is non-empty in the loop body");
+                color.insert(node, Color::Black);
+            }
+        }
+        false
+    }
 }
 
 fn analyze_call_graph(

test_programs/compile_success_empty/regression_12042/Nargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "regression_12042"
+type = "bin"
+authors = [""]
+
+[dependencies]

test_programs/compile_success_empty/regression_12042/src/main.nr

@@ -0,0 +1,14 @@
+// Previously the EmbeddedCurveScalar::from_field call would not be fully constant folded before the CFG
+// flattening pass, which would prevent the MSM opcode from being simplified away.
+pub fn main(inp: Field) {
+    if inp == 1 {
+        // Use from_field to prevent constant folding the scalar construction;
+        // if both lo and hi are known constants, the MSM gets simplified away
+        // entirely and the bug is not exercised.
+        let scalar = std::embedded_curve_ops::EmbeddedCurveScalar::from_field(
+            340282366920938463463374607431768211456,
+        );
+        let point = std::embedded_curve_ops::fixed_base_scalar_mul(scalar);
+        assert(point.x != 0);
+    }
+}

test_programs/execution_success/regression_12034/Prover.toml

@@ -1 +1 @@
-inp = true
+x = "1"

test_programs/execution_success/regression_12034/src/main.nr

@@ -1,15 +1,11 @@
 // Regression test for #12034: MSM opcode must not mix constant/witness
 // for scalar halves (lo, hi). When one half is constant and the other
 // is a witness, the backend's to_grumpkin_scalar rejects the input.
-pub fn main(inp: bool) {
-    if inp {
-        // Use from_field to prevent constant folding the scalar construction;
-        // if both lo and hi are known constants, the MSM gets simplified away
-        // entirely and the bug is not exercised.
-        let scalar = std::embedded_curve_ops::EmbeddedCurveScalar::from_field(
-            340282366920938463463374607431768211456,
-        );
-        let point = std::embedded_curve_ops::fixed_base_scalar_mul(scalar);
-        assert(point.x != 0);
-    }
+//
+// The scalar fits in the lower 128 bits, so lo is a witness and hi is
+// constant zero, exercising the mixed constant/witness path.
+pub fn main(x: Field) {
+    let scalar = std::embedded_curve_ops::EmbeddedCurveScalar { lo: x, hi: 0 };
+    let point = std::embedded_curve_ops::fixed_base_scalar_mul(scalar);
+    assert(point.x != 0);
 }