fix(values): encode BoundedInt in its compact representation; deref boxed returns

Two remaining marshaling asymmetries:

- `Value::to_ptr` wrote a BoundedInt as the un-biased value in a 32-byte
  felt slot, while the native representation (and `from_ptr`) is the
  compact one: `value - lower` stored in `repr_bit_width()` bits. A
  BoundedInt nested in an aggregate (top-level arguments are still gated
  by the arch.rs panic, #1217) was both silently mis-decoded and
  overran its slot, corrupting neighboring elements —
  `Array<BoundedInt<3, 10>>` returned garbage.

- `parse_result` passed a returned `Box<T>`'s return-pointer slot
  straight to `from_ptr` without dereferencing it. Any function that
  uses a non-ZST builtin returns through a return pointer, so e.g.
  `fn(...) -> Box<felt252>` using pedersen decoded two raw heap
  addresses as the payload. Mirror the Nullable arm's deref.

Add `RangeExt::repr_encode`/`repr_decode` as the single implementation
of the compact encoding (felt-wrapping subtraction so negative lower
bounds round-trip) and use the decode side from both existing
duplicates (`Value::from_ptr` and the register path in `parse_result`).
The `to_ptr` arm now also validates against the type's range rather
than only the value's embedded one.

Fixes `test_to_ptr_bounded_int_valid`, which asserted the buggy 32-byte
encoding ([16, ...] instead of `value - lower` = [6, 0] in the 2-byte
9-bit representation), and its embedded range, which didn't match the
type's (`BoundedInt<10, 510>` parses to the range `[10, 511)`).

Adds a VM-vs-native test for a boxed return forced through the return
pointer, and round-trip unit tests for `Array<BoundedInt<3, 10>>`
(asserting the 1-byte biased element buffer) and a negative-lower
range.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: TomerStarkware

src/arch.rs

@@ -79,6 +79,9 @@ impl AbiArgument for ValueWithInfoWrapper<'_> {
                 abi.capacity.to_bytes(buffer)?;
             }
             (Value::BoundedInt { .. }, CoreTypeConcrete::BoundedInt(_)) => {
+                // TODO: implement top-level BoundedInt arguments on top of
+                // `RangeExt::repr_encode` (dispatch on `repr_bit_width()`: <=64 via the
+                // `u64` impl, <=128 via `u128`, wider by memory like `Felt`).
                 // See: https://github.com/starkware-libs/cairo_native/issues/1217
                 native_panic!("todo: implement AbiArgument for Value::BoundedInt case")
             }

src/executor.rs

@@ -42,8 +42,7 @@ use cairo_lang_sierra::{
     program_registry::ProgramRegistry,
 };
 use libc::c_void;
-use num_bigint::BigInt;
-use num_traits::One;
+use num_bigint::BigUint;
 use std::{alloc::Layout, arch::global_asm, ptr::NonNull};
 
 mod aot;
@@ -454,9 +453,14 @@ fn parse_result(
             registry,
         )?),
         CoreTypeConcrete::Box(info) => unsafe {
-            let ptr =
-                return_ptr.unwrap_or_else(|| NonNull::new_unchecked(ret_registers[0] as *mut ()));
-            let value = Value::from_ptr(ptr, &info.ty, registry)?;
+            // With a return pointer the returned value is the box's inline
+            // representation — a slot holding the payload pointer — so it must be
+            // dereferenced once. Without one, the register holds the payload
+            // pointer itself.
+            let ptr = return_ptr.map_or(ret_registers[0] as *mut (), |x| {
+                *x.cast::<*mut ()>().as_ref()
+            });
+            let value = Value::from_ptr(NonNull::new_unchecked(ptr), &info.ty, registry)?;
             Ok(value)
         },
         CoreTypeConcrete::EcPoint(_) | CoreTypeConcrete::EcState(_) => Ok(Value::from_ptr(
@@ -523,17 +527,14 @@ fn parse_result(
             match return_ptr {
                 Some(return_ptr) => Ok(Value::from_ptr(return_ptr, type_id, registry)?),
                 None => {
-                    let mut data = if info.range.repr_bit_width() <= 64 {
-                        BigInt::from(ret_registers[0])
+                    let raw = if info.range.repr_bit_width() <= 64 {
+                        BigUint::from(ret_registers[0])
                     } else {
-                        BigInt::from(((ret_registers[1] as u128) << 64) | ret_registers[0] as u128)
+                        BigUint::from(((ret_registers[1] as u128) << 64) | ret_registers[0] as u128)
                     };
 
-                    data &= (BigInt::one() << info.range.repr_bit_width()) - BigInt::one();
-                    data += &info.range.lower;
-
                     Ok(Value::BoundedInt {
-                        value: data.into(),
+                        value: info.range.repr_decode(raw).into(),
                         range: info.range.clone(),
                     })
                 }

src/utils/range_ext.rs

@@ -1,12 +1,21 @@
+use crate::utils::{get_integer_layout, PRIME};
 use cairo_lang_sierra::extensions::utils::Range;
 use num_bigint::{BigInt, BigUint, Sign};
-use num_traits::One;
+use num_traits::{Euclid, One};
 
 pub trait RangeExt {
     /// Width in bits when the offset is zero (aka. the natural representation).
     fn zero_based_bit_width(&self) -> u32;
     /// Width in bits when the offset is not necessarily zero (aka. the compact representation).
     fn repr_bit_width(&self) -> u32;
+    /// Encode a value into the compact representation: the stored bits are
+    /// `(value - lower) mod PRIME`, laid out little-endian over the full
+    /// `get_integer_layout(repr_bit_width())` size. Returns `None` when the
+    /// value is not within the range.
+    fn repr_encode(&self, value: &BigInt) -> Option<Vec<u8>>;
+    /// Decode raw stored bits back into the value they represent: mask to
+    /// `repr_bit_width()` bits and add `lower`.
+    fn repr_decode(&self, raw: BigUint) -> BigInt;
 }
 
 impl RangeExt for Range {
@@ -44,4 +53,24 @@ impl RangeExt for Range {
         // FIXME: Workaround for segfault in canonicalization (including LLVM 19).
         ((self.size() - BigInt::one()).bits() as u32).max(1)
     }
+
+    fn repr_encode(&self, value: &BigInt) -> Option<Vec<u8>> {
+        // The subtraction is felt arithmetic so that ranges with a negative
+        // lower bound round-trip: values are field elements in `[0, PRIME)`,
+        // and `repr_decode`'s addition wraps the same way.
+        let prime = BigInt::from_biguint(Sign::Plus, PRIME.clone());
+        let stored = (value - &self.lower).rem_euclid(&prime);
+        if stored >= self.size() {
+            return None;
+        }
+
+        let mut bytes = stored.magnitude().to_bytes_le();
+        bytes.resize(get_integer_layout(self.repr_bit_width()).size(), 0);
+        Some(bytes)
+    }
+
+    fn repr_decode(&self, raw: BigUint) -> BigInt {
+        let mask = (BigUint::one() << self.repr_bit_width()) - BigUint::one();
+        BigInt::from_biguint(Sign::Plus, raw & mask) + &self.lower
+    }
 }

src/values.rs

@@ -8,7 +8,7 @@ use crate::{
     runtime::FeltDict,
     starknet::{ArrayAbi, Secp256k1Point, Secp256r1Point},
     types::{ec_point, ec_state, TypeBuilder},
-    utils::{felt252_bigint, felt_from_slot, get_integer_layout, layout_repeat, RangeExt, PRIME},
+    utils::{felt252_bigint, felt_from_slot, get_integer_layout, layout_repeat, RangeExt},
 };
 use bumpalo::Bump;
 use cairo_lang_sierra::{
@@ -23,7 +23,7 @@ use cairo_lang_sierra::{
 };
 use educe::Educe;
 use num_bigint::{BigInt, BigUint, Sign};
-use num_traits::{Euclid, One};
+use num_traits::One;
 use starknet_types_core::{curve::ProjectivePoint, felt::Felt};
 use std::{
     alloc::Layout,
@@ -229,22 +229,35 @@ impl Value {
                         .into());
                     }
 
-                    let prime = BigInt::from_biguint(Sign::Plus, PRIME.clone());
-                    let lower = lower.rem_euclid(&prime);
-                    let upper = upper.rem_euclid(&prime);
+                    let range = match Self::resolve_type(ty, registry)? {
+                        CoreTypeConcrete::BoundedInt(info)
+                        | CoreTypeConcrete::BoundedIntGuarantee(info) => &info.range,
+                        _ => {
+                            return Err(Error::UnexpectedValue(format!(
+                                "expected value of type {:?} but got a bounded int",
+                                type_id.debug_name
+                            )))
+                        }
+                    };
 
-                    // Check if value is within the valid range
-                    if !(lower <= value && value < upper) {
-                        return Err(CompilerError::BoundedIntOutOfRange {
+                    // The native representation is the compact one: `value - lower`
+                    // stored in `repr_bit_width()` bits, which is what `from_ptr`
+                    // decodes and what compiled code expects.
+                    let data = range.repr_encode(&value).ok_or_else(|| {
+                        CompilerError::BoundedIntOutOfRange {
                             value: Box::new(value),
-                            range: Box::new((lower, upper)),
+                            range: Box::new((range.lower.clone(), range.upper.clone())),
                         }
-                        .into());
-                    }
+                    })?;
 
-                    let ptr = arena.alloc_layout(get_integer_layout(252)).cast();
-                    let data = felt252_bigint(value).to_bytes_le();
-                    ptr.cast::<[u8; 32]>().as_mut().copy_from_slice(&data);
+                    let ptr: NonNull<()> = arena
+                        .alloc_layout(get_integer_layout(range.repr_bit_width()))
+                        .cast();
+                    std::ptr::copy_nonoverlapping(
+                        data.as_ptr(),
+                        ptr.cast::<u8>().as_ptr(),
+                        data.len(),
+                    );
                     ptr
                 }
 
@@ -853,19 +866,13 @@ impl Value {
                 CoreTypeConcrete::Const(_) => native_panic!("implement const from_ptr"),
                 CoreTypeConcrete::BoundedInt(info)
                 | CoreTypeConcrete::BoundedIntGuarantee(info) => {
-                    let mut data = BigInt::from_biguint(
-                        Sign::Plus,
-                        BigUint::from_bytes_le(slice::from_raw_parts(
-                            ptr.cast::<u8>().as_ptr(),
-                            (info.range.repr_bit_width().next_multiple_of(8) >> 3) as usize,
-                        )),
-                    );
-
-                    data &= (BigInt::one() << info.range.repr_bit_width()) - BigInt::one();
-                    data += &info.range.lower;
+                    let raw = BigUint::from_bytes_le(slice::from_raw_parts(
+                        ptr.cast::<u8>().as_ptr(),
+                        (info.range.repr_bit_width().next_multiple_of(8) >> 3) as usize,
+                    ));
 
                     Self::BoundedInt {
-                        value: data.into(),
+                        value: info.range.repr_decode(raw).into(),
                         range: info.range.clone(),
                     }
                 }
@@ -1474,23 +1481,93 @@ mod test {
         // Create the registry for the program
         let registry = ProgramRegistry::<CoreType, CoreLibfunc>::new(&program).unwrap();
 
-        // Valid case
+        // Valid case: `BoundedInt<10, 510>` (a `Range` of `[10, 511)`) has a
+        // 9-bit compact representation (2 bytes), storing `value - lower`.
+        let value = Value::BoundedInt {
+            value: Felt::from(16),
+            range: Range {
+                lower: BigInt::from(10),
+                upper: BigInt::from(511),
+            },
+        };
+
+        let arena = Bump::new();
+        let ptr = value
+            .to_ptr(&arena, &registry, &program.type_declarations[1].id)
+            .unwrap();
+
+        assert_eq!(unsafe { *ptr.cast::<[u8; 2]>().as_ptr() }, [6, 0]);
         assert_eq!(
-            unsafe {
-                *Value::BoundedInt {
-                    value: Felt::from(16),
-                    range: Range {
-                        lower: BigInt::from(10),
-                        upper: BigInt::from(510),
-                    },
-                }
-                .to_ptr(&Bump::new(), &registry, &program.type_declarations[1].id)
-                .unwrap()
-                .cast::<[u32; 8]>()
-                .as_ptr()
+            Value::from_ptr(ptr, &program.type_declarations[1].id, &registry).unwrap(),
+            value
+        );
+    }
+
+    #[test]
+    fn test_roundtrip_bounded_int_array() {
+        // `BoundedInt<3, 10>` has a 3-bit compact representation (1-byte layout
+        // and stride), storing `value - 3`.
+        let program = ProgramParser::new()
+            .parse(
+                "type B = BoundedInt<3, 10>;
+                type A = Array<B>;",
+            )
+            .unwrap();
+        let registry = ProgramRegistry::<CoreType, CoreLibfunc>::new(&program).unwrap();
+
+        let bounded = |v: u64| Value::BoundedInt {
+            value: Felt::from(v),
+            range: Range {
+                lower: BigInt::from(3),
+                upper: BigInt::from(11),
             },
-            [16, 0, 0, 0, 0, 0, 0, 0]
+        };
+        let value = Value::Array(vec![bounded(3), bounded(7), bounded(9)]);
+
+        let arena = Bump::new();
+        let ptr = value
+            .to_ptr(&arena, &registry, &program.type_declarations[1].id)
+            .unwrap();
+
+        // Check the raw element buffer: 1-byte stride of biased values.
+        let abi = unsafe { ptr.cast::<crate::starknet::ArrayAbi<u8>>().as_ref() };
+        assert_eq!(
+            unsafe { std::slice::from_raw_parts(abi.ptr, 3) },
+            &[0, 4, 6]
         );
+
+        assert_eq!(
+            Value::from_ptr(ptr, &program.type_declarations[1].id, &registry).unwrap(),
+            value
+        );
+    }
+
+    #[test]
+    fn test_roundtrip_bounded_int_negative_lower() {
+        let program = ProgramParser::new()
+            .parse("type B = BoundedInt<-5, 5>;")
+            .unwrap();
+        let registry = ProgramRegistry::<CoreType, CoreLibfunc>::new(&program).unwrap();
+
+        for v in [-5_i64, -1, 0, 4] {
+            let value = Value::BoundedInt {
+                value: Felt::from(v),
+                range: Range {
+                    lower: BigInt::from(-5),
+                    upper: BigInt::from(6),
+                },
+            };
+
+            let arena = Bump::new();
+            let ptr = value
+                .to_ptr(&arena, &registry, &program.type_declarations[0].id)
+                .unwrap();
+
+            assert_eq!(
+                Value::from_ptr(ptr, &program.type_declarations[0].id, &registry).unwrap(),
+                value
+            );
+        }
     }
 
     #[test]

test_data/programs/box_return_with_pedersen.cairo

@@ -0,0 +1,5 @@
+use core::pedersen::pedersen;
+
+fn run_test(a: felt252, b: felt252) -> Box<felt252> {
+    BoxTrait::new(pedersen(a, b))
+}

tests/tests/programs.rs

@@ -314,3 +314,37 @@ fn no_op() {
     )
     .unwrap();
 }
+
+#[test]
+fn box_return_forced_through_return_ptr() {
+    // The pedersen builtin makes the function return more than one value (the
+    // builtin plus the box), so the box comes back through the return pointer,
+    // which `parse_result` must dereference once before reading the payload.
+    let program = &load_program_and_runner("programs/box_return_with_pedersen");
+
+    let (a, b) = (Felt::from(1234), Felt::from(5678));
+
+    let result_vm = run_vm_program(
+        program,
+        "run_test",
+        vec![Arg::Value(a), Arg::Value(b)],
+        Some(DEFAULT_GAS as usize),
+    )
+    .unwrap();
+
+    let result_native = run_native_program(
+        program,
+        "run_test",
+        &[Value::Felt252(a), Value::Felt252(b)],
+        Some(DEFAULT_GAS),
+        Option::<DummySyscallHandler>::None,
+    );
+
+    compare_outputs(
+        &program.1,
+        &program.2.find_function("run_test").unwrap().id,
+        &result_vm,
+        &result_native,
+    )
+    .expect("boxed return through return pointer must agree between VM and native");
+}