Skip to content

fix(ast_fuzzer): forbid rebinding &mut T locals in constrained code - #12791

Merged
TomAFrench merged 4 commits into
masterfrom
cb/70557b3f0e2a
May 26, 2026
Merged

fix(ast_fuzzer): forbid rebinding &mut T locals in constrained code#12791
TomAFrench merged 4 commits into
masterfrom
cb/70557b3f0e2a

Conversation

@AztecBot

@AztecBot AztecBot commented May 26, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The nightly AST fuzzer run on 2026-05-25 failed three seeds (0x309f877a00100000, 0x39b83dfd00100000, 0x66eee82e00100000) under orig_vs_morph, min_vs_full, and pass_vs_prev respectively. All three panicked at RuntimeError::ReturnedReferenceFromDynamicIf (compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/value_merger.rs:115) and share a single root cause: in constrained code the AST fuzzer was emitting ref_var = &mut X; (bare rebind of a &mut T-typed local) inside a dynamic if/loop branch. The Noir frontend would normally reject that with MonomorphizationError::AssignedToVarContainingReference (compiler/noirc_frontend/src/monomorphization/mod.rs:2576), but the AST fuzzer bypasses the elaborator, so the bad assignment reached SSA and the value-merger refused to merge two Type::Reference(..) values across the merge point.
  • The fix is in tooling/ast_fuzzer/src/program/func.rs: gen_assign still keeps &mut T-typed mutable locals in the candidate set (so it can keep generating *ref_var = ... deref-assigns), but it now forces can_rebind = false for them in constrained code, which makes gen_lvalue always take the LValue::Dereference branch. The pre-existing filter that excluded aggregates containing references in constrained code is unchanged.

RCA gist

https://gist.github.com/AztecBot/2c7b203652f488725b8e2f1e45119bf8 — full analysis of all three failing ASTs and the shared bug pattern.

Failing CI: actions/runs/26383307255/job/77656632078.

Test plan

  • NOIR_AST_FUZZER_SEED=0x309f877a00100000 cargo test --release -p noir_ast_fuzzer_fuzz orig_vs_morph — passes
  • NOIR_AST_FUZZER_SEED=0x39b83dfd00100000 cargo test --release -p noir_ast_fuzzer_fuzz min_vs_full — passes
  • NOIR_AST_FUZZER_SEED=0x66eee82e00100000 cargo test --release -p noir_ast_fuzzer_fuzz pass_vs_prev — passes
  • CI=1 cargo test --release -p noir_ast_fuzzer_fuzz — 6 passed, no regressions
  • cargo test --release -p noir_ast_fuzzer --lib — 15 passed

No seed-based regression tests are included: they would pin to the current arb_program byte layout, so any future tweak to the generation algorithm would move what those seeds exercise and the tests would silently stop testing the intended pattern.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Brillig Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e6fe726 Previous: e5e4436 Ratio
private-kernel-reset 0.046 s 0.038 s 1.21

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

Drop the regression-seed scaffolding per review feedback: it relied on
arb_program's generation algorithm not changing, so any future tweak to
generation makes the seeds exercise unrelated paths.
@TomAFrench
TomAFrench marked this pull request as ready for review May 26, 2026 11:33
@TomAFrench
TomAFrench enabled auto-merge May 26, 2026 11:33

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e6fe726 Previous: c1f34cd Ratio
test_report_noir-lang_noir_bigcurve_ 321 s 253 s 1.27

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Elaboration Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e6fe726 Previous: e5e4436 Ratio
rollup-checkpoint-root 1.78 s 1.41 s 1.26
rollup-tx-base-private 1.668 s 1.38 s 1.21

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e6fe726 Previous: e5e4436 Ratio
rollup-checkpoint-root 7.61 s 5.92 s 1.29

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e6fe726 Previous: e5e4436 Ratio
rollup-tx-base-private 21.48 s 17.64 s 1.22

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Brillig Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: e6fe726 Previous: e5e4436 Ratio
rollup-checkpoint-root 2.83 s 2.27 s 1.25

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@TomAFrench
TomAFrench added this pull request to the merge queue May 26, 2026
Merged via the queue into master with commit bf1f2b3 May 26, 2026
136 checks passed
@TomAFrench
TomAFrench deleted the cb/70557b3f0e2a branch May 26, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants