Skip to content

fix(ssa): Handle partially removed ArrayGet groups of complex type during OOB checks#10027

Merged
aakoshh merged 5 commits intomasterfrom
af/9851-fix-die-oob
Sep 30, 2025
Merged

fix(ssa): Handle partially removed ArrayGet groups of complex type during OOB checks#10027
aakoshh merged 5 commits intomasterfrom
af/9851-fix-die-oob

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Sep 29, 2025

Description

Problem*

Resolves #9851
Resolves #9852
Resolves #9873

Summary*

Fixes handle_array_get_group to not assume that all array_get and add pairs will be there.

The new detection mechanism is basically to loop through the next instructions and check that they are either an add, or an array_get from the same array with an unsafe index. If either of those condition do not hold, then we conclude that the unsafe&unused instruction we are currently on is the last of the group. Then, we check how many array_get we encountered in the group and how many of those were also found in the possible_index_out_of_bounds_indexes collection, which indicates they were also unused. If the two matches, then a constraint will be included.

This means something like array_get v1, u32 10000000; array_get v1, u32 0; array_get v1, u32 100000001 will have a constraint inserted after the 1st and 3rd array_get separately, as they are treated as different groups, even though they are on the same array, because in between them is a safe access. Such code will not be generated by the initial SSA, but ostensibly they could be a result of an earlier DIE pass having removed and merged 3 different groups.

Additional Context

In the example I found, the SSA before the first DIE pass (part of Preprocess), was like this:

acir(inline) predicate_pure fn main f0 {
  b0():
    v2 = make_array b"KO"
    v4 = make_array [u1 0, v2] : [(u1, [u8; 2]); 1]
    v6 = array_get v4, index u32 20 -> u1
    v8 = array_get v4, index u32 21 -> [u8; 2]
    v9 = allocate -> &mut u1
    store v6 at v9
    return u1 1
}

So v8 is unused, but v6 is not. This resulted in scenario c) in the handle_array_get_group function, so it let DIE remove the second array_get. Then, mem2reg figured out that we don't need the store v6 at v9 instruction, leaving v6 unused as well, however this time DIE didn't realise that it needs to insert a constraint, because it couldnt' find the entire group.

I did not attempt any refactoring as mentioned in #9890 and #9891 but I agree that it's difficult to follow the index manipulation.

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.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2025

Changes to circuit sizes

Generated at commit: caf1ea45a03cfb6a2ef22979ca64ee747153c9d1, compared to commit: 42a64e705e7efd4a385f169736a64e37c4ba4e61

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
lambda_from_array +32 ❌ +1.77% +30 ❌ +0.75%
regression_struct_array_conditional +9 ❌ +11.84% +9 ❌ +0.28%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
lambda_from_array 1,841 (+32) +1.77% 4,054 (+30) +0.75%
regression_struct_array_conditional 85 (+9) +11.84% 3,221 (+9) +0.28%
nested_array_dynamic 3,237 (+32) +1.00% 12,115 (+30) +0.25%
lambda_from_global_array 30 (+4) +15.38% 2,802 (+4) +0.14%
databus_composite_calldata 137 (+4) +3.01% 3,103 (+4) +0.13%
array_dynamic_nested_blackbox_input 135 (+4) +3.05% 17,416 (0) 0.00%

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: 3bedb8e Previous: 70cb55c Ratio
test_report_zkpassport_noir-ecdsa_ 3 s 2 s 1.50

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

CC: @TomAFrench

@aakoshh aakoshh marked this pull request as ready for review September 29, 2025 16:01
@aakoshh aakoshh requested a review from a team September 29, 2025 16:14
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: 0498d74 Previous: 42a64e7 Ratio
sha512-100-bytes 0.095 s 0.056 s 1.70

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

CC: @TomAFrench

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

Reasoning makes sense to me

