Skip to content

fix(fuzz): Do not access arrays with &mut inside if using dynamic condition#9072

Merged
aakoshh merged 10 commits intomasterfrom
af/9035-fuzz-dyn-if
Jul 10, 2025
Merged

fix(fuzz): Do not access arrays with &mut inside if using dynamic condition#9072
aakoshh merged 10 commits intomasterfrom
af/9035-fuzz-dyn-if

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Jul 1, 2025

Description

Problem*

Resolves #9035

Summary*

  • Add an in_dynamic flag to the AST generator which is turned on when we are inside the then or the else of an if statement where the condition is using a dynamic variable. Under such circumstances anything we do inside is under some predicate that will tie the outcome to the circuit inputs in ACIR.
  • Do not use arrays with references in them if we are in an in_dynamic region.
  • Move the DIE post-check to happen as the last step of the SSA pipeline, to let the dynamic index access verifier provide better feedback than what the panic does.

Waiting on #9047

Additional Context

Instead of panicking on leftover loads, we can defer it to runtime using #8943
Or we can keep panicking and try to turn those panics into better user errors, like the verify_no_dynamic_indices_to_references, once we have examples of them.

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@aakoshh aakoshh requested a review from a team July 1, 2025 10:10
Copy link
Contributor

@github-actions github-actions bot left a comment

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: c6d667a Previous: 441f39a Ratio
test_report_zkpassport_noir_rsa_ 1 s 0 s +∞

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

CC: @TomAFrench

@aakoshh aakoshh added the Blocked PR is blocked on an issue label Jul 1, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

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: ca0f12b Previous: 077bb6d Ratio
rollup-block-root-single-tx 227 s 185 s 1.23

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

CC: @TomAFrench

Copy link
Collaborator

@asterite asterite left a comment

Choose a reason for hiding this comment

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

Looks good! I left a question but it's likely me misunderstanding.

@aakoshh aakoshh enabled auto-merge July 10, 2025 11:25
@aakoshh aakoshh added this pull request to the merge queue Jul 10, 2025
Merged via the queue into master with commit 00980ec Jul 10, 2025
121 of 124 checks passed
@aakoshh aakoshh deleted the af/9035-fuzz-dyn-if branch July 10, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Blocked PR is blocked on an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DIE post-check crash: load left after mem2reg because of dynamic indexing

3 participants