From 3099964db10071841fc72de5d8e52a325265b9e6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 6 Jan 2026 14:10:06 -0300 Subject: [PATCH 1/5] fix(ACIR): check vector length is not zero before pop_front --- .../src/acir/call/intrinsics/vector_ops.rs | 16 +++++++++++++++- .../Nargo.toml | 7 +++++++ .../Prover.toml | 2 ++ .../src/main.nr | 14 ++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 test_programs/execution_failure/conditional_pop_front_from_empty_vector/Nargo.toml create mode 100644 test_programs/execution_failure/conditional_pop_front_from_empty_vector/Prover.toml create mode 100644 test_programs/execution_failure/conditional_pop_front_from_empty_vector/src/main.nr diff --git a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs index a8ba751b8c5..208702d58fa 100644 --- a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs +++ b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs @@ -359,10 +359,24 @@ impl Context<'_> { let vector_length = self.convert_value(arguments[0], dfg).into_var()?; let vector_contents = arguments[1]; + // Check that the vector length is not zero + let zero = self.acir_context.add_constant(FieldElement::zero()); + let assert_message = self.acir_context.generate_assertion_message_payload( + "Attempt to pop_front from an empty vector".to_string(), + ); + self.acir_context.assert_neq_var( + vector_length, + zero, + self.current_side_effects_enabled_var, + Some(assert_message), + )?; + let vector_typ = dfg.type_of_value(vector_contents); let block_id = self.ensure_array_is_initialized(vector_contents, dfg)?; - // Check if we're trying to pop from an empty vector + // Check if we're trying to pop from a known empty vector. + // Note that this is different from the previous check as this only considers static + // vectors and arrays, not dynamic ones where the length is not known at compile time. if self.has_zero_length(vector_contents, dfg) { // Make sure this code is disabled, or fail with "Index out of bounds". let msg = "cannot pop from a vector with length 0".to_string(); diff --git a/test_programs/execution_failure/conditional_pop_front_from_empty_vector/Nargo.toml b/test_programs/execution_failure/conditional_pop_front_from_empty_vector/Nargo.toml new file mode 100644 index 00000000000..6e8e5dbd098 --- /dev/null +++ b/test_programs/execution_failure/conditional_pop_front_from_empty_vector/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "conditional_pop_front_from_empty_vector" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/test_programs/execution_failure/conditional_pop_front_from_empty_vector/Prover.toml b/test_programs/execution_failure/conditional_pop_front_from_empty_vector/Prover.toml new file mode 100644 index 00000000000..c3aeb7874fd --- /dev/null +++ b/test_programs/execution_failure/conditional_pop_front_from_empty_vector/Prover.toml @@ -0,0 +1,2 @@ +input = [1] +b = true diff --git a/test_programs/execution_failure/conditional_pop_front_from_empty_vector/src/main.nr b/test_programs/execution_failure/conditional_pop_front_from_empty_vector/src/main.nr new file mode 100644 index 00000000000..0813b9c5962 --- /dev/null +++ b/test_programs/execution_failure/conditional_pop_front_from_empty_vector/src/main.nr @@ -0,0 +1,14 @@ +pub fn main(input: [u32; 1], b: bool) -> pub u32 { + let mut s = input.as_vector(); + + if (b) { + let (_, new_slice) = s.pop_front(); + s = new_slice; + } else { + s = s.push_front(5); + } + + let (ret, _) = s.pop_front(); + + ret +} From da67f30e752f4f58e1974af96ffe9d2f885aabb1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 6 Jan 2026 16:33:02 -0300 Subject: [PATCH 2/5] Add regression tests for the other vector ops --- .../Nargo.toml | 7 +++++++ .../Prover.toml | 2 ++ .../src/main.nr | 14 ++++++++++++++ .../Nargo.toml | 7 +++++++ .../Prover.toml | 2 ++ .../src/main.nr | 14 ++++++++++++++ 6 files changed, 46 insertions(+) create mode 100644 test_programs/execution_failure/conditional_pop_back_from_empty_vector/Nargo.toml create mode 100644 test_programs/execution_failure/conditional_pop_back_from_empty_vector/Prover.toml create mode 100644 test_programs/execution_failure/conditional_pop_back_from_empty_vector/src/main.nr create mode 100644 test_programs/execution_failure/conditional_remove_from_empty_vector/Nargo.toml create mode 100644 test_programs/execution_failure/conditional_remove_from_empty_vector/Prover.toml create mode 100644 test_programs/execution_failure/conditional_remove_from_empty_vector/src/main.nr diff --git a/test_programs/execution_failure/conditional_pop_back_from_empty_vector/Nargo.toml b/test_programs/execution_failure/conditional_pop_back_from_empty_vector/Nargo.toml new file mode 100644 index 00000000000..be43c7c09cb --- /dev/null +++ b/test_programs/execution_failure/conditional_pop_back_from_empty_vector/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "conditional_pop_back_from_empty_vector" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/test_programs/execution_failure/conditional_pop_back_from_empty_vector/Prover.toml b/test_programs/execution_failure/conditional_pop_back_from_empty_vector/Prover.toml new file mode 100644 index 00000000000..c3aeb7874fd --- /dev/null +++ b/test_programs/execution_failure/conditional_pop_back_from_empty_vector/Prover.toml @@ -0,0 +1,2 @@ +input = [1] +b = true diff --git a/test_programs/execution_failure/conditional_pop_back_from_empty_vector/src/main.nr b/test_programs/execution_failure/conditional_pop_back_from_empty_vector/src/main.nr new file mode 100644 index 00000000000..bf02b0d7002 --- /dev/null +++ b/test_programs/execution_failure/conditional_pop_back_from_empty_vector/src/main.nr @@ -0,0 +1,14 @@ +pub fn main(input: [u32; 1], b: bool) -> pub u32 { + let mut s = input.as_vector(); + + if (b) { + let (new_slice, _) = s.pop_back(); + s = new_slice; + } else { + s = s.push_front(5); + } + + let (_, ret) = s.pop_back(); + + ret +} diff --git a/test_programs/execution_failure/conditional_remove_from_empty_vector/Nargo.toml b/test_programs/execution_failure/conditional_remove_from_empty_vector/Nargo.toml new file mode 100644 index 00000000000..9e4d4e129dc --- /dev/null +++ b/test_programs/execution_failure/conditional_remove_from_empty_vector/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "conditional_remove_from_empty_vector" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/test_programs/execution_failure/conditional_remove_from_empty_vector/Prover.toml b/test_programs/execution_failure/conditional_remove_from_empty_vector/Prover.toml new file mode 100644 index 00000000000..c3aeb7874fd --- /dev/null +++ b/test_programs/execution_failure/conditional_remove_from_empty_vector/Prover.toml @@ -0,0 +1,2 @@ +input = [1] +b = true diff --git a/test_programs/execution_failure/conditional_remove_from_empty_vector/src/main.nr b/test_programs/execution_failure/conditional_remove_from_empty_vector/src/main.nr new file mode 100644 index 00000000000..43a99b83fe7 --- /dev/null +++ b/test_programs/execution_failure/conditional_remove_from_empty_vector/src/main.nr @@ -0,0 +1,14 @@ +pub fn main(input: [u32; 1], b: bool) -> pub u32 { + let mut s = input.as_vector(); + + if (b) { + let (new_slice, _) = s.remove(0); + s = new_slice; + } else { + s = s.push_front(5); + } + + let (_, ret) = s.remove(0); + + ret +} From b8f909fc4563989d958c3b309d716c5cd52ac828 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 7 Jan 2026 11:39:46 -0300 Subject: [PATCH 3/5] Move check to SSA --- .../src/acir/call/intrinsics/vector_ops.rs | 14 -------------- compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 14 ++------------ 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs index 208702d58fa..5de584ea688 100644 --- a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs +++ b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs @@ -359,24 +359,10 @@ impl Context<'_> { let vector_length = self.convert_value(arguments[0], dfg).into_var()?; let vector_contents = arguments[1]; - // Check that the vector length is not zero - let zero = self.acir_context.add_constant(FieldElement::zero()); - let assert_message = self.acir_context.generate_assertion_message_payload( - "Attempt to pop_front from an empty vector".to_string(), - ); - self.acir_context.assert_neq_var( - vector_length, - zero, - self.current_side_effects_enabled_var, - Some(assert_message), - )?; - let vector_typ = dfg.type_of_value(vector_contents); let block_id = self.ensure_array_is_initialized(vector_contents, dfg)?; // Check if we're trying to pop from a known empty vector. - // Note that this is different from the previous check as this only considers static - // vectors and arrays, not dynamic ones where the length is not known at compile time. if self.has_zero_length(vector_contents, dfg) { // Make sure this code is disabled, or fail with "Index out of bounds". let msg = "cannot pop from a vector with length 0".to_string(); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 22763a6f5da..ec6a4f25c79 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -1122,18 +1122,8 @@ impl FunctionContext<'_> { Intrinsic::VectorRemove => { self.codegen_access_check(arguments[2], arguments[0]); } - Intrinsic::VectorPopFront | Intrinsic::VectorPopBack - if self.builder.current_function.runtime().is_brillig() => - { - // We need to put in a constraint to protect against accessing empty vectors: - // * In Brillig this is essential, otherwise it would read an unrelated piece of memory. - // * In ACIR we do have protection against reading empty vectors (it returns "Index Out of Bounds"), so we don't get invalid reads. - // The memory operations in ACIR ignore the side effect variables, so even if we added a constraint here, it could still fail - // when it inevitably tries to read from an empty vector anyway. We have to handle that by removing operations which are known - // to fail and replace them with conditional constraints that do take the side effect into account. - // By doing this in the SSA we might be able to optimize this away later. - let zero = - self.builder.numeric_constant(0u32, NumericType::Unsigned { bit_size: 32 }); + Intrinsic::VectorPopFront | Intrinsic::VectorPopBack => { + let zero = self.builder.numeric_constant(0u32, NumericType::unsigned(32)); self.codegen_access_check(zero, arguments[0]); } _ => { From 3240f52d621748d48d3d866bc5209de79ff07726 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 7 Jan 2026 16:29:33 -0300 Subject: [PATCH 4/5] Revert "Move check to SSA" This reverts commit b8f909fc4563989d958c3b309d716c5cd52ac828. --- .../src/acir/call/intrinsics/vector_ops.rs | 14 ++++++++++++++ compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs | 14 ++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs index 5de584ea688..208702d58fa 100644 --- a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs +++ b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs @@ -359,10 +359,24 @@ impl Context<'_> { let vector_length = self.convert_value(arguments[0], dfg).into_var()?; let vector_contents = arguments[1]; + // Check that the vector length is not zero + let zero = self.acir_context.add_constant(FieldElement::zero()); + let assert_message = self.acir_context.generate_assertion_message_payload( + "Attempt to pop_front from an empty vector".to_string(), + ); + self.acir_context.assert_neq_var( + vector_length, + zero, + self.current_side_effects_enabled_var, + Some(assert_message), + )?; + let vector_typ = dfg.type_of_value(vector_contents); let block_id = self.ensure_array_is_initialized(vector_contents, dfg)?; // Check if we're trying to pop from a known empty vector. + // Note that this is different from the previous check as this only considers static + // vectors and arrays, not dynamic ones where the length is not known at compile time. if self.has_zero_length(vector_contents, dfg) { // Make sure this code is disabled, or fail with "Index out of bounds". let msg = "cannot pop from a vector with length 0".to_string(); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index ec6a4f25c79..22763a6f5da 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -1122,8 +1122,18 @@ impl FunctionContext<'_> { Intrinsic::VectorRemove => { self.codegen_access_check(arguments[2], arguments[0]); } - Intrinsic::VectorPopFront | Intrinsic::VectorPopBack => { - let zero = self.builder.numeric_constant(0u32, NumericType::unsigned(32)); + Intrinsic::VectorPopFront | Intrinsic::VectorPopBack + if self.builder.current_function.runtime().is_brillig() => + { + // We need to put in a constraint to protect against accessing empty vectors: + // * In Brillig this is essential, otherwise it would read an unrelated piece of memory. + // * In ACIR we do have protection against reading empty vectors (it returns "Index Out of Bounds"), so we don't get invalid reads. + // The memory operations in ACIR ignore the side effect variables, so even if we added a constraint here, it could still fail + // when it inevitably tries to read from an empty vector anyway. We have to handle that by removing operations which are known + // to fail and replace them with conditional constraints that do take the side effect into account. + // By doing this in the SSA we might be able to optimize this away later. + let zero = + self.builder.numeric_constant(0u32, NumericType::Unsigned { bit_size: 32 }); self.codegen_access_check(zero, arguments[0]); } _ => { From 3ffde3c1aacf1a4f4888866b2466b25c476f3e77 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 8 Jan 2026 09:39:01 -0300 Subject: [PATCH 5/5] Perform runtime check after comptime check --- .../src/acir/call/intrinsics/vector_ops.rs | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs index 208702d58fa..422fa140dc9 100644 --- a/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs +++ b/compiler/noirc_evaluator/src/acir/call/intrinsics/vector_ops.rs @@ -359,24 +359,10 @@ impl Context<'_> { let vector_length = self.convert_value(arguments[0], dfg).into_var()?; let vector_contents = arguments[1]; - // Check that the vector length is not zero - let zero = self.acir_context.add_constant(FieldElement::zero()); - let assert_message = self.acir_context.generate_assertion_message_payload( - "Attempt to pop_front from an empty vector".to_string(), - ); - self.acir_context.assert_neq_var( - vector_length, - zero, - self.current_side_effects_enabled_var, - Some(assert_message), - )?; - let vector_typ = dfg.type_of_value(vector_contents); let block_id = self.ensure_array_is_initialized(vector_contents, dfg)?; // Check if we're trying to pop from a known empty vector. - // Note that this is different from the previous check as this only considers static - // vectors and arrays, not dynamic ones where the length is not known at compile time. if self.has_zero_length(vector_contents, dfg) { // Make sure this code is disabled, or fail with "Index out of bounds". let msg = "cannot pop from a vector with length 0".to_string(); @@ -402,6 +388,19 @@ impl Context<'_> { return Ok(results); } + // Check that the vector length is not zero. + // This is different from the previous check as this is a runtime check. + let zero = self.acir_context.add_constant(FieldElement::zero()); + let assert_message = self.acir_context.generate_assertion_message_payload( + "Attempt to pop_front from an empty vector".to_string(), + ); + self.acir_context.assert_neq_var( + vector_length, + zero, + self.current_side_effects_enabled_var, + Some(assert_message), + )?; + let one = self.acir_context.add_constant(FieldElement::one()); let new_vector_length = self.acir_context.sub_var(vector_length, one)?;