Skip to content

fix(ssa): Keep defaults for values returned in the databus#10042

Merged
aakoshh merged 6 commits intomasterfrom
af/9903-rm-unreach-databus
Oct 6, 2025
Merged

fix(ssa): Keep defaults for values returned in the databus#10042
aakoshh merged 6 commits intomasterfrom
af/9903-rm-unreach-databus

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Oct 1, 2025

Description

Problem*

Resolves #9903

Summary*

Make sure that any value returned in the databus is not removed from SSA:

  • remove_uncreachable_instruction will replace them with defaults even if the block is Unreachable and all other instructions are removed.
  • dead_instruction_elimination will consider these instructions used, even if their results are not used by anything else, because they stop appearing in the return terminator of an unreachable block.

Used #9991 to add unit tests, because integration tests don't cover SSA.

Additional Context

I tested with a program like this:

global G_A: [u32; 2] = [1, 2];
fn main(b: bool) -> return_data [u32; 2] {
    if b {
        let _a = G_A[2];
    } else {
        let _a = G_A[3];
    }
    G_A
}

I expected this to result in a CFG like b0 -> [b1, b2] -> b3, where b1 and b2 both become unreachable, and then b3, where the return_data is created, would not even appear in reachable_blocks(), and leaving the value in the databus unmapped, leading to a panic. Lucky for us: 1) remove_unreachable_instruction runs on the flattened CFG in ACIR and 2) in Brillig the return_data becomes None; otherwise this would be a problem.

In our case we have this SSA:

After Simplifying (4) (step 33):
g0 = u32 1
g1 = u32 2
g2 = make_array [u32 1, u32 2] : [u32; 2]

acir(inline) predicate_pure fn main f0 {
  b0(v3: u1):
    constrain u1 0 == v3, "Index out of bounds"
    constrain u1 0 == u1 1, "Index out of bounds"
    enable_side_effects u1 1
    v8 = make_array [Field 1, Field 2] : [Field; 2]
    return v8
}

After Remove Unreachable Instructions (1) (step 34):
g0 = u32 1
g1 = u32 2
g2 = make_array [u32 1, u32 2] : [u32; 2]

acir(inline) predicate_pure fn main f0 {
  b0(v3: u1):
    constrain u1 0 == v3, "Index out of bounds"
    constrain u1 0 == u1 1, "Index out of bounds"
    v7 = make_array [Field 0, Field 0] : [Field; 2]
    unreachable
}

So the change ensures that v7, which is what assigns the value to the returned Field array, still gets created, even though it's never returned.

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

Benchmark suite Current: 7b88baf Previous: 39f193c Ratio
perfectly_parallel_batch_inversion_opcodes 2781693 ns/iter (± 1335) 2254940 ns/iter (± 1115) 1.23

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

CC: @TomAFrench

@aakoshh aakoshh requested a review from a team October 1, 2025 11:01
@aakoshh aakoshh marked this pull request as ready for review October 1, 2025 11:01
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: 7b88baf Previous: 39f193c 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 added this pull request to the merge queue Oct 6, 2025
Merged via the queue into master with commit 348d92c Oct 6, 2025
132 checks passed
@aakoshh aakoshh deleted the af/9903-rm-unreach-databus branch October 6, 2025 08:52
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

Development

Successfully merging this pull request may close these issues.

Crash: SSA normalization fails to map the databus when terminator is unreachable

2 participants