Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions compiler/noirc_evaluator/src/acir/acir_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,23 @@ impl<F: AcirField> AcirContext<F> {
Ok(())
}

/// Assert that an [AcirVar] equals zero, or fail with a message.
pub(crate) fn assert_zero_var(
&mut self,
var: AcirVar,
msg: String,
) -> Result<(), RuntimeError> {
let msg = self.generate_assertion_message_payload(msg);
let zero = self.add_constant(F::zero());
self.assert_eq_var(var, zero, Some(msg))
}

/// Add an always-fail assertion with a message.
pub(crate) fn assert_always_fail(&mut self, msg: String) -> Result<(), RuntimeError> {
let one = self.add_constant(F::one());
self.assert_zero_var(one, msg)
}

pub(crate) fn values_to_expressions_or_memory(
&self,
values: &[AcirValue],
Expand Down Expand Up @@ -860,8 +877,7 @@ impl<F: AcirField> AcirContext<F> {
let msg = format!(
"attempted to divide by constant larger than operand type: {max_rhs_bits} > {bit_size}"
);
let msg = self.generate_assertion_message_payload(msg);
self.assert_eq_var(zero, one, Some(msg))?;
self.assert_always_fail(msg)?;
return Ok((zero, zero));
}
(bit_size - max_rhs_bits + 1, max_rhs_bits)
Expand Down
4 changes: 1 addition & 3 deletions compiler/noirc_evaluator/src/acir/arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,9 +265,7 @@ impl Context<'_> {
}
// Make sure this code is disabled, or fail with "Index out of bounds".
let msg = "Index out of bounds, array has size 0".to_string();
let msg = self.acir_context.generate_assertion_message_payload(msg);
let zero = self.acir_context.add_constant(FieldElement::zero());
self.acir_context.assert_eq_var(self.current_side_effects_enabled_var, zero, Some(msg))?;
self.acir_context.assert_zero_var(self.current_side_effects_enabled_var, msg)?;
Ok(true)
}

Expand Down
112 changes: 75 additions & 37 deletions compiler/noirc_evaluator/src/acir/call/intrinsics/slice_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,18 +235,57 @@ impl Context<'_> {
dfg: &DataFlowGraph,
result_ids: &[ValueId],
) -> Result<Vec<AcirValue>, RuntimeError> {
let slice_length = self.convert_value(arguments[0], dfg).into_var()?;
let slice_length_var = arguments[0];
let slice_contents = arguments[1];

let slice_value = self.convert_value(slice_length_var, dfg);
let slice_length = slice_value.clone().into_var()?;
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())?;

// Fill the result with default values.
let mut results = Vec::with_capacity(result_ids.len());

// The results shall be: [new len, new slice, ...popped]
results.push(slice_value);
results.push(slice);

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

// For unknown length under a side effect variable, we want to multiply with the side effect variable
// to ensure we don't end up trying to look up an item at index -1, when the semantic length is 0.
let is_unknown_length = dfg.get_numeric_constant(slice_length_var).is_none();

let one = self.acir_context.add_constant(FieldElement::one());
let new_slice_length = self.acir_context.sub_var(slice_length, one)?;
let mut new_slice_length = self.acir_context.sub_var(slice_length, one)?;

if is_unknown_length {
new_slice_length = self
.acir_context
.mul_var(new_slice_length, self.current_side_effects_enabled_var)?;
}

// For a pop back operation we want to fetch from the `length - 1` as this is the
// last valid index that can be accessed in a slice. After the pop back operation
// the elements stored at that index will no longer be able to be accessed.
let mut var_index = new_slice_length;

let block_id = self.ensure_array_is_initialized(slice_contents, dfg)?;

let slice_type = dfg.type_of_value(slice_contents);
let item_size = slice_type.element_size();
let item_size = self.acir_context.add_constant(item_size);
Expand All @@ -258,7 +297,6 @@ impl Context<'_> {
popped_elements.push(elem);
}

let slice = self.convert_value(slice_contents, dfg);
let mut new_slice = self.read_array_with_type(slice, &slice_type)?;
for _ in 0..popped_elements.len() {
new_slice.pop_back();
Expand Down Expand Up @@ -358,33 +396,33 @@ impl Context<'_> {
///
/// # Arguments
///
/// * `arguments[0]` - Current slice length
/// * `arguments[1]` - Slice contents
/// * `arguments[2]` - Insert index (logical element index, not flattened)
/// * `arguments[3..]` - Elements to insert
/// * `result_ids[0]` - Updated slice length
/// * `result_ids[1]` - Updated slice contents
/// * `arguments[0]` - Current slice length
/// * `arguments[1]` - Slice contents
/// * `arguments[2]` - Insert index (logical element index, not flattened)
/// * `arguments[3..]` - Elements to insert
/// * `result_ids[0]` - Updated slice length
/// * `result_ids[1]` - Updated slice contents
///
/// # Returns
///
/// A vector of [AcirValue]s containing:
/// 1. Updated slice length (incremented by one)
/// 2. Updated slice contents with the new elements inserted at the given index
/// 1. Updated slice length (incremented by one)
/// 2. Updated slice contents with the new elements inserted at the given index
///
/// # Design
///
/// Slices are represented in **flattened form** in memory. Inserting requires
/// shifting a contiguous region of elements upward to make room for the new ones:
///
/// 1. Compute the flattened insert index:
/// - Multiply the logical insert index by the element size.
/// 1. Compute the flattened insert index:
/// - Multiply the logical insert index by the element size.
/// - Adjust for non-homogenous structures via [Self::get_flattened_index].
/// 2. Flatten the new elements (`flattened_elements`)
/// 3. For each position in the result slice:
/// - If below the insert index, copy from the original slice.
/// - 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.
/// 3. For each position in the result slice:
/// - If below the insert index, copy from the original slice.
/// - 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.
pub(super) fn convert_slice_insert(
&mut self,
arguments: &[ValueId],
Expand Down Expand Up @@ -546,40 +584,40 @@ impl Context<'_> {
///
/// # Arguments
///
/// * `arguments[0]` - Current slice length
/// * `arguments[1]` - Slice contents
/// * `arguments[2]` - Removal index (logical element index, not flattened)
/// * `result_ids[0]` - Updated slice length
/// * `result_ids[1]` - Updated slice contents
/// * `result_ids[2..]` - Locations for the removed elements
/// * `arguments[0]` - Current slice length
/// * `arguments[1]` - Slice contents
/// * `arguments[2]` - Removal index (logical element index, not flattened)
/// * `result_ids[0]` - Updated slice length
/// * `result_ids[1]` - Updated slice contents
/// * `result_ids[2..]` - Locations for the removed elements
///
/// # Returns
///
/// A vector of [AcirValue]s containing:
/// 1. Updated slice length (decremented by one)
/// 2. Updated slice contents with the target elements removed
/// 3. The removed elements, in order
/// 1. Updated slice length (decremented by one)
/// 2. Updated slice contents with the target elements removed
/// 3. The removed elements, in order
///
/// # Design
///
/// Slices are stored in **flattened form** in memory. Removing requires
/// shifting a contiguous region of elements downward to overwrite the removed values:
///
/// 1. Compute the flattened remove index:
/// - Multiply the logical remove index by the element size.
/// 1. Compute the flattened remove index:
/// - Multiply the logical remove index by the element size.
/// - Adjust for non-homogenous structures via [Self::get_flattened_index].
/// 2. Read out the element(s) to be removed:
/// 2. Read out the element(s) to be removed:
/// - Iterate over `result_ids[2..(2 + element_size)]`
/// - `element_size` refers to the result of [crate::ssa::ir::types::Type::element_size].
/// - Use these IDs to fetch the appropriate type information for the values to remove and drive `array_get_value`.
/// While extracting the values to remove we compute the total `popped_elements_size` (the flattened width of the removed data).
/// 3. For each index in the result slice:
/// - If the index is below the remove index, copy directly.
/// While extracting the values to remove we compute the total `popped_elements_size` (the flattened width of the removed data).
/// 3. For each index in the result slice:
/// - If the index is below the remove index, copy directly.
/// - If the index is at or beyond the removed element, fetch the value from `index + popped_elements_size`
/// in the original slice and write it to the current index.
/// in the original slice and write it to the current index.
/// - 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.
/// 4. Initialize a new memory block for the resulting slice, ensuring its type information is preserved.
pub(super) fn convert_slice_remove(
&mut self,
arguments: &[ValueId],
Expand Down
114 changes: 113 additions & 1 deletion compiler/noirc_evaluator/src/acir/tests/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,64 @@ fn slice_pop_back() {
");
}

#[test]
fn slice_pop_back_zero_length() {
let src = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
v7 = make_array [] : [Field]
enable_side_effects v1
v9, v10, v11 = call slice_pop_back(u32 0, v7) -> (u32, [Field], Field)
return
}
";
let program = ssa_to_acir_program(src);

// 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.
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
");
}

#[test]
fn slice_pop_back_unknown_length() {
let src = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
v5 = cast v1 as u32
v6 = unchecked_mul u32 1, v5
v7 = make_array [Field 1]: [Field]
enable_side_effects v1
v9, v10, v11 = call slice_pop_back(v6, v7) -> (u32, [Field], Field)
return
}
";
let program = ssa_to_acir_program(src);

// In practice the multiplication will come from flattening, resulting in a slice
// that can have a semantic length of 0, but only when the side effects are disabled;
// popping should not fail in such a scenario.
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 w2 = 1
INIT b0 = [w2]
ASSERT w3 = w1*w1 - w1
READ w4 = b0[w3]
");
}

#[test]
fn slice_pop_front() {
let src = "
Expand Down Expand Up @@ -444,7 +502,7 @@ fn slice_push_front_not_affected_by_predicate() {
}

#[test]
fn slice_pop_back_not_affected_by_predicate() {
fn slice_pop_back_positive_length_not_affected_by_predicate() {
let src_side_effects = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
Expand Down Expand Up @@ -472,6 +530,60 @@ fn slice_pop_back_not_affected_by_predicate() {
assert_eq!(program_side_effects, program_no_side_effects);
}

#[test]
fn slice_pop_back_zero_length_not_affected_by_predicate() {
let src_side_effects = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
v7 = make_array [] : [Field]
enable_side_effects v1
v9, v10, v11 = call slice_pop_back(u32 0, v7) -> (u32, [Field], Field)
return
}
";
let src_no_side_effects = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
v7 = make_array [] : [Field]
v9, v10, v11 = call slice_pop_back(u32 0, v7) -> (u32, [Field], Field)
return
}
";

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

#[test]
fn slice_pop_back_unknown_length_affected_by_predicate() {
let src_side_effects = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
v4 = cast v1 as u32
v5 = unchecked_mul u32 1, v4
v7 = make_array [Field 1] : [Field]
enable_side_effects v1
v9, v10, v11 = call slice_pop_back(v5, v7) -> (u32, [Field], Field)
return
}
";
let src_no_side_effects = "
acir(inline) predicate_pure fn main f0 {
b0(v0: u32, v1: u1):
v4 = cast v1 as u32
v5 = unchecked_mul u32 1, v4
v7 = make_array [Field 1] : [Field]
v9, v10, v11 = call slice_pop_back(v5, v7) -> (u32, [Field], Field)
return
}
";

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_ne!(program_side_effects, program_no_side_effects);
}

#[test]
fn slice_pop_front_not_affected_by_predicate() {
let src_side_effects = "
Expand Down
6 changes: 6 additions & 0 deletions test_programs/execution_success/regression_10446/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_10446"
type = "bin"
authors = [""]

[dependencies]
2 changes: 2 additions & 0 deletions test_programs/execution_success/regression_10446/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
a = true
return = true
16 changes: 16 additions & 0 deletions test_programs/execution_success/regression_10446/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
fn main(a: bool) -> pub bool {
if !a {
let mut c: [str<2>] = &["IR"];
for _ in 0..2 {
c = if a {
let _ = c.pop_back();
&["QM", "HF"]
} else {
&["VD", "YH", "SE", "FB"]
};
}
false
} else {
true
}
}
Loading
Loading