Skip to content

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Sep 10, 2025

Description

Problem*

Follow up for #9786

Summary*

With a 1/10 chance leave out the modulo operation that the fuzzer uses to make sure that an arbitrary index will fall into the bounds of an array or slice. This will most certainly result in "Index out of bounds", and the goal is to make sure all backends handle it the same way.

It is only used if avoid_index_out_of_bounds is false, which we randomise in fuzz targets so that half the time we don't fall into trivial errors, but give the circuit a chance to complete.

Additional Context

This triggered a new bug: #9804

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.

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 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 42feaba Previous: d672757 Ratio
private-kernel-inner 0.017 s 0.013 s 1.31

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

CC: @TomAFrench

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: e6ed0e1 Previous: 2a5315c Ratio
test_report_zkpassport_noir_rsa_ 2 s 1 s 2

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

CC: @TomAFrench

Copy link
Contributor

@rkarabut rkarabut left a comment

Choose a reason for hiding this comment

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

I think I would prefer an explicit option to avoid OOB. I don't like it implicitly depending on avoid_overflow and otherwise being random 🤔

@aakoshh
Copy link
Contributor Author

aakoshh commented Sep 11, 2025

Thanks for pointing that out @rkarabut , added a new flag and a default_config function to make it easier to control this initialization.

Base automatically changed from af/9601-fuzz-slice-ins-rm to master September 11, 2025 10:55
@aakoshh aakoshh requested a review from rkarabut September 11, 2025 10:56
@aakoshh aakoshh enabled auto-merge September 11, 2025 19:55
@aakoshh aakoshh added this pull request to the merge queue Sep 11, 2025
Merged via the queue into master with commit 1578b7c Sep 11, 2025
123 checks passed
@aakoshh aakoshh deleted the af/fuzz-index-oob branch September 11, 2025 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants