fix(values): make Value::to_ptr always return the inline representation by TomerStarkware · Pull Request #1655 · starkware-libs/cairo_native

TomerStarkware · 2026-08-18T07:40:07Z

Problem

For memory-allocated types (>=2-variant enums and aggregates transitively containing one), Value::to_ptr returned a wrapper pointer (a slot holding the data pointer), while Value::from_ptr always reads the inline representation. Every recursive consumer of to_ptr had to un-box the wrapper — and four of six forgot:

Array elements (values.rs): copied elem_layout.size() bytes out of the 8-byte wrapper. Even Array<bool> was corrupted — the 16-byte-aligned enum body keeps the tag bit clear, so every element decoded as variant 0 (and gas diverged from the VM).
Felt252Dict values: same omission (Felt252Dict<bool> is legal Cairo via BoolFelt252DictValue).
AbiArgument Box and Nullable arms (arch.rs): memcpy'd the wrapper bytes into the heap payload (Box<SomeEnum>).

Fix

Enumerating every .to_ptr( call site shows nothing consumes the wrapper — each caller either stripped it immediately (Struct/Enum arms, arch.rs enum/struct arms) or was one of the bugs. So instead of patching each site, invert the contract:

to_ptr always returns a pointer to the value's inline representation per TypeBuilder::layout() — exactly what from_ptr reads. The by-pointer ABI decision lives only in crate::arch's AbiArgument impl.

This deletes the wrapping in the Struct/Enum arms and the un-wrapping in arch.rs, and fixes the four forgetting sites by construction. The Felt252Dict arm is also brought under the same convention: it returned the FeltDict* itself instead of a slot holding it, so a dict nested inside an aggregate copied 8 bytes of HashMap internals.

Only reachable via invoke_dynamic(&[Value]); the Starknet contract path marshals felts directly and is unaffected. Codegen-side is_memory_allocated uses are the compiled ABI and are untouched — the wrapper never crossed the FFI boundary.

Tests

VM-vs-native regression: Array<bool> argument.
to_ptr→from_ptr round-trip unit tests, now valid under the symmetric contract: bool array, struct with bool, nested enum, bool dict.

Follow-up of #1650, which fixed this same bug class for enum payloads.

🤖 Generated with Claude Code

This change is

github-actions · 2026-08-18T07:40:36Z

✅ Code is now correctly formatted.

github-actions · 2026-08-18T08:16:49Z

Benchmark results Main vs HEAD.

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_insert.cairo (JIT)`	1.876 ± 0.018	1.857	1.906	1.02 ± 0.01
`base dict_insert.cairo (AOT)`	1.844 ± 0.016	1.811	1.869	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head dict_insert.cairo (JIT)`	1.945 ± 0.035	1.896	1.996	1.01 ± 0.02
`head dict_insert.cairo (AOT)`	1.930 ± 0.018	1.906	1.954	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_snapshot.cairo (JIT)`	1.687 ± 0.012	1.666	1.707	1.02 ± 0.01
`base dict_snapshot.cairo (AOT)`	1.658 ± 0.016	1.634	1.682	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head dict_snapshot.cairo (JIT)`	1.698 ± 0.024	1.660	1.754	1.00 ± 0.02
`head dict_snapshot.cairo (AOT)`	1.696 ± 0.015	1.673	1.715	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base factorial_2M.cairo (JIT)`	1.743 ± 0.020	1.718	1.777	1.02 ± 0.02
`base factorial_2M.cairo (AOT)`	1.713 ± 0.019	1.682	1.737	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head factorial_2M.cairo (JIT)`	1.780 ± 0.024	1.750	1.836	1.03 ± 0.02
`head factorial_2M.cairo (AOT)`	1.734 ± 0.018	1.708	1.763	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base fib_2M.cairo (JIT)`	1.679 ± 0.020	1.664	1.724	1.01 ± 0.02
`base fib_2M.cairo (AOT)`	1.667 ± 0.017	1.647	1.707	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head fib_2M.cairo (JIT)`	1.703 ± 0.019	1.684	1.744	1.02 ± 0.02
`head fib_2M.cairo (AOT)`	1.665 ± 0.018	1.642	1.692	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search.cairo (JIT)`	1.704 ± 0.009	1.691	1.716	1.02 ± 0.01
`base linear_search.cairo (AOT)`	1.667 ± 0.013	1.645	1.687	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head linear_search.cairo (JIT)`	1.699 ± 0.012	1.678	1.716	1.02 ± 0.01
`head linear_search.cairo (AOT)`	1.666 ± 0.017	1.636	1.691	1.00

Base

Command	Mean [s]	Min [s]	Max [s]	Relative
`base logistic_map.cairo (JIT)`	1.679 ± 0.019	1.654	1.715	1.02 ± 0.01
`base logistic_map.cairo (AOT)`	1.643 ± 0.014	1.613	1.665	1.00

Head

Command	Mean [s]	Min [s]	Max [s]	Relative
`head logistic_map.cairo (JIT)`	1.685 ± 0.013	1.657	1.699	1.00
`head logistic_map.cairo (AOT)`	1.710 ± 0.031	1.647	1.754	1.01 ± 0.02

github-actions · 2026-08-18T08:21:46Z

Benchmarking results

Benchmark for program `dict_insert`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	8.393 ± 0.044	8.347	8.493	6.16 ± 0.05
`cairo-native (embedded AOT)`	1.363 ± 0.007	1.358	1.375	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	1.372 ± 0.006	1.363	1.379	1.01 ± 0.01

Benchmark for program `dict_snapshot`

Open benchmarks

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Cairo-vm (Rust, Cairo 1)`	387.5 ± 8.6	379.5	402.2	1.00
`cairo-native (embedded AOT)`	1213.6 ± 3.3	1209.2	1218.1	3.13 ± 0.07
`cairo-native (embedded JIT using LLVM's ORC Engine)`	1245.5 ± 10.5	1225.2	1257.6	3.21 ± 0.08

Benchmark for program `factorial_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	3.805 ± 0.015	3.788	3.834	3.00 ± 0.02
`cairo-native (embedded AOT)`	1.269 ± 0.009	1.257	1.287	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	1.288 ± 0.007	1.278	1.297	1.01 ± 0.01

Benchmark for program `fib_2M`

Open benchmarks

Command	Mean [s]	Min [s]	Max [s]	Relative
`Cairo-vm (Rust, Cairo 1)`	3.746 ± 0.010	3.731	3.765	3.07 ± 0.02
`cairo-native (embedded AOT)`	1.222 ± 0.006	1.213	1.230	1.00
`cairo-native (embedded JIT using LLVM's ORC Engine)`	1.238 ± 0.009	1.228	1.253	1.01 ± 0.01

Benchmark for program `linear_search`

Open benchmarks

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Cairo-vm (Rust, Cairo 1)`	426.6 ± 7.7	414.2	437.0	1.00
`cairo-native (embedded AOT)`	1235.2 ± 5.9	1225.6	1246.1	2.90 ± 0.05
`cairo-native (embedded JIT using LLVM's ORC Engine)`	1264.3 ± 7.7	1253.7	1279.5	2.96 ± 0.06

Benchmark for program `logistic_map`

Open benchmarks

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`Cairo-vm (Rust, Cairo 1)`	360.6 ± 9.3	350.5	378.6	1.00
`cairo-native (embedded AOT)`	1216.9 ± 7.4	1206.8	1227.6	3.37 ± 0.09
`cairo-native (embedded JIT using LLVM's ORC Engine)`	1248.6 ± 8.1	1235.0	1261.3	3.46 ± 0.09

orizi

@orizi reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).

For memory-allocated types (>=2-variant enums and aggregates containing one), `Value::to_ptr` returned a *wrapper* pointer (a slot holding the data pointer) while `Value::from_ptr` always reads the inline representation. Every recursive consumer had to un-box the wrapper, and four of six forgot: - the Array arm copied `elem_layout.size()` bytes out of the 8-byte wrapper, so even `Array<bool>` was corrupted (the 16-byte-aligned enum body keeps the tag bit clear, decoding every element as variant 0); - the Felt252Dict arm had the same omission (`Felt252Dict<bool>`); - the `AbiArgument` Box and Nullable arms memcpy'd the wrapper bytes into the heap block (`Box<SomeEnum>`). Enumerating all callers shows nothing consumes the wrapper — every call site either stripped it immediately or was one of these bugs — so invert the contract instead of patching each site: `to_ptr` now always returns a pointer to the inline representation per `TypeBuilder::layout()`, exactly what `from_ptr` reads, and the by-pointer ABI decision lives only in `crate::arch`'s `AbiArgument` impl. Also make the Felt252Dict arm follow the same convention: it returned the `FeltDict*` itself instead of a slot holding it, so a dict nested in an aggregate copied 8 bytes of HashMap internals instead of the dict pointer. The arch.rs dict arm now dereferences the slot once. The Nullable arm also now passes the payload type id like the Box arm does. Only reachable via the `invoke_dynamic(&[Value])` API; the Starknet contract path marshals felts directly and is unaffected. Adds a VM-vs-native regression test for `Array<bool>` and `to_ptr`->`from_ptr` round-trip unit tests (bool array, struct with bool, nested enum, bool dict) that the new symmetric contract enables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

This was referenced Aug 18, 2026

fix(values): support Box and Nullable nested inside aggregates in to_ptr #1656

Merged

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

Merged

TomerStarkware force-pushed the tomer/values_inline_to_ptr branch 2 times, most recently from ab01fbe to c534003 Compare August 19, 2026 16:07

TomerStarkware requested a review from orizi August 20, 2026 12:28

orizi approved these changes Aug 20, 2026

View reviewed changes

TomerStarkware force-pushed the tomer/values_inline_to_ptr branch from c534003 to 5d7a5d3 Compare August 23, 2026 08:31

TomerStarkware added this pull request to the merge queue Aug 23, 2026

Merged via the queue into main with commit a1bfb99 Aug 23, 2026
15 checks passed

TomerStarkware deleted the tomer/values_inline_to_ptr branch August 23, 2026 09:47

Navigation Menu

Uh oh!

fix(values): make Value::to_ptr always return the inline representation - #1655

fix(values): make Value::to_ptr always return the inline representation#1655
TomerStarkware merged 1 commit into
mainfrom
tomer/values_inline_to_ptr

TomerStarkware commented Aug 18, 2026 •

edited by orizi

Loading

Uh oh!

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

orizi left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TomerStarkware commented Aug 18, 2026 • edited by orizi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Tests

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark results Main vs HEAD.

Base

Head

Base

Head

Base

Head

Base

Head

Base

Head

Base

Head

Uh oh!

github-actions Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarking results

Benchmark for program dict_insert

Benchmark for program dict_snapshot

Benchmark for program factorial_2M

Benchmark for program fib_2M

Benchmark for program linear_search

Benchmark for program logistic_map

Uh oh!

orizi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TomerStarkware commented Aug 18, 2026 •

edited by orizi

Loading

github-actions Bot commented Aug 18, 2026 •

edited

Loading

github-actions Bot commented Aug 18, 2026 •

edited

Loading

github-actions Bot commented Aug 18, 2026 •

edited

Loading

Benchmark for program `dict_insert`

Benchmark for program `dict_snapshot`

Benchmark for program `factorial_2M`

Benchmark for program `fib_2M`

Benchmark for program `linear_search`

Benchmark for program `logistic_map`