fix(acir): Do not read empty array under inactive predicates for various slice ops

compiler/noirc_evaluator/src/acir/arrays.rs

@@ -1096,7 +1096,7 @@ impl Context<'_> {
     /// This is different from `flattened_size` in that a non-zero length
     /// array containing zero length arrays has zero size, but we can still
     /// access its elements.
-    fn has_zero_length(&mut self, array: ValueId, dfg: &DataFlowGraph) -> bool {
+    pub(super) fn has_zero_length(&mut self, array: ValueId, dfg: &DataFlowGraph) -> bool {
         if let Type::Array(_, size) = dfg.type_of_value(array) {
             size == 0
         } else {

compiler/noirc_evaluator/src/acir/call/intrinsics/slice_ops.rs

@@ -229,6 +229,13 @@ impl Context<'_> {
     ///
     /// The `result_ids` provided by the SSA to fetch the appropriate type information to be popped.
     /// The `result_ids` encode the type/shape of the removed element.
+    ///
+    /// # Empty Slice Handling
+    ///
+    /// If the slice has zero length, this function skips the memory read and returns zero values.
+    /// It asserts that the current side effects must be disabled (predicate = 0), otherwise fails
+    /// with "cannot pop from a slice with length 0". This prevents reading from empty memory blocks
+    /// which would cause "Index out of bounds" errors.
     pub(super) fn convert_slice_pop_back(
         &mut self,
         arguments: &[ValueId],
@@ -243,14 +250,10 @@ impl Context<'_> {
         let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?;
         let slice = self.convert_value(slice_contents, dfg);
 
-        // Slices with constant zero length should be eliminated as unreachable instructions,
-        // but in case they aren't, fail with a constant rather than try to subtract from the index.
-        let is_constant_zero_length =
-            dfg.get_numeric_constant(slice_length_var).is_some_and(|c| c.is_zero());
-
-        if is_constant_zero_length {
-            self.acir_context
-                .assert_always_fail("cannot pop from a slice with length 0".to_string())?;
+        if self.has_zero_length(slice_contents, dfg) {
+            // Make sure this code is disabled, or fail with "Index out of bounds".
+            let msg = "cannot pop from a slice with length 0".to_string();
+            self.acir_context.assert_zero_var(self.current_side_effects_enabled_var, msg)?;
 
             // Fill the result with default values.
             let mut results = Vec::with_capacity(result_ids.len());
@@ -343,7 +346,14 @@ impl Context<'_> {
     /// Unlike in [Self::convert_slice_pop_back], the returned slice contents differ from the input:
     /// the underlying array is logically truncated at the *front* rather than
     /// the back. The `result_ids` ensure that this logical shift is applied
-    /// consistently with the element’s type.
+    /// consistently with the element's type.
+    ///
+    /// # Empty Slice Handling
+    ///
+    /// If the slice has zero length, this function skips the memory read and returns zero values.
+    /// It asserts that the current side effects must be disabled (predicate = 0), otherwise fails
+    /// with "cannot pop from a slice with length 0". This prevents reading from empty memory blocks
+    /// which would cause "Index out of bounds" errors.
     pub(super) fn convert_slice_pop_front(
         &mut self,
         arguments: &[ValueId],
@@ -356,6 +366,32 @@ impl Context<'_> {
         let slice_typ = dfg.type_of_value(slice_contents);
         let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?;
 
+        // Check if we're trying to pop from an empty slice
+        if self.has_zero_length(slice_contents, dfg) {
+            // Make sure this code is disabled, or fail with "Index out of bounds".
+            let msg = "cannot pop from a slice with length 0".to_string();
+            self.acir_context.assert_zero_var(self.current_side_effects_enabled_var, msg)?;
+
+            // Fill the result with default values.
+            let mut results = Vec::with_capacity(result_ids.len());
+
+            let element_size = slice_typ.element_size();
+            // For pop_front, results order is: [popped_elements..., new_len, new_slice]
+            for result_id in &result_ids[..element_size] {
+                let result_type = dfg.type_of_value(*result_id);
+                let result_zero = self.array_zero_value(&result_type)?;
+                results.push(result_zero);
+            }
+
+            let slice_value = self.convert_value(arguments[0], dfg);
+            results.push(slice_value);
+
+            let slice = self.convert_value(slice_contents, dfg);
+            results.push(slice);
+
+            return Ok(results);
+        }
+
         let one = self.acir_context.add_constant(FieldElement::one());
         let new_slice_length = self.acir_context.sub_var(slice_length, one)?;
 
@@ -425,6 +461,13 @@ impl Context<'_> {
     ///    - If within the insertion window, write values from `flattened_elements`.
     ///    - If above the window, shift elements upward by the size of the inserted data.
     /// 4. Initialize a new memory block for the resulting slice, ensuring its type information is preserved.
+    ///
+    /// # Empty Slice Handling
+    ///
+    /// If the slice has zero length, this function skips the memory read and returns zero values.
+    /// It asserts that the current side effects must be disabled (predicate = 0), otherwise fails
+    /// with "Index out of bounds, slice has size 0". This prevents reading from empty memory blocks
+    /// which would cause "Index out of bounds" errors.
     pub(super) fn convert_slice_insert(
         &mut self,
         arguments: &[ValueId],
@@ -437,6 +480,25 @@ impl Context<'_> {
         let slice_typ = dfg.type_of_value(slice_contents);
         let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?;
 
+        // Check if we're trying to insert into an empty slice
+        if self.has_zero_length(slice_contents, dfg) {
+            // Make sure this code is disabled, or fail with "Index out of bounds".
+            let msg = "Index out of bounds, slice has size 0".to_string();
+            self.acir_context.assert_zero_var(self.current_side_effects_enabled_var, msg)?;
+
+            // Fill the result with default values.
+            let mut results = Vec::with_capacity(result_ids.len());
+
+            // For insert, results are: [new_len, new_slice]
+            let slice_length_value = self.convert_value(arguments[0], dfg);
+            results.push(slice_length_value);
+
+            let slice = self.convert_value(slice_contents, dfg);
+            results.push(slice);
+
+            return Ok(results);
+        }
+
         let slice = self.convert_value(slice_contents, dfg);
         let insert_index = self.convert_value(arguments[2], dfg).into_var()?;
 
@@ -620,6 +682,13 @@ impl Context<'_> {
     ///   - If `index + popped_elements_size` would exceed the slice length we do nothing. This ensures safe access at the tail of the array
     ///     and is safe to do as we are decreasing the slice length which gates slice accesses.
     /// 4. Initialize a new memory block for the resulting slice, ensuring its type information is preserved.
+    ///
+    /// # Empty Slice Handling
+    ///
+    /// If the slice has zero length, this function skips the memory read and returns zero values.
+    /// It asserts that the current side effects must be disabled (predicate = 0), otherwise fails
+    /// with "Index out of bounds, slice has size 0". This prevents reading from empty memory blocks
+    /// which would cause "Index out of bounds" errors.
     pub(super) fn convert_slice_remove(
         &mut self,
         arguments: &[ValueId],
@@ -633,6 +702,32 @@ impl Context<'_> {
         let slice_typ = dfg.type_of_value(slice_contents);
         let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?;
 
+        // Check if we're trying to remove from an empty slice
+        if self.has_zero_length(slice_contents, dfg) {
+            // Make sure this code is disabled, or fail with "Index out of bounds".
+            let msg = "Index out of bounds, slice has size 0".to_string();
+            self.acir_context.assert_zero_var(self.current_side_effects_enabled_var, msg)?;
+
+            // Fill the result with default values.
+            let mut results = Vec::with_capacity(result_ids.len());
+
+            // For remove, results are: [new_len, new_slice, ...removed_elements]
+            let slice_length_value = self.convert_value(arguments[0], dfg);
+            results.push(slice_length_value);
+
+            let slice = self.convert_value(slice_contents, dfg);
+            results.push(slice);
+
+            // Add zero values for removed elements
+            for result_id in &result_ids[2..] {
+                let result_type = dfg.type_of_value(*result_id);
+                let result_zero = self.array_zero_value(&result_type)?;
+                results.push(result_zero);
+            }
+
+            return Ok(results);
+        }
+
         let slice = self.convert_value(slice_contents, dfg);
         let remove_index = self.convert_value(arguments[2], dfg).into_var()?;
 

compiler/noirc_evaluator/src/acir/tests/intrinsics.rs

@@ -189,14 +189,14 @@ fn slice_pop_back_zero_length() {
 
     // An SSA with constant zero slice length should be removed in the "Remove unreachable instructions" pass,
     // however if it wasn't, we'd still want to generate a runtime constraint failure.
+    // The constraint should be based off of the side effects variable.
     assert_circuit_snapshot!(program, @r"
     func 0
     private parameters: [w0, w1]
     public parameters: []
     return values: []
     BLACKBOX::RANGE input: w0, bits: 32
-    BLACKBOX::RANGE input: w1, bits: 1
-    ASSERT 0 = 1
+    ASSERT w1 = 0
     ");
 }
 
@@ -604,7 +604,7 @@ fn slice_pop_back_positive_length_not_affected_by_predicate() {
 }
 
 #[test]
-fn slice_pop_back_zero_length_not_affected_by_predicate() {
+fn slice_pop_back_zero_length_affected_by_predicate() {
     let src_side_effects = "
     acir(inline) predicate_pure fn main f0 {
       b0(v0: u32, v1: u1):
@@ -625,7 +625,7 @@ fn slice_pop_back_zero_length_not_affected_by_predicate() {
 
     let program_side_effects = ssa_to_acir_program(src_side_effects);
     let program_no_side_effects = ssa_to_acir_program(src_no_side_effects);
-    assert_eq!(program_side_effects, program_no_side_effects);
+    assert_ne!(program_side_effects, program_no_side_effects);
 }
 
 #[test]
@@ -657,6 +657,62 @@ fn slice_pop_back_unknown_length_affected_by_predicate() {
     assert_ne!(program_side_effects, program_no_side_effects);
 }
 
+#[test]
+fn slice_pop_back_empty_slice_with_unknown_length_from_previous_pop() {
+    let src = "
+    acir(inline) predicate_pure fn main f0 {
+      b0(v0: [u32; 1], v1: u32, v2: u32):
+        v4, v5 = call as_slice(v0) -> (u32, [u32])
+        v7 = eq v1, u32 3
+        v8 = not v7
+        enable_side_effects v8
+        v11, v12, v13 = call slice_pop_back(u32 1, v5) -> (u32, [u32], u32)
+        v14, v15, v16 = call slice_pop_back(v11, v12) -> (u32, [u32], u32)
+        v17 = cast v8 as u32
+        v18 = unchecked_mul v16, v17
+        v19 = unchecked_mul v2, v17
+        constrain v18 == v19
+        v20 = unchecked_mul v14, v17
+        constrain v20 == v17
+        enable_side_effects u1 1
+        return
+    }
+    ";
+    let program = ssa_to_acir_program(src);
+
+    // We read the element for the first pop back into w6
+    // However, by the second pop back we are working with an empty slice, thus
+    // we simply assert that the side effects predicate is equal to zero.
+    // w1 is being checked whether it is equal to `3`.
+    assert_circuit_snapshot!(program, @r"
+    func 0
+    private parameters: [w0, w1, w2]
+    public parameters: []
+    return values: []
+    BLACKBOX::RANGE input: w0, bits: 32
+    BLACKBOX::RANGE input: w1, bits: 32
+    BLACKBOX::RANGE input: w2, bits: 32
+    INIT b1 = [w0]
+    BRILLIG CALL func: 0, inputs: [w1 - 3], outputs: [w3]
+    ASSERT w4 = -w1*w3 + 3*w3 + 1
+    ASSERT 0 = w1*w4 - 3*w4
+    ASSERT w5 = 0
+    READ w6 = b1[w5]
+    ASSERT w4 = 1
+
+    unconstrained func 0: directive_invert
+    0: @21 = const u32 1
+    1: @20 = const u32 0
+    2: @0 = calldata copy [@20; @21]
+    3: @2 = const field 0
+    4: @3 = field eq @0, @2
+    5: jump if @3 to 8
+    6: @1 = const field 1
+    7: @0 = field field_div @1, @0
+    8: stop &[@20; @21]
+    ");
+}
+
 #[test]
 fn slice_pop_front_not_affected_by_predicate() {
     let src_side_effects = "

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -191,14 +191,13 @@ impl Intrinsic {
             Intrinsic::ToBits(_) | Intrinsic::ToRadix(_) => true,
 
             // These imply a check that the slice is non-empty and should fail otherwise.
-            Intrinsic::SlicePopBack | Intrinsic::SlicePopFront | Intrinsic::SliceRemove => true,
+            Intrinsic::SlicePopBack | Intrinsic::SlicePopFront | Intrinsic::SliceRemove | Intrinsic::SliceInsert => true,
 
             Intrinsic::ArrayLen
             | Intrinsic::ArrayAsStrUnchecked
             | Intrinsic::AsSlice
             | Intrinsic::SlicePushBack
             | Intrinsic::SlicePushFront
-            | Intrinsic::SliceInsert
             | Intrinsic::StrAsBytes
             | Intrinsic::IsUnconstrained
             | Intrinsic::DerivePedersenGenerators
@@ -225,9 +224,11 @@ impl Intrinsic {
             Intrinsic::BlackBox(func) if func.has_side_effects() => Purity::PureWithPredicate,
 
             // Operations that remove items from a slice don't modify the slice, they just assert it's non-empty.
-            Intrinsic::SlicePopBack | Intrinsic::SlicePopFront | Intrinsic::SliceRemove => {
-                Purity::PureWithPredicate
-            }
+            // Slice insert also reads from its input slice, thus needing to assert that it is non-empty.
+            Intrinsic::SlicePopBack
+            | Intrinsic::SlicePopFront
+            | Intrinsic::SliceRemove
+            | Intrinsic::SliceInsert => Purity::PureWithPredicate,
 
             Intrinsic::AssertConstant
             | Intrinsic::StaticAssert

test_programs/execution_failure/slice_insert_oob/Nargo.toml

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

test_programs/execution_failure/slice_insert_oob/Prover.toml

@@ -0,0 +1,4 @@
+dummy_arr = [5]
+dummy_val = 2
+x = 0
+y = 3

test_programs/execution_failure/slice_insert_oob/src/main.nr

@@ -0,0 +1,14 @@
+fn main(dummy_val: u32, dummy_arr: [u32; 1], x: u32, y: u32) {
+    let mut slice = dummy_arr.as_slice();
+    if x == 3 {
+        slice = slice.push_back(dummy_val);
+    } else {
+        // This else branch executes when x != 3
+        // Should fail - inserting into empty slice with active predicate
+        let (slice, _) = slice.remove(0);
+        let s2 = slice.insert(0, 5);
+        assert_eq(x, y);
+        assert_eq(s2.len(), 1);
+    }
+    assert_eq(slice.len(), 2);
+}

test_programs/execution_failure/slice_pop_back_oob/Nargo.toml

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

test_programs/execution_failure/slice_pop_back_oob/Prover.toml

@@ -0,0 +1,4 @@
+dummy_arr = [5]
+dummy_val = 2
+x = 0
+y = 3

test_programs/execution_failure/slice_pop_back_oob/src/main.nr

@@ -0,0 +1,14 @@
+fn main(dummy_val: u32, dummy_arr: [u32; 1], x: u32, y: u32) {
+    let mut slice = dummy_arr.as_slice();
+    if x == 3 {
+        slice = slice.push_back(dummy_val);
+    } else {
+        // This else branch executes when x != 3
+        // Should fail - popping from empty slice with active predicate
+        let (slice, _) = slice.pop_back();
+        let (s2, x) = slice.pop_back();
+        assert_eq(x, y);
+        assert_eq(s2.len(), 1);
+    }
+    assert_eq(slice.len(), 2);
+}

test_programs/execution_failure/slice_pop_front_oob/Nargo.toml

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

test_programs/execution_failure/slice_pop_front_oob/Prover.toml

@@ -0,0 +1,4 @@
+dummy_arr = [5]
+dummy_val = 2
+x = 0
+y = 3

test_programs/execution_failure/slice_pop_front_oob/src/main.nr

@@ -0,0 +1,14 @@
+fn main(dummy_val: u32, dummy_arr: [u32; 1], x: u32, y: u32) {
+    let mut slice = dummy_arr.as_slice();
+    if x == 3 {
+        slice = slice.push_back(dummy_val);
+    } else {
+        // This else branch executes when x != 3
+        // Should fail - popping from empty slice with active predicate
+        let (_, slice) = slice.pop_front();
+        let (x, s2) = slice.pop_front();
+        assert_eq(x, y);
+        assert_eq(s2.len(), 1);
+    }
+    assert_eq(slice.len(), 2);
+}

test_programs/execution_failure/slice_remove_oob/Nargo.toml

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

test_programs/execution_failure/slice_remove_oob/Prover.toml

@@ -0,0 +1,4 @@
+dummy_arr = [5]
+dummy_val = 2
+x = 0
+y = 3