Skip to content

fix: validate binary operations which do not allow fields#9649

Merged
TomAFrench merged 7 commits intomasterfrom
tf/disallow-bitwise-ops-on-fields-in-acir
Aug 27, 2025
Merged

fix: validate binary operations which do not allow fields#9649
TomAFrench merged 7 commits intomasterfrom
tf/disallow-bitwise-ops-on-fields-in-acir

Conversation

@TomAFrench
Copy link
Member

Description

Problem*

Resolves #9627

Summary*

These programs were not possible except through constructing invalid SSA. This PR adds additional SSA validation to catch these cases. I've also beefed up ACIRgen to panic rather than assuming that we've caught this case in the frontend.

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.

@TomAFrench TomAFrench requested a review from a team August 26, 2025 20:11
@TomAFrench TomAFrench enabled auto-merge August 26, 2025 21:10
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: b78797e Previous: 2b4dc88 Ratio
test_report_zkpassport_noir_rsa_ 2 s 1 s 2

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

CC: @TomAFrench

@TomAFrench
Copy link
Member Author

Not merging this yet as this shouldn't be changing the ACIRgen output so doing some investigation.

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: 39e9bea Previous: 2b4dc88 Ratio
private-kernel-inner 0.019 s 0.015 s 1.27

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

CC: @TomAFrench

@TomAFrench TomAFrench added this pull request to the merge queue Aug 27, 2025
Merged via the queue into master with commit bf662ee Aug 27, 2025
122 checks passed
@TomAFrench TomAFrench deleted the tf/disallow-bitwise-ops-on-fields-in-acir branch August 27, 2025 10:36
let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs, 1)?;
Ok(outputs[0])
match typ.to_numeric_type() {
NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there's no i1, but this is also harmless so it's probably fine.

github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 28, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: make Ord for slices lexicographic (elements first, then length)
(noir-lang/noir#9555)
chore(ssa): Refactor `unrolling`
(noir-lang/noir#9653)
chore(docs): Update dependency page's examples
(noir-lang/noir#9634)
fix(ssa): Constant fold Brillig calls using the SSA interpreter
(noir-lang/noir#9655)
chore: LICM refactors (noir-lang/noir#9642)
chore: add test for trait bound on implementing type
(noir-lang/noir#9652)
chore: pass `DataFlowGraph` instead of `Function` as arg
(noir-lang/noir#9656)
feat: Group one audit tests
(noir-lang/noir#9445)
fix(expand): better handling of dereferences (again)
(noir-lang/noir#9654)
feat(mem2reg): address last known value is independent of its aliases
(take three) (noir-lang/noir#9633)
chore: remove handling for slice arguments to MSM
(noir-lang/noir#9648)
fix: validate binary operations which do not allow fields
(noir-lang/noir#9649)
END_COMMIT_OVERRIDE
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 28, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: make Ord for slices lexicographic (elements first, then length)
(noir-lang/noir#9555)
chore(ssa): Refactor `unrolling`
(noir-lang/noir#9653)
chore(docs): Update dependency page's examples
(noir-lang/noir#9634)
fix(ssa): Constant fold Brillig calls using the SSA interpreter
(noir-lang/noir#9655)
chore: LICM refactors (noir-lang/noir#9642)
chore: add test for trait bound on implementing type
(noir-lang/noir#9652)
chore: pass `DataFlowGraph` instead of `Function` as arg
(noir-lang/noir#9656)
feat: Group one audit tests
(noir-lang/noir#9445)
fix(expand): better handling of dereferences (again)
(noir-lang/noir#9654)
feat(mem2reg): address last known value is independent of its aliases
(take three) (noir-lang/noir#9633)
chore: remove handling for slice arguments to MSM
(noir-lang/noir#9648)
fix: validate binary operations which do not allow fields
(noir-lang/noir#9649)
END_COMMIT_OVERRIDE
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.

ACIR vs Brillig: ACIR allows bitwise ops for fields

3 participants