diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 9abb5b306b0..08b5b5a0a80 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -173,7 +173,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { } pub(crate) fn solve(&mut self) -> Result, OpcodeResolutionError> { - let status = self.vm.process_opcodes(); + let status = self.vm.process_opcodes(None); self.handle_vm_status(status) } diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index 3de772c4359..eeb2b1670e3 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -30,6 +30,9 @@ mod black_box; mod cast; mod memory; +/// A suggested limit for the number of opcodes that can be executed in a single Brillig VM process. +pub const DEFAULT_EXECUTION_LIMIT: u32 = 10_000_000; + /// The error call stack contains the opcode indexes of the call stack at the time of failure, plus the index of the opcode that failed. pub type ErrorCallStack = Vec; @@ -196,6 +199,20 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { self.status.clone() } + pub fn get_memory(&self) -> &[MemoryValue] { + self.memory.values() + } + + pub fn write_memory_at(&mut self, ptr: usize, value: MemoryValue) { + self.memory.write(MemoryAddress::direct(ptr), value); + } + + /// Returns the VM's current call stack, including the actual program + /// counter in the last position of the returned vector. + pub fn get_call_stack(&self) -> Vec { + self.call_stack.iter().copied().chain(std::iter::once(self.program_counter)).collect() + } + /// Sets the current status of the VM to Finished (completed execution). fn finish(&mut self, return_data_offset: usize, return_data_size: usize) -> VMStatus { self.status(VMStatus::Finished { return_data_offset, return_data_size }) @@ -247,26 +264,32 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { } /// Loop over the bytecode and update the program counter - pub fn process_opcodes(&mut self) -> VMStatus { - while !matches!( - self.process_opcode(), - VMStatus::Finished { .. } | VMStatus::Failure { .. } | VMStatus::ForeignCallWait { .. } - ) {} - self.status.clone() - } - - pub fn get_memory(&self) -> &[MemoryValue] { - self.memory.values() - } + pub fn process_opcodes(&mut self, limit: Option) -> VMStatus { + let mut opcodes_processed = 0; + loop { + let new_status = self.process_opcode(); + + if matches!( + new_status, + VMStatus::Finished { .. } + | VMStatus::Failure { .. } + | VMStatus::ForeignCallWait { .. } + ) { + // If we reach these states then we either have finished execution or need to + // hand control back to the caller to process a foreign call. + break; + } - pub fn write_memory_at(&mut self, ptr: usize, value: MemoryValue) { - self.memory.write(MemoryAddress::direct(ptr), value); - } + if let Some(limit) = limit { + opcodes_processed += 1; + if opcodes_processed >= limit { + // If we have reached the limit, we stop processing opcodes + break; + } + } + } - /// Returns the VM's current call stack, including the actual program - /// counter in the last position of the returned vector. - pub fn get_call_stack(&self) -> Vec { - self.call_stack.iter().copied().chain(std::iter::once(self.program_counter)).collect() + self.status.clone() } /// Process a single opcode and modify the program counter. @@ -618,7 +641,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { self.set_program_counter(self.program_counter + 1) } - /// Increments the program counter by `value`. + /// Sets the program counter to `value`. /// If the program counter no longer points to an opcode /// in the bytecode, then the VMStatus reports halted. fn set_program_counter(&mut self, value: usize) -> VMStatus { diff --git a/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs b/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs index 0cde0a2a1ef..91e1beb5b2d 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs @@ -338,7 +338,7 @@ fn execute_brillig>( let mut vm = VM::new(calldata, code, blackbox_solver, profiling_active, None); // Run the Brillig VM on these inputs, bytecode, etc! - let vm_status = vm.process_opcodes(); + let vm_status = vm.process_opcodes(Some(acvm::brillig_vm::DEFAULT_EXECUTION_LIMIT)); // Check the status of the Brillig VM. // It may be finished, in-progress, failed, or may be waiting for results of a foreign call. @@ -347,7 +347,12 @@ fn execute_brillig>( VMStatus::Finished { return_data_offset, return_data_size } => Some( vm.get_memory()[return_data_offset..(return_data_offset + return_data_size)].to_vec(), ), - VMStatus::InProgress => unreachable!("Brillig VM has not completed execution"), + VMStatus::InProgress => { + // The brillig bytecode didn't terminate before the opcode limit was reached. + // This is likely due to an infinite loop which is preventing the brillig function from completing. + // We then leave the opcode in place to not block compilation. + None + } VMStatus::Failure { .. } => { // TODO: Return an error stating that the brillig function failed. None diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 81a40597839..af728d7a009 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -412,7 +412,7 @@ pub(crate) mod tests { let profiling_active = false; let mut vm = VM::new(calldata, bytecode, &DummyBlackBoxSolver, profiling_active, None); - let status = vm.process_opcodes(); + let status = vm.process_opcodes(None); if let VMStatus::Finished { return_data_offset, return_data_size } = status { (vm, return_data_offset, return_data_size) } else { @@ -491,7 +491,7 @@ pub(crate) mod tests { let bytecode: Vec> = context.artifact().finish().byte_code; let mut vm = VM::new(vec![], &bytecode, &DummyBlackBoxSolver, false, None); - let status = vm.process_opcodes(); + let status = vm.process_opcodes(None); assert_eq!( status, VMStatus::ForeignCallWait { @@ -505,7 +505,7 @@ pub(crate) mod tests { let response = ForeignCallResult { values: vec![ForeignCallParam::Array(number_sequence)] }; vm.resolve_foreign_call(response); - let status = vm.process_opcodes(); + let status = vm.process_opcodes(None); assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index f42b2e1d568..bf00612a696 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -651,7 +651,8 @@ impl<'brillig> Context<'brillig> { let black_box_solver = Bn254BlackBoxSolver(pedantic_solving); let profiling_active = false; let mut vm = VM::new(calldata, bytecode, &black_box_solver, profiling_active, None); - let vm_status: VMStatus<_> = vm.process_opcodes(); + let vm_status: VMStatus<_> = + vm.process_opcodes(Some(acvm::brillig_vm::DEFAULT_EXECUTION_LIMIT)); let VMStatus::Finished { return_data_offset, return_data_size } = vm_status else { return EvaluationResult::CannotEvaluate; }; diff --git a/test_programs/compile_success_no_bug/infinite_loop_brillig/Nargo.toml b/test_programs/compile_success_no_bug/infinite_loop_brillig/Nargo.toml new file mode 100644 index 00000000000..a647e8bc7db --- /dev/null +++ b/test_programs/compile_success_no_bug/infinite_loop_brillig/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "infinite_loop_brillig" +type = "bin" +authors = [""] +compiler_version = ">=0.31.0" + +[dependencies] diff --git a/test_programs/compile_success_no_bug/infinite_loop_brillig/src/main.nr b/test_programs/compile_success_no_bug/infinite_loop_brillig/src/main.nr new file mode 100644 index 00000000000..a894377f207 --- /dev/null +++ b/test_programs/compile_success_no_bug/infinite_loop_brillig/src/main.nr @@ -0,0 +1,11 @@ +fn main() { + unsafe { func_2() }; +} + +unconstrained fn func_2() { + loop { + if false { + break + } + } +}