Skip to content

fix(acir): guard empty vector pop/remove on the semantic length - #13501

Open
asterite wants to merge 1 commit into
masterfrom
ab/fix-empty-vector-pop-remove-acir
Open

fix(acir): guard empty vector pop/remove on the semantic length#13501
asterite wants to merge 1 commit into
masterfrom
ab/fix-empty-vector-pop-remove-acir

Conversation

@asterite

@asterite asterite commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a compiler crash (and latent miscompilation) when a pop_back/pop_front/remove intrinsic is applied to a vector whose semantic length is zero but whose backing store is not.

acir_gen decided emptiness with has_zero_length, which inspects the backing store. But a slice's backing store can be non-empty while its semantic length is zero — merging two branch arms of unequal length pads the shorter arm. In that state the empty-vector arm was skipped and the new length was computed as 0 - 1 = p - 1 in the field; convert_vector_push_back then aborted the compiler in FieldElement::to_u128 ("field element too large for u128").

The zero length is not always visible in the SSA. An empty-vector assertion can collapse the side-effects predicate to a constant zero during ACIR generation, and the predicate multiplication then folds a live vector's length to zero while its SSA value stays dynamic — so no SSA pass can see it. The new vector_length_is_known_zero helper checks both the SSA constant and the folded ACIR expression, and the three converters take the empty arm when either the backing store or the semantic length is known to be zero.

Changes

  • convert_vector_pop_back / convert_vector_pop_front / convert_vector_remove now key their empty check on the semantic length as well as the backing store, via the new vector_length_is_known_zero helper.

Testing

  • New regression program vector_ops_collapsed_predicate reproduces the predicate-collapse crash end to end; it panicked the compiler before this change and now compiles and executes across the ACIR, Brillig, and interpreter variants.
  • The active (predicate-enabled) empty case still fails at runtime with Failed assertion rather than returning padding — soundness preserved.
  • All existing noirc_evaluator vector tests pass; acir_vs_brillig, min_vs_full, orig_vs_morph, and comptime_vs_brillig_nargo AST-fuzzer targets pass.

Status

Draft: opening for review of the approach. The Brillig update_vector_length wrapping-Sub and the SSA interpreter's vector_remove guard are related but separate surfaces (source programs reach them only through their own ssa_gen checks); this PR scopes to the ACIR generation path.

🤖 Generated with Claude Code

@asterite
asterite force-pushed the ab/fix-empty-vector-pop-remove-acir branch from cea0420 to d1e1318 Compare August 10, 2026 20:39
`convert_vector_pop_back`, `convert_vector_pop_front` and
`convert_vector_remove` decided whether the vector was empty with
`has_zero_length`, which inspects the backing store. A slice's backing
store can be non-empty while its semantic length is zero: merging branch
arms of unequal length pads the shorter arm. In that state the empty arm
was skipped and `length - 1` was computed as `0 - 1 = p - 1` in the field,
which `convert_vector_push_back` then aborted on in `to_u128`.

The zero length is not always visible in the SSA: an empty-vector
assertion can collapse the side-effects predicate to a constant zero
during ACIR generation, and that folds a live vector's length to zero via
the predicate multiplication while the SSA value stays dynamic. So the new
`vector_length_is_known_zero` checks both the SSA constant and the folded
ACIR expression, and the three converters take the empty arm when either
the backing store or the semantic length is known to be zero.

The regression program `vector_ops_collapsed_predicate` exercises the
predicate-collapse route end to end: it panicked the compiler before this
change and now compiles and executes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@asterite
asterite force-pushed the ab/fix-empty-vector-pop-remove-acir branch from d1e1318 to ce6f9be Compare August 10, 2026 20:56
@asterite
asterite marked this pull request as ready for review August 11, 2026 12:13
@asterite
asterite requested a review from TomAFrench August 11, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant