fix: add limit to brillig vm execution trace

acvm-repo/acvm/src/pwg/brillig.rs

@@ -173,7 +173,7 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
     }
 
     pub(crate) fn solve(&mut self) -> Result<BrilligSolverStatus<F>, OpcodeResolutionError<F>> {
-        let status = self.vm.process_opcodes();
+        let status = self.vm.process_opcodes(None);
         self.handle_vm_status(status)
     }
 

acvm-repo/brillig_vm/src/lib.rs

@@ -30,6 +30,9 @@
 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<usize>;
 
@@ -196,6 +199,20 @@
         self.status.clone()
     }
 
+    pub fn get_memory(&self) -> &[MemoryValue<F>] {
+        self.memory.values()
+    }
+
+    pub fn write_memory_at(&mut self, ptr: usize, value: MemoryValue<F>) {
+        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<usize> {
+        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<F> {
         self.status(VMStatus::Finished { return_data_offset, return_data_size })
@@ -247,26 +264,32 @@
     }
 
     /// Loop over the bytecode and update the program counter
-    pub fn process_opcodes(&mut self) -> VMStatus<F> {
-        while !matches!(
-            self.process_opcode(),
-            VMStatus::Finished { .. } | VMStatus::Failure { .. } | VMStatus::ForeignCallWait { .. }
-        ) {}
-        self.status.clone()
-    }
-
-    pub fn get_memory(&self) -> &[MemoryValue<F>] {
-        self.memory.values()
-    }
+    pub fn process_opcodes(&mut self, limit: Option<u32>) -> VMStatus<F> {
+        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<F>) {
-        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<usize> {
-        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.
@@ -583,7 +606,7 @@
                 self.set_program_counter(*location)
             }
             Opcode::Const { destination, value, bit_size } => {
                 // Consts are not checked in runtime to fit in the bit size, since they can safely be checked statically.
                 self.memory.write(*destination, MemoryValue::new_from_field(*value, *bit_size));
                 self.increment_program_counter()
             }
@@ -618,7 +641,7 @@
         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<F> {
@@ -1119,7 +1142,7 @@
     }
 
     #[test]
     fn jmpifnot_opcode() {
         let calldata: Vec<FieldElement> = vec![1u128.into(), 2u128.into()];
 
         let opcodes = vec![
@@ -1368,7 +1391,7 @@
     }
 
     #[test]
     fn cmov_opcode() {
         let calldata: Vec<FieldElement> =
             vec![(0u128).into(), (1u128).into(), (2u128).into(), (3u128).into()];
 
@@ -1625,7 +1648,7 @@
     }
 
     #[test]
     fn iconst_opcode() {
         let opcodes = &[
             Opcode::Const {
                 destination: MemoryAddress::direct(0),

compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs

@@ -338,7 +338,7 @@ fn execute_brillig<F: AcirField, B: BlackBoxFunctionSolver<F>>(
     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<F: AcirField, B: BlackBoxFunctionSolver<F>>(
         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

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<BrilligOpcode<FieldElement>> = 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 });
     }
 }

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;
         };

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]

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
+        }
+    }
+}