fix: signed division by -1 can overflow by asterite · Pull Request #9976 · noir-lang/noir

asterite · 2025-09-24T13:15:28Z

Description

Problem

Resolves #9904
Resolves #9971
Resolves #9975
Resolves #10008

Summary

Handles the fact that i8::min / -1 can overflow in all scenarios (comptime interpreter, SSA interpreter, when doing SSA simplification, etc.)

Then there was a related bug in ACIR where for checking this we didn't use the predicate.

Additional Context

I wanted to fix this now because I didn't know if this was related to constant_folding (it partially is) and without this fix it's hard to tell if bugs I find in constant_folding are just a duplicate of one of these issues.

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.

compiler/noirc_evaluator/src/acir/acir_context/mod.rs

github-actions

⚠️ 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: `4b2fe87`	Previous: `075a31b`	Ratio
`rollup-checkpoint-merge`	`0.004` s	`0.003` s	`1.33`

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

CC: @TomAFrench

github-actions

⚠️ 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: `13b46dc`	Previous: `075a31b`	Ratio
`test_report_zkpassport_noir_rsa_`	`3` s	`2` s	`1.50`

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

CC: @TomAFrench

compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs

.../snapshots/compile_failure/signed_division_by_minus_one_overflow/execute__tests__stderr.snap

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

compiler/noirc_evaluator/src/acir/acir_context/mod.rs

vezenovm

Looks good. Before we merge can you check if we solve this too #10008

asterite · 2025-09-26T15:06:50Z

Looks good. Before we merge can you check if we solve this too #10008

Yes, it solves that too.

Should I add a regression test for it? I'm not sure because maybe in the end it's a duplicate of some of the other issues...

vezenovm · 2025-09-26T15:10:04Z

Should I add a regression test for it? I'm not sure because maybe in the end it's a duplicate of some of the other issues...

That test has a dynamic divisor. So if you see a test with a dynamic divisor no need, but otherwise doesn't hurt.

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>

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

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>

asterite added 11 commits September 24, 2025 10:14

comptime -128_i8 % -1 is overflow

1489dca

Improve comptime % 0 error message

d31f0e5

Correctly simplify signed division by -1

ea26916

Div or mod by -1 has side effects

b804e1b

Do not hoist division by -1

62a35e5

Add tests for the comptime case

e70dadf

Add a couple more tests

be67122

Fix the SSA interpreter

bc447b2

Add a regression test

cf1cec3

Missing predicate check in ACIR

e682f5d

Add a regression test for #9971

bd98900

asterite commented Sep 24, 2025

View reviewed changes

compiler/noirc_evaluator/src/acir/acir_context/mod.rs Show resolved Hide resolved

asterite marked this pull request as ready for review September 24, 2025 14:40

github-actions bot reviewed Sep 24, 2025

View reviewed changes

Handle one more comptime arithmetic failure

0d232c9

asterite requested a review from a team September 24, 2025 15:27

github-actions bot reviewed Sep 24, 2025

View reviewed changes

aakoshh reviewed Sep 25, 2025

View reviewed changes

compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs Outdated Show resolved Hide resolved

Reuse two_pow_bit_size_minus_one

7b41126

vezenovm reviewed Sep 26, 2025

View reviewed changes

asterite added 3 commits September 26, 2025 11:21

Remove extra snapshot

53759d6

Use IntegerConstant

ab9e090

Add "hashers" to cspell

13b46dc

vezenovm approved these changes Sep 26, 2025

View reviewed changes

Add a regression test for #10008

4b2fe87

asterite enabled auto-merge September 26, 2025 15:18

asterite added this pull request to the merge queue Sep 26, 2025

Merged via the queue into master with commit 8ca4af7 Sep 26, 2025
128 checks passed

asterite deleted the ab/signed-division-overflow branch September 26, 2025 16:03

noirwhal mentioned this pull request Sep 26, 2025

chore: Release Noir(1.0.0-beta.14) #9942

Merged

AztecBot mentioned this pull request Sep 27, 2025

chore: Bump Noir reference AztecProtocol/aztec-packages#17349

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: signed division by -1 can overflow#9976

fix: signed division by -1 can overflow#9976
asterite merged 17 commits intomasterfrom
ab/signed-division-overflow

asterite commented Sep 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot left a comment •

edited

Loading

Uh oh!

github-actions bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vezenovm left a comment

Uh oh!

asterite commented Sep 26, 2025

Uh oh!

vezenovm commented Sep 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

asterite commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Summary

Additional Context

Documentation

PR Checklist

Uh oh!

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

github-actions bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vezenovm left a comment

Choose a reason for hiding this comment

Uh oh!

asterite commented Sep 26, 2025

Uh oh!

vezenovm commented Sep 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asterite commented Sep 24, 2025 •

edited

Loading

github-actions bot left a comment •

edited

Loading

github-actions bot left a comment •

edited

Loading