fix: prevent incorrect ACIRgen caused by noop truncations #7456

TomAFrench · 2025-02-20T14:36:45Z

Description

Problem*

Resolves #7451

Summary*

This PR fixes an issue where doing a bitwise AND which covers the full width of its type will result in a truncation instruction which results in invalid ACIR.

I've fixed this by avoiding creating this bad instruction but also added checks in acirgen so that we'll reject any invalid truncations and also any other divisions by constant values which are greater than the allowed type.

Additional Context

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/mod.rs

jfecher

LGTM

chore: remove `disable_macros` compile option (noir-lang/noir#7468) chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465) chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346) chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467) feat(experimental): show macro errors where they happen (noir-lang/noir#7333) feat: optimize FieldElement::num_bits (noir-lang/noir#7147) chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436) feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464) chore(docs): Noir Profiler external documentation (noir-lang/noir#7457) feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443) chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458) fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456) feat: add native `u128` type (noir-lang/noir#7301) chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450) fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)

* master: (89 commits) chore: bump external pinned commits (#7472) chore: remove `disable_macros` compile option (#7468) chore(ci): add workflow to automate bumping aztec-packages commit (#7465) chore: Release Noir(1.0.0-beta.3) (#7346) chore(ci): Missing dash in profiler command argument (#7467) feat(experimental): show macro errors where they happen (#7333) feat: optimize FieldElement::num_bits (#7147) chore(profiler): Docs on profiler command and more complete error reporting (#7436) feat(ci): Release noir-inspector in binaries (#7464) chore(docs): Noir Profiler external documentation (#7457) feat(ci): Publish binaries for noir-profiler (#7443) chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (#7458) fix: prevent incorrect ACIRgen caused by noop truncations (#7456) feat: add native `u128` type (#7301) chore: standardize that doc comments on top of statements and expression are allowed but warn (#7450) fix: don't let nargo fmt produce multiple trailing newlines (#7444) feat(cli): add noir-execute binary (#7384) chore!: make `ResolverError::OracleMarkedAsConstrained` into a full error (#7426) chore: simplify reports (#7421) fix: do not discard negative sign from field literals in comptime interpreter (#7439) ...

fix: don't panic when shifting too much (noir-lang/noir#7429) chore: bump external pinned commits (noir-lang/noir#7472) chore: remove `disable_macros` compile option (noir-lang/noir#7468) chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465) chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346) chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467) feat(experimental): show macro errors where they happen (noir-lang/noir#7333) feat: optimize FieldElement::num_bits (noir-lang/noir#7147) chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436) feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464) chore(docs): Noir Profiler external documentation (noir-lang/noir#7457) feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443) chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458) fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456) feat: add native `u128` type (noir-lang/noir#7301) chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450) fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444)

Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE feat: Sync from aztec-packages (noir-lang/noir#7474) fix: don't panic when shifting too much (noir-lang/noir#7429) chore: bump external pinned commits (noir-lang/noir#7472) chore: remove `disable_macros` compile option (noir-lang/noir#7468) chore(ci): add workflow to automate bumping aztec-packages commit (noir-lang/noir#7465) chore: Release Noir(1.0.0-beta.3) (noir-lang/noir#7346) chore(ci): Missing dash in profiler command argument (noir-lang/noir#7467) feat(experimental): show macro errors where they happen (noir-lang/noir#7333) feat: optimize FieldElement::num_bits (noir-lang/noir#7147) chore(profiler): Docs on profiler command and more complete error reporting (noir-lang/noir#7436) feat(ci): Release noir-inspector in binaries (noir-lang/noir#7464) chore(docs): Noir Profiler external documentation (noir-lang/noir#7457) feat(ci): Publish binaries for noir-profiler (noir-lang/noir#7443) chore: Copy #7387 docs into v1.0.0-beta.2 versioned_docs (noir-lang/noir#7458) fix: prevent incorrect ACIRgen caused by noop truncations (noir-lang/noir#7456) feat: add native `u128` type (noir-lang/noir#7301) chore: standardize that doc comments on top of statements and expression are allowed but warn (noir-lang/noir#7450) fix: don't let nargo fmt produce multiple trailing newlines (noir-lang/noir#7444) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <tom@tomfren.ch>

TomAFrench added 2 commits February 20, 2025 14:33

fix: prevent incorrect ACIRgen caused by noop truncations

e6cc0cb

fix: panic in acir-gen if performing invalid division

85b9fef

TomAFrench requested a review from a team February 20, 2025 14:50

TomAFrench commented Feb 20, 2025

View reviewed changes

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

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

5cbcc9d

jfecher approved these changes Feb 20, 2025

View reviewed changes

TomAFrench added this pull request to the merge queue Feb 20, 2025

Merged via the queue into master with commit 1fa9b33 Feb 20, 2025
102 checks passed

TomAFrench deleted the tf/regression-7451 branch February 20, 2025 15:33

noirwhal mentioned this pull request Feb 20, 2025

chore: Release Noir(1.0.0-beta.3) #7346

Merged

AztecBot mentioned this pull request Feb 20, 2025

feat: Sync from noir AztecProtocol/aztec-packages#12129

Closed

AztecBot mentioned this pull request Feb 20, 2025

feat: Sync from noir AztecProtocol/aztec-packages#12176

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent incorrect ACIRgen caused by noop truncations #7456

fix: prevent incorrect ACIRgen caused by noop truncations #7456

TomAFrench commented Feb 20, 2025 •

edited

Loading

jfecher left a comment

fix: prevent incorrect ACIRgen caused by noop truncations #7456

fix: prevent incorrect ACIRgen caused by noop truncations #7456

Conversation

TomAFrench commented Feb 20, 2025 • edited Loading

Description

Problem*

Summary*

Additional Context

Documentation*

PR Checklist*

jfecher left a comment

Choose a reason for hiding this comment

TomAFrench commented Feb 20, 2025 •

edited

Loading