@aakoshh aakoshh enabled auto-merge September 30, 2025 09:13
@aakoshh aakoshh added this pull request to the merge queue Sep 30, 2025
Merged via the queue into master with commit 39f193c Sep 30, 2025
132 checks passed
@aakoshh aakoshh deleted the af/9851-fix-die-oob branch September 30, 2025 12:21
AztecBot added a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 8, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: Remove unnecessary allocation in `expr_with` (noir-lang/noir#10103)
chore(ACIR): inline `maybe_eq_predicate` (noir-lang/noir#10095)
chore: use `u64` over `Field` in template program (noir-lang/noir#10096)
chore: disallow slice arguments to blackbox functions (noir-lang/noir#10090)
chore(brillig_vm): Separate fuzzing module (noir-lang/noir#10091)
chore: ConstrainNotEqual requires acir predicate (noir-lang/noir#10062)
chore: bump version of bb used in tests (noir-lang/noir#10093)
chore: wrapping arithmetic tests (noir-lang/noir#9714)
chore(brillig_vm): Foreign call module and test re-org (noir-lang/noir#10089)
chore: add extra constraint folding pass (noir-lang/noir#9766)
chore: refactor brillig_blocks (noir-lang/noir#10088)
chore: add regression test for #9986 (noir-lang/noir#10087)
chore: Release Noir(1.0.0-beta.14) (noir-lang/noir#9942)
fix(tag_attr): keep whitespace tokens when parsing (noir-lang/noir#9981)
fix: hoist and then deduplicate (noir-lang/noir#10047)
chore: typos and some refactors in `acvm/src/pwg` (noir-lang/noir#10086)
chore: add in hack for `public_dispatch` (noir-lang/noir#10084)
fix(ssa): Avoid going through `i128` when casting signed to `u128` (noir-lang/noir#10045)
chore: avoid zero bits range-checks (noir-lang/noir#10083)
chore: bump external pinned commits (noir-lang/noir#10082)
fix(stdlib): Only compute the garbage `embedded_curve_result` result if we know we will need it (noir-lang/noir#10077)
fix(ssa): Keep defaults for values returned in the databus (noir-lang/noir#10042)
chore: remove unused predicate from mem-op solver (noir-lang/noir#10079)
chore(ACIR): snapshot tests for each instruction (noir-lang/noir#10071)
fix: remove generic length from ECDSA message hash in stdlib (noir-lang/noir#10043)
chore: validate that no jumps to function entry block exist (noir-lang/noir#10076)
feat(brillig): Centralize memory layout policy and reorganize memory regions (noir-lang/noir#9985)
chore(ci): fix permissions about publishing rustdoc (noir-lang/noir#10075)
chore(ACVM): use Vec instead of Hash for memory blocks (noir-lang/noir#10072)
feat(ssa): `constant_folding` with loop (noir-lang/noir#10019)
chore: take truncate into account for bit size (noir-lang/noir#10059)
chore: update check for `u128` overflow in `check_u128_mul_overflow` (noir-lang/noir#9998)
chore: update check for field overflow in `check_u128_mul_overflow` (noir-lang/noir#9968)
chore(ACIR): binary instructions snapshots (noir-lang/noir#10054)
chore(acir): SliceRemove refactor (noir-lang/noir#10058)
fix(fuzzer): Mark DivisionByZero with different types as equivalent (noir-lang/noir#10066)
chore(fuzz): Remove `is_frontend_friendly` from the AST fuzzer (noir-lang/noir#10046)
chore: use new ACIR syntax in docs, and some tests (noir-lang/noir#10057)
fix(ssa): SSA interpreter to use the 2nd arg in `slice_refcount` (noir-lang/noir#10034)
fix(ssa): SSA interpreter to return 0 for `Intrinsic::*RefCount` when constrained (noir-lang/noir#10033)
chore(ssa_fuzzer): fix array get/set  (noir-lang/noir#10031)
fix(acir): Extend slice on dynamic insertion and compilation panic when flattening (noir-lang/noir#10051)
chore(ACIR): extract convert_constrain_error helper (noir-lang/noir#10050)
chore(ACIR): expand signed lt, div and mod in SSA (noir-lang/noir#10036)
chore(ACIR): more consistent syntax and with less noise (noir-lang/noir#10014)
chore(acir): Code gen tests for slice intrinsics (noir-lang/noir#10017)
feat: parse and display SSA databus (noir-lang/noir#9991)
fix(ssa): Handle partially removed `ArrayGet` groups of complex type during OOB checks (noir-lang/noir#10027)
chore(acir): binary operations always have the same operand types (noir-lang/noir#10028)
feat: Add Module::parent and Module::child_modules (noir-lang/noir#10005)
chore: green light for ACVM optimisation (noir-lang/noir#10002)
chore: unit test for brillig solver (greenlight ACVM execution) (noir-lang/noir#9967)
chore(acir): avoid duplication when invoking brillig stdlib call (noir-lang/noir#10025)
chore: Use 8 partitions for rust tests (noir-lang/noir#10026)
chore: green light for ACVM execution audit (noir-lang/noir#9982)
chore: greenlight for ACVM execution (PWG) (noir-lang/noir#9961)
feat: optimize out noop casts on constants (noir-lang/noir#10024)
fix(mem2reg): consider call return aliases (noir-lang/noir#10016)
chore: bump external pinned commits (noir-lang/noir#10022)
fix(parser): enforce left brace after match expression (noir-lang/noir#10018)
chore(acir): Intrinsics and slice_ops modules as well as slice_ops doc comments (noir-lang/noir#10012)
fix: signed division by -1 can overflow (noir-lang/noir#9976)
chore(ci): fix external checks (noir-lang/noir#10009)
chore(ci): add provenance attestations to npm packages (noir-lang/noir#10011)
chore(ACIR): show all expressions as polynomials (noir-lang/noir#10007)
chore: remove unused feature flag (noir-lang/noir#9993)
chore(ci): fix docs breaking JS releases (noir-lang/noir#10010)
chore(ssa_fuzzer): add external coverage registration  (noir-lang/noir#9974)
END_COMMIT_OVERRIDE

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 8, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: Remove unnecessary allocation in `expr_with`
(noir-lang/noir#10103)
chore(ACIR): inline `maybe_eq_predicate`
(noir-lang/noir#10095)
chore: use `u64` over `Field` in template program
(noir-lang/noir#10096)
chore: disallow slice arguments to blackbox functions
(noir-lang/noir#10090)
chore(brillig_vm): Separate fuzzing module
(noir-lang/noir#10091)
chore: ConstrainNotEqual requires acir predicate
(noir-lang/noir#10062)
chore: bump version of bb used in tests
(noir-lang/noir#10093)
chore: wrapping arithmetic tests
(noir-lang/noir#9714)
chore(brillig_vm): Foreign call module and test re-org
(noir-lang/noir#10089)
chore: add extra constraint folding pass
(noir-lang/noir#9766)
chore: refactor brillig_blocks
(noir-lang/noir#10088)
chore: add regression test for #9986
(noir-lang/noir#10087)
chore: Release Noir(1.0.0-beta.14)
(noir-lang/noir#9942)
fix(tag_attr): keep whitespace tokens when parsing
(noir-lang/noir#9981)
fix: hoist and then deduplicate
(noir-lang/noir#10047)
chore: typos and some refactors in `acvm/src/pwg`
(noir-lang/noir#10086)
chore: add in hack for `public_dispatch`
(noir-lang/noir#10084)
fix(ssa): Avoid going through `i128` when casting signed to `u128`
(noir-lang/noir#10045)
chore: avoid zero bits range-checks
(noir-lang/noir#10083)
chore: bump external pinned commits
(noir-lang/noir#10082)
fix(stdlib): Only compute the garbage `embedded_curve_result` result if
we know we will need it (noir-lang/noir#10077)
fix(ssa): Keep defaults for values returned in the databus
(noir-lang/noir#10042)
chore: remove unused predicate from mem-op solver
(noir-lang/noir#10079)
chore(ACIR): snapshot tests for each instruction
(noir-lang/noir#10071)
fix: remove generic length from ECDSA message hash in stdlib
(noir-lang/noir#10043)
chore: validate that no jumps to function entry block exist
(noir-lang/noir#10076)
feat(brillig): Centralize memory layout policy and reorganize memory
regions (noir-lang/noir#9985)
chore(ci): fix permissions about publishing rustdoc
(noir-lang/noir#10075)
chore(ACVM): use Vec instead of Hash for memory blocks
(noir-lang/noir#10072)
feat(ssa): `constant_folding` with loop
(noir-lang/noir#10019)
chore: take truncate into account for bit size
(noir-lang/noir#10059)
chore: update check for `u128` overflow in `check_u128_mul_overflow`
(noir-lang/noir#9998)
chore: update check for field overflow in `check_u128_mul_overflow`
(noir-lang/noir#9968)
chore(ACIR): binary instructions snapshots
(noir-lang/noir#10054)
chore(acir): SliceRemove refactor
(noir-lang/noir#10058)
fix(fuzzer): Mark DivisionByZero with different types as equivalent
(noir-lang/noir#10066)
chore(fuzz): Remove `is_frontend_friendly` from the AST fuzzer
(noir-lang/noir#10046)
chore: use new ACIR syntax in docs, and some tests
(noir-lang/noir#10057)
fix(ssa): SSA interpreter to use the 2nd arg in `slice_refcount`
(noir-lang/noir#10034)
fix(ssa): SSA interpreter to return 0 for `Intrinsic::*RefCount` when
constrained (noir-lang/noir#10033)
chore(ssa_fuzzer): fix array get/set
(noir-lang/noir#10031)
fix(acir): Extend slice on dynamic insertion and compilation panic when
flattening (noir-lang/noir#10051)
chore(ACIR): extract convert_constrain_error helper
(noir-lang/noir#10050)
chore(ACIR): expand signed lt, div and mod in SSA
(noir-lang/noir#10036)
chore(ACIR): more consistent syntax and with less noise
(noir-lang/noir#10014)
chore(acir): Code gen tests for slice intrinsics
(noir-lang/noir#10017)
feat: parse and display SSA databus
(noir-lang/noir#9991)
fix(ssa): Handle partially removed `ArrayGet` groups of complex type
during OOB checks (noir-lang/noir#10027)
chore(acir): binary operations always have the same operand types
(noir-lang/noir#10028)
feat: Add Module::parent and Module::child_modules
(noir-lang/noir#10005)
chore: green light for ACVM optimisation
(noir-lang/noir#10002)
chore: unit test for brillig solver (greenlight ACVM execution)
(noir-lang/noir#9967)
chore(acir): avoid duplication when invoking brillig stdlib call
(noir-lang/noir#10025)
chore: Use 8 partitions for rust tests
(noir-lang/noir#10026)
chore: green light for ACVM execution audit
(noir-lang/noir#9982)
chore: greenlight for ACVM execution (PWG)
(noir-lang/noir#9961)
feat: optimize out noop casts on constants
(noir-lang/noir#10024)
fix(mem2reg): consider call return aliases
(noir-lang/noir#10016)
chore: bump external pinned commits
(noir-lang/noir#10022)
fix(parser): enforce left brace after match expression
(noir-lang/noir#10018)
chore(acir): Intrinsics and slice_ops modules as well as slice_ops doc
comments (noir-lang/noir#10012)
fix: signed division by -1 can overflow
(noir-lang/noir#9976)
chore(ci): fix external checks
(noir-lang/noir#10009)
chore(ci): add provenance attestations to npm packages
(noir-lang/noir#10011)
chore(ACIR): show all expressions as polynomials
(noir-lang/noir#10007)
chore: remove unused feature flag
(noir-lang/noir#9993)
chore(ci): fix docs breaking JS releases
(noir-lang/noir#10010)
chore(ssa_fuzzer): add external coverage registration
(noir-lang/noir#9974)
END_COMMIT_OVERRIDE
mralj pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 13, 2025
Automated pull of nightly from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
chore: Remove unnecessary allocation in `expr_with` (noir-lang/noir#10103)
chore(ACIR): inline `maybe_eq_predicate` (noir-lang/noir#10095)
chore: use `u64` over `Field` in template program (noir-lang/noir#10096)
chore: disallow slice arguments to blackbox functions (noir-lang/noir#10090)
chore(brillig_vm): Separate fuzzing module (noir-lang/noir#10091)
chore: ConstrainNotEqual requires acir predicate (noir-lang/noir#10062)
chore: bump version of bb used in tests (noir-lang/noir#10093)
chore: wrapping arithmetic tests (noir-lang/noir#9714)
chore(brillig_vm): Foreign call module and test re-org (noir-lang/noir#10089)
chore: add extra constraint folding pass (noir-lang/noir#9766)
chore: refactor brillig_blocks (noir-lang/noir#10088)
chore: add regression test for #9986 (noir-lang/noir#10087)
chore: Release Noir(1.0.0-beta.14) (noir-lang/noir#9942)
fix(tag_attr): keep whitespace tokens when parsing (noir-lang/noir#9981)
fix: hoist and then deduplicate (noir-lang/noir#10047)
chore: typos and some refactors in `acvm/src/pwg` (noir-lang/noir#10086)
chore: add in hack for `public_dispatch` (noir-lang/noir#10084)
fix(ssa): Avoid going through `i128` when casting signed to `u128` (noir-lang/noir#10045)
chore: avoid zero bits range-checks (noir-lang/noir#10083)
chore: bump external pinned commits (noir-lang/noir#10082)
fix(stdlib): Only compute the garbage `embedded_curve_result` result if we know we will need it (noir-lang/noir#10077)
fix(ssa): Keep defaults for values returned in the databus (noir-lang/noir#10042)
chore: remove unused predicate from mem-op solver (noir-lang/noir#10079)
chore(ACIR): snapshot tests for each instruction (noir-lang/noir#10071)
fix: remove generic length from ECDSA message hash in stdlib (noir-lang/noir#10043)
chore: validate that no jumps to function entry block exist (noir-lang/noir#10076)
feat(brillig): Centralize memory layout policy and reorganize memory regions (noir-lang/noir#9985)
chore(ci): fix permissions about publishing rustdoc (noir-lang/noir#10075)
chore(ACVM): use Vec instead of Hash for memory blocks (noir-lang/noir#10072)
feat(ssa): `constant_folding` with loop (noir-lang/noir#10019)
chore: take truncate into account for bit size (noir-lang/noir#10059)
chore: update check for `u128` overflow in `check_u128_mul_overflow` (noir-lang/noir#9998)
chore: update check for field overflow in `check_u128_mul_overflow` (noir-lang/noir#9968)
chore(ACIR): binary instructions snapshots (noir-lang/noir#10054)
chore(acir): SliceRemove refactor (noir-lang/noir#10058)
fix(fuzzer): Mark DivisionByZero with different types as equivalent (noir-lang/noir#10066)
chore(fuzz): Remove `is_frontend_friendly` from the AST fuzzer (noir-lang/noir#10046)
chore: use new ACIR syntax in docs, and some tests (noir-lang/noir#10057)
fix(ssa): SSA interpreter to use the 2nd arg in `slice_refcount` (noir-lang/noir#10034)
fix(ssa): SSA interpreter to return 0 for `Intrinsic::*RefCount` when constrained (noir-lang/noir#10033)
chore(ssa_fuzzer): fix array get/set  (noir-lang/noir#10031)
fix(acir): Extend slice on dynamic insertion and compilation panic when flattening (noir-lang/noir#10051)
chore(ACIR): extract convert_constrain_error helper (noir-lang/noir#10050)
chore(ACIR): expand signed lt, div and mod in SSA (noir-lang/noir#10036)
chore(ACIR): more consistent syntax and with less noise (noir-lang/noir#10014)
chore(acir): Code gen tests for slice intrinsics (noir-lang/noir#10017)
feat: parse and display SSA databus (noir-lang/noir#9991)
fix(ssa): Handle partially removed `ArrayGet` groups of complex type during OOB checks (noir-lang/noir#10027)
chore(acir): binary operations always have the same operand types (noir-lang/noir#10028)
feat: Add Module::parent and Module::child_modules (noir-lang/noir#10005)
chore: green light for ACVM optimisation (noir-lang/noir#10002)
chore: unit test for brillig solver (greenlight ACVM execution) (noir-lang/noir#9967)
chore(acir): avoid duplication when invoking brillig stdlib call (noir-lang/noir#10025)
chore: Use 8 partitions for rust tests (noir-lang/noir#10026)
chore: green light for ACVM execution audit (noir-lang/noir#9982)
chore: greenlight for ACVM execution (PWG) (noir-lang/noir#9961)
feat: optimize out noop casts on constants (noir-lang/noir#10024)
fix(mem2reg): consider call return aliases (noir-lang/noir#10016)
chore: bump external pinned commits (noir-lang/noir#10022)
fix(parser): enforce left brace after match expression (noir-lang/noir#10018)
chore(acir): Intrinsics and slice_ops modules as well as slice_ops doc comments (noir-lang/noir#10012)
fix: signed division by -1 can overflow (noir-lang/noir#9976)
chore(ci): fix external checks (noir-lang/noir#10009)
chore(ci): add provenance attestations to npm packages (noir-lang/noir#10011)
chore(ACIR): show all expressions as polynomials (noir-lang/noir#10007)
chore: remove unused feature flag (noir-lang/noir#9993)
chore(ci): fix docs breaking JS releases (noir-lang/noir#10010)
chore(ssa_fuzzer): add external coverage registration  (noir-lang/noir#9974)
END_COMMIT_OVERRIDE

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants