-
Notifications
You must be signed in to change notification settings - Fork 382
chore: Ban dynamic array indices with reference elements #8888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
107 changes: 107 additions & 0 deletions
107
compiler/noirc_evaluator/src/ssa/validation/dynamic_array_indices.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| use crate::{ | ||
| errors::{RtResult, RuntimeError}, | ||
| ssa::{ | ||
| ir::{function::Function, instruction::Instruction}, | ||
| ssa_gen::Ssa, | ||
| }, | ||
| }; | ||
|
|
||
| impl Ssa { | ||
| /// Verifies there are no `array_get` or `array_set` instructions remaining | ||
| /// with dynamic indices where the element type may contain a reference type. | ||
| /// This effectively bans dynamic-indexing of arrays with reference elements | ||
| /// since we cannot guarantee we optimize references out of the program in that case. | ||
| /// | ||
| /// This pass expects to be run late in the Ssa pipeline such that all array indices | ||
| /// used are either constant or derived from an input to the program. E.g. we expect | ||
| /// optimizations from inlining, flattening, and mem2reg to already be complete. | ||
| pub(crate) fn verify_no_dynamic_indices_to_references(self) -> RtResult<Ssa> { | ||
| for function in self.functions.values() { | ||
| function.verify_no_dynamic_indices_to_references()?; | ||
| } | ||
| Ok(self) | ||
| } | ||
| } | ||
|
|
||
| impl Function { | ||
| pub(crate) fn verify_no_dynamic_indices_to_references(&self) -> RtResult<()> { | ||
| if self.runtime().is_brillig() { | ||
| return Ok(()); | ||
| } | ||
|
|
||
| for block in self.reachable_blocks() { | ||
| for instruction in self.dfg[block].instructions() { | ||
| match &self.dfg[*instruction] { | ||
| Instruction::ArrayGet { array, index, .. } | ||
| | Instruction::ArraySet { array, index, .. } => { | ||
| let array_type = self.dfg.type_of_value(*array); | ||
| let contains_reference = array_type.contains_reference(); | ||
|
|
||
| if contains_reference && self.dfg.get_numeric_constant(*index).is_none() { | ||
| let call_stack = self.dfg.get_instruction_call_stack(*instruction); | ||
| return Err(RuntimeError::DynamicIndexingWithReference { call_stack }); | ||
| } | ||
| } | ||
| _ => (), | ||
| } | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use crate::{errors::RuntimeError, ssa::ssa_gen::Ssa}; | ||
|
|
||
| #[test] | ||
| fn dynamic_array_of_2_mut_bools() { | ||
| // https://github.com/noir-lang/noir/issues/8750 | ||
| // fn main(c: u32) -> pub bool { | ||
| // let b: [&mut bool; 2] = [&mut false, &mut true]; | ||
| // *b[c % 2] | ||
| //} | ||
| let src = r#" | ||
| acir(inline) predicate_pure fn main f0 { | ||
| b0(v0: u32): | ||
| v1 = allocate -> &mut u1 | ||
| v2 = allocate -> &mut u1 | ||
| v3 = make_array [v1, v2] : [&mut u1; 2] | ||
| v4 = truncate v0 to 1 bits, max_bit_size: 32 | ||
| v6 = lt v4, u32 2 | ||
| constrain v6 == u1 1, "Index out of bounds" | ||
| v8 = array_get v3, index v4 -> &mut u1 | ||
| v9 = load v8 -> u1 | ||
| return v9 | ||
| }"#; | ||
|
|
||
| let ssa = Ssa::from_str(src).unwrap(); | ||
| let result = ssa.verify_no_dynamic_indices_to_references(); | ||
| assert!(matches!(result, Err(RuntimeError::DynamicIndexingWithReference { .. }))); | ||
| } | ||
|
|
||
| #[test] | ||
| fn no_error_in_brillig() { | ||
| // unconstrained fn main(c: u32) -> pub bool { | ||
| // let b: [&mut bool; 2] = [&mut false, &mut true]; | ||
| // *b[c % 2] | ||
| //} | ||
| let src = r#" | ||
| brillig(inline) predicate_pure fn main f0 { | ||
| b0(v0: u32): | ||
| v1 = allocate -> &mut u1 | ||
| v2 = allocate -> &mut u1 | ||
| v3 = make_array [v1, v2] : [&mut u1; 2] | ||
| v4 = truncate v0 to 1 bits, max_bit_size: 32 | ||
| v6 = lt v4, u32 2 | ||
| constrain v6 == u1 1, "Index out of bounds" | ||
| v8 = array_get v3, index v4 -> &mut u1 | ||
| v9 = load v8 -> u1 | ||
| return v9 | ||
| }"#; | ||
|
|
||
| let ssa = Ssa::from_str(src).unwrap(); | ||
| let result = ssa.verify_no_dynamic_indices_to_references(); | ||
| assert!(result.is_ok()); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.