fix(ssa_gen): Add constraint on slice length before popping

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -1519,6 +1519,10 @@ impl<'block, Registers: RegisterAllocator> BrilligBlock<'block, Registers> {
     /// and not a flattened length used internally to represent arrays of tuples.
     /// The length inside of `RegisterOrMemory::HeapVector` represents the entire flattened number
     /// of fields in the vector.
+    ///
+    /// Note that when we subtract a value, we expect that there is a constraint in SSA
+    /// to check that the length isn't already 0. We could add a constraint opcode here,
+    /// but if it's in SSA, there is a chance it can be optimized out.
     fn update_slice_length(
         &mut self,
         target_len: MemoryAddress,

compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/call.rs

@@ -138,7 +138,8 @@ pub(super) fn simplify_call(
                         slice.push_back(*elem);
                     }
 
-                    let new_slice_length = increment_slice_length(arguments[0], dfg, block);
+                    let new_slice_length =
+                        increment_slice_length(arguments[0], dfg, block, call_stack);
 
                     let new_slice = make_array(dfg, slice, element_type, block, call_stack);
                     return SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]);
@@ -156,7 +157,7 @@ pub(super) fn simplify_call(
                     slice.push_front(*elem);
                 }
 
-                let new_slice_length = increment_slice_length(arguments[0], dfg, block);
+                let new_slice_length = increment_slice_length(arguments[0], dfg, block, call_stack);
 
                 let new_slice = make_array(dfg, slice, element_type, block, call_stack);
                 SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
@@ -196,7 +197,7 @@ pub(super) fn simplify_call(
                     slice.pop_front().expect("There are no elements in this slice to be removed")
                 });
 
-                let new_slice_length = decrement_slice_length(arguments[0], dfg, block);
+                let new_slice_length = decrement_slice_length(arguments[0], dfg, block, call_stack);
 
                 results.push(new_slice_length);
 
@@ -229,7 +230,7 @@ pub(super) fn simplify_call(
                     index += 1;
                 }
 
-                let new_slice_length = increment_slice_length(arguments[0], dfg, block);
+                let new_slice_length = increment_slice_length(arguments[0], dfg, block, call_stack);
 
                 let new_slice = make_array(dfg, slice, typ, block, call_stack);
                 SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
@@ -267,7 +268,7 @@ pub(super) fn simplify_call(
                 let new_slice = make_array(dfg, slice, typ, block, call_stack);
                 results.insert(0, new_slice);
 
-                let new_slice_length = decrement_slice_length(arguments[0], dfg, block);
+                let new_slice_length = decrement_slice_length(arguments[0], dfg, block, call_stack);
 
                 results.insert(0, new_slice_length);
 
@@ -449,27 +450,30 @@ fn update_slice_length(
     dfg: &mut DataFlowGraph,
     operator: BinaryOp,
     block: BasicBlockId,
+    call_stack: CallStackId,
 ) -> ValueId {
     let one = dfg.make_constant(FieldElement::one(), NumericType::length_type());
     let instruction = Instruction::Binary(Binary { lhs: slice_len, operator, rhs: one });
-    let call_stack = dfg.get_value_call_stack_id(slice_len);
     dfg.insert_instruction_and_results(instruction, block, None, call_stack).first()
 }
 
 fn increment_slice_length(
     slice_len: ValueId,
     dfg: &mut DataFlowGraph,
     block: BasicBlockId,
+    call_stack: CallStackId,
 ) -> ValueId {
-    update_slice_length(slice_len, dfg, BinaryOp::Add { unchecked: false }, block)
+    update_slice_length(slice_len, dfg, BinaryOp::Add { unchecked: false }, block, call_stack)
 }
 
 fn decrement_slice_length(
     slice_len: ValueId,
     dfg: &mut DataFlowGraph,
     block: BasicBlockId,
+    call_stack: CallStackId,
 ) -> ValueId {
-    update_slice_length(slice_len, dfg, BinaryOp::Sub { unchecked: true }, block)
+    // Simplifications only run if the length is a known non-zero constant, so the subtraction should never overflow.
+    update_slice_length(slice_len, dfg, BinaryOp::Sub { unchecked: true }, block, call_stack)
 }
 
 fn simplify_slice_push_back(
@@ -492,7 +496,7 @@ fn simplify_slice_push_back(
         .insert_instruction_and_results(len_not_equals_capacity_instr, block, None, call_stack)
         .first();
 
-    let new_slice_length = increment_slice_length(arguments[0], dfg, block);
+    let new_slice_length = increment_slice_length(arguments[0], dfg, block, call_stack);
 
     for elem in &arguments[2..] {
         slice.push_back(*elem);
@@ -545,7 +549,7 @@ fn simplify_slice_pop_back(
     let element_count = element_types.len();
     let mut results = VecDeque::with_capacity(element_count + 1);
 
-    let new_slice_length = decrement_slice_length(arguments[0], dfg, block);
+    let new_slice_length = decrement_slice_length(arguments[0], dfg, block, call_stack);
 
     let element_size =
         dfg.make_constant((element_count as u128).into(), NumericType::length_type());
@@ -555,11 +559,11 @@ fn simplify_slice_pop_back(
         Instruction::binary(BinaryOp::Mul { unchecked: true }, arguments[0], element_size);
     let mut flattened_len =
         dfg.insert_instruction_and_results(flattened_len_instr, block, None, call_stack).first();
-    flattened_len = decrement_slice_length(flattened_len, dfg, block);
 
     // We must pop multiple elements in the case of a slice of tuples
     // Iterating through element types in reverse here since we're popping from the end
     for element_type in element_types.iter().rev() {
+        flattened_len = decrement_slice_length(flattened_len, dfg, block, call_stack);
         let get_last_elem_instr = Instruction::ArrayGet {
             array: arguments[1],
             index: flattened_len,
@@ -571,8 +575,6 @@ fn simplify_slice_pop_back(
             .insert_instruction_and_results(get_last_elem_instr, block, element_type, call_stack)
             .first();
         results.push_front(get_last_elem);
-
-        flattened_len = decrement_slice_length(flattened_len, dfg, block);
     }
 
     results.push_front(arguments[1]);

compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs

@@ -1079,6 +1079,17 @@ impl FunctionContext<'_> {
                 Intrinsic::SliceRemove => {
                     self.codegen_access_check(arguments[2], arguments[0]);
                 }
+                Intrinsic::SlicePopFront | Intrinsic::SlicePopBack => {
+                    // If the slice was empty, ACIR would fail appropriately as we check memory block sizes,
+                    // but for Brillig we need to lay down an explicit check. By doing this in the SSA we
+                    // might be able to optimize this away later.
+                    if self.builder.current_function.runtime().is_brillig() {
+                        let zero = self
+                            .builder
+                            .numeric_constant(0u32, NumericType::Unsigned { bit_size: 32 });
+                        self.codegen_access_check(zero, arguments[0]);
+                    }
+                }
                 _ => {
                     // Do nothing as the other intrinsics do not require checks
                 }

test_programs/execution_failure/regression_9266/Nargo.toml

@@ -0,0 +1,6 @@
+[package]
+name = "regression_9266"
+type = "bin"
+authors = [""]
+
+[dependencies]

test_programs/execution_failure/regression_9266/src/main.nr

@@ -0,0 +1,6 @@
+global G_A: [Field] = &[-282608142060723387702497189224219065418];
+fn main() -> pub Field {
+    let (a, _) = G_A.pop_back();
+    let (_, f) = a.pop_back();
+    f
+}

tooling/nargo_cli/build.rs

@@ -463,7 +463,7 @@ fn generate_execution_failure_tests(test_file: &mut File, test_data_dir: &Path)
             &test_dir,
             "execute",
             "execution_failure(nargo);",
-            &MatrixConfig::default(),
+            &MatrixConfig { vary_brillig: true, ..Default::default() },
         );
     }
     writeln!(test_file, "}}").unwrap();