From 87196e9419f9c12bc7739024e2f649dcbd3e7340 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Fri, 7 Feb 2025 11:09:46 +0100 Subject: [PATCH] fix: allows for infinite brillig loops (#7296) --- .../src/brillig/brillig_gen/variable_liveness.rs | 4 ++++ .../checks/check_for_underconstrained_values.rs | 3 ++- compiler/noirc_evaluator/src/ssa/ir/function.rs | 10 +++------- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 7 +++---- .../regression_7103/Nargo.toml | 7 +++++++ .../regression_7103/src/main.nr | 16 ++++++++++++++++ 6 files changed, 35 insertions(+), 12 deletions(-) create mode 100644 test_programs/compile_success_with_bug/regression_7103/Nargo.toml create mode 100644 test_programs/compile_success_with_bug/regression_7103/src/main.nr diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs index 37a63466119..97b200d657a 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs @@ -277,6 +277,10 @@ impl VariableLiveness { fn compute_loop_body(&self, edge: BackEdge) -> HashSet { let mut loop_blocks = HashSet::default(); + if edge.header == edge.start { + loop_blocks.insert(edge.header); + return loop_blocks; + } loop_blocks.insert(edge.header); loop_blocks.insert(edge.start); diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 24ad67c3b69..4dbdf18ac72 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -663,10 +663,11 @@ impl Context { &mut self, function: &Function, ) -> BTreeSet { + let returns = function.returns(); let variable_parameters_and_return_values = function .parameters() .iter() - .chain(function.returns()) + .chain(returns) .filter(|id| function.dfg.get_numeric_constant(**id).is_none()) .map(|value_id| function.dfg.resolve(*value_id)); diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index e7748b5f13f..7db8e317c46 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -165,17 +165,13 @@ impl Function { /// Returns the return types of this function. pub(crate) fn returns(&self) -> &[ValueId] { - let blocks = self.reachable_blocks(); - let mut function_return_values = None; - for block in blocks { + for block in self.reachable_blocks() { let terminator = self.dfg[block].terminator(); if let Some(TerminatorInstruction::Return { return_values, .. }) = terminator { - function_return_values = Some(return_values); - break; + return return_values; } } - function_return_values - .expect("Expected a return instruction, as function construction is finished") + &[] } /// Collects all the reachable blocks of this function. diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 0b5e37e817e..e5753aeba4e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -470,14 +470,14 @@ impl<'function> PerFunctionContext<'function> { &mut self, mut returns: Vec<(BasicBlockId, Vec)>, ) -> Vec { - // Clippy complains if this were written as an if statement match returns.len() { + 0 => Vec::new(), 1 => { let (return_block, return_values) = returns.remove(0); self.context.builder.switch_to_block(return_block); return_values } - n if n > 1 => { + _ => { // If there is more than 1 return instruction we'll need to create a single block we // can return to and continue inserting in afterwards. let return_block = self.context.builder.insert_block(); @@ -490,7 +490,6 @@ impl<'function> PerFunctionContext<'function> { self.context.builder.switch_to_block(return_block); self.context.builder.block_parameters(return_block).to_vec() } - _ => unreachable!("Inlined function had no return values"), } } @@ -682,7 +681,7 @@ impl<'function> PerFunctionContext<'function> { block_id: BasicBlockId, block_queue: &mut VecDeque, ) -> Option<(BasicBlockId, Vec)> { - match self.source_function.dfg[block_id].unwrap_terminator() { + match &self.source_function.dfg[block_id].unwrap_terminator() { TerminatorInstruction::Jmp { destination, arguments, call_stack } => { let destination = self.translate_block(*destination, block_queue); let arguments = vecmap(arguments, |arg| self.translate_value(*arg)); diff --git a/test_programs/compile_success_with_bug/regression_7103/Nargo.toml b/test_programs/compile_success_with_bug/regression_7103/Nargo.toml new file mode 100644 index 00000000000..fcffef80530 --- /dev/null +++ b/test_programs/compile_success_with_bug/regression_7103/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "regression_7103" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] diff --git a/test_programs/compile_success_with_bug/regression_7103/src/main.nr b/test_programs/compile_success_with_bug/regression_7103/src/main.nr new file mode 100644 index 00000000000..7ce01c2b079 --- /dev/null +++ b/test_programs/compile_success_with_bug/regression_7103/src/main.nr @@ -0,0 +1,16 @@ +fn main() { + /// Safety: n/a + unsafe { loophole() }; +} + + +unconstrained fn loophole() { + let mut i = 0; + loop { + println(i); + i += 1; + if false { + break; + } + } +} \ No newline at end of file