starkware-libs / cairo

Cairo is the first Turing-complete language for creating provable programs for general computation.
Apache License 2.0
1.46k stars 450 forks source link

bug: Cast from usize to i16 failed during `apply_ap_change` #2942

Open clexmond opened 1 year ago

clexmond commented 1 year ago

Bug Report

Cairo version:

Commit 66357681

Current behavior:

When calling a fairly involved method (specifically a port of this noise3 method https://github.com/influenceth/cairo-rand-64x61/blob/master/contracts/cairo_rand_64x61/simplex.cairo) cairo-test exits roughly 2/3rds of the way through with the following (full Rust backtrace included):

thread 'main' panicked at 'Cast from usize to i16 failed: 34401', crates/cairo-lang-utils/src/casts.rs:4:41
stack backtrace:
   0:        0x107b3aa56 - std::backtrace_rs::backtrace::libunwind::trace::hb6e20554c370f2e3
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:        0x107b3aa56 - std::backtrace_rs::backtrace::trace_unsynchronized::h4cf6d4d5ecaf68bc
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x107b3aa56 - std::sys_common::backtrace::_print_fmt::h082ea160697d6d76
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/sys_common/backtrace.rs:65:5
   3:        0x107b3aa56 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h47bed05ceb7970ee
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/sys_common/backtrace.rs:44:22
   4:        0x107b556da - core::fmt::write::hefd823d99333384a
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/fmt/mod.rs:1213:17
   5:        0x107b37ecc - std::io::Write::write_fmt::hb978956bb1b85e2b
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/io/mod.rs:1682:15
   6:        0x107b3a83a - std::sys_common::backtrace::_print::hb9f21d71c6f0ffcc
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/sys_common/backtrace.rs:47:5
   7:        0x107b3a83a - std::sys_common::backtrace::print::hc9e298c4664da113
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/sys_common/backtrace.rs:34:9
   8:        0x107b3c083 - std::panicking::default_hook::{{closure}}::hb97411d6e916e1d2
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:267:22
   9:        0x107b3bdd8 - std::panicking::default_hook::hc3ac81176817192f
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:286:9
  10:        0x107b3c7bf - std::panicking::rust_panic_with_hook::hc11209fa2686218a
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:688:13
  11:        0x107b3c584 - std::panicking::begin_panic_handler::{{closure}}::h5bad1bd665ab68a6
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:579:13
  12:        0x107b3aef8 - std::sys_common::backtrace::__rust_end_short_backtrace::h6f4efa73918edf57
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/sys_common/backtrace.rs:137:18
  13:        0x107b3c24d - rust_begin_unwind
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
  14:        0x107bc78c3 - core::panicking::panic_fmt::hb5474ac70e328787
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
  15:        0x107b081db - cairo_lang_utils::casts::usize_as_i16::he3a26ece79471845
  16:        0x1072d10b7 - <cairo_lang_casm::cell_expression::CellExpression as cairo_lang_casm::ap_change::ApplyApChange>::apply_known_ap_change::h6d1fac5ecb4a5ca4
  17:        0x1071e4b31 - alloc::vec::in_place_collect::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter::h06bf04c516398dee
  18:        0x107254b74 - core::iter::adapters::try_process::h8eb49fde1d51793a
  19:        0x10727bcd7 - cairo_lang_casm::ap_change::ApplyApChange::apply_ap_change::hddc510de03414c3b
  20:        0x107256ce5 - cairo_lang_sierra_to_casm::annotations::ProgramAnnotations::propagate_annotations::h278767b207e26e02
  21:        0x10727ef2a - cairo_lang_sierra_to_casm::compiler::compile::hd8ff56e79bf56822
  22:        0x10719bf7d - cairo_lang_runner::SierraCasmRunner::new::h518581e461ad1987
  23:        0x1070a878c - cairo_test::main::h1c9c9c28c04eb5ff
  24:        0x10709ede6 - std::sys_common::backtrace::__rust_begin_short_backtrace::h276174c95e608718
  25:        0x1070addf1 - std::rt::lang_start::{{closure}}::h2ab030b3b2aa885d
  26:        0x107b33a80 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc4cda2832816fdfe
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/ops/function.rs:287:13
  27:        0x107b33a80 - std::panicking::try::do_call::h88890c8efd541404
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:483:40
  28:        0x107b33a80 - std::panicking::try::h0433cd1f4549a792
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:447:19
  29:        0x107b33a80 - std::panic::catch_unwind::hb89cd1eb3d81922b
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panic.rs:140:14
  30:        0x107b33a80 - std::rt::lang_start_internal::{{closure}}::h5b901d64a1361729
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/rt.rs:148:48
  31:        0x107b33a80 - std::panicking::try::do_call::hce6f590abf58f16f
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:483:40
  32:        0x107b33a80 - std::panicking::try::hcaf91f2458700054
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:447:19
  33:        0x107b33a80 - std::panic::catch_unwind::h0896b24bc6706178
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panic.rs:140:14
  34:        0x107b33a80 - std::rt::lang_start_internal::h36e955fa8337ef16
                               at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/rt.rs:148:20
  35:        0x1070ac21c - _main
  36:     0x7ff80de7341f - <unknown>
make: *** [test] Error 101

Expected behavior:

Method should execute and return as it did in Cairo 0.

Steps to reproduce / related code:

Pull the noise branch and run the simplex3_test suite. Execution will fail prior to line 97: https://github.com/influenceth/cubit/blob/noise/src/procgen/simplex3.cairo#L97

Additional Information:

cairo-compile . succeeds without error.

orizi commented 1 year ago

can you remove some of the inlining or split into some functions? generally - the issue here is the fact that you have a single function here with a pretty extreme ap change.

clexmond commented 1 year ago

can you remove some of the inlining or split into some functions? generally - the issue here is the fact that you have a single function here with a pretty extreme ap change.

Splitting into functions on its own fails, I am going to try splitting and including #[inline(never)]

orizi commented 1 year ago

also maybe try adding a internal::revoke_ap_tracking(); at the start of the large function. I suspect this all has something to do with the attempt to align ap over all the functions branches.

clexmond commented 1 year ago

also maybe try adding a internal::revoke_ap_tracking(); at the start of the large function. I suspect this all has something to do with the attempt to align ap over all the functions branches.

Splitting up into functions with #[inline(never)] and adding internal::revoke_ap_tracking(); doesn't seem to make any difference. Execution still fails to proceed at the same line. Only distinction is the error reflects the following rather than a casting error:

Caused by:
    #369->#370: Got 'Offset overflow' error while moving [6].
spapinistarkware commented 1 year ago

Updating here: adding revoke_ap_tracking() in the middle fixed this. We should do this in the compiler

clexmond commented 1 year ago

Just confirming that the inclusion of internal::revoke_ap_tracking() here https://github.com/influenceth/cubit/blob/noise/src/procgen/simplex3.cairo#L94 did solve the issue.

milancermak commented 1 year ago

@orizi @spapinistarkware bump on this issue. Just encountered it in a different codebase, applying the internal::revoke_ap_tracking() helped, but I presume that's a temporary hack.

7r8igv

ponderingdemocritus commented 1 year ago

has anyone successfully declared a Starknet contract using this method? I'm hitting an issue when declaring the contract

Error: StarkException: (400, {'code': <StarknetErrorCode.COMPILATION_FAILED: 3>, 'message': "Compilation failed. Error: Compilation failed.\n\nCaused by:\n    #5140->#5141: Got 'Offset overflow' error while moving [25].\n"})
clexmond commented 1 year ago

Easy to reproduce by commenting out https://github.com/influenceth/cubit/blob/main/src/procgen/simplex3.cairo#L83 and running tests (or trying to deploy a contract using that method).

Currently getting the following error still:

cairo-test .
thread 'main' panicked at 'Failed to cast from 52100.', ~/.cairo/v2.0.1/crates/cairo-lang-utils/src/casts.rs:8:35
stack backtrace:
   0: rust_begin_unwind
             at /rustc/e9e1bbc7a820c472b39d3de54b3049bf14050655/library/std/src/panicking.rs:578:5
   1: core::panicking::panic_fmt
             at /rustc/e9e1bbc7a820c472b39d3de54b3049bf14050655/library/core/src/panicking.rs:67:14
   2: <cairo_lang_casm::cell_expression::CellExpression as cairo_lang_casm::ap_change::ApplyApChange>::apply_known_ap_change
   3: alloc::vec::in_place_collect::<impl alloc::vec::spec_from_iter::SpecFromIter<T,I> for alloc::vec::Vec<T>>::from_iter
   4: core::iter::adapters::try_process
   5: cairo_lang_casm::ap_change::ApplyApChange::apply_ap_change
   6: cairo_lang_sierra_to_casm::annotations::ProgramAnnotations::propagate_annotations
   7: cairo_lang_sierra_to_casm::compiler::compile
   8: cairo_lang_runner::SierraCasmRunner::new
   9: cairo_lang_test_runner::run_tests
  10: cairo_lang_test_runner::TestRunner::run
  11: cairo_test::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
make: *** [test] Error 101
ponderingdemocritus commented 1 year ago

I have a feeling deploying a contract with this totally messes up the gas calculations

Check this tx: https://testnet.starkscan.co/tx/0x7644341a28aaa7a2b753b7e53d98564ec3d18129857b52eacda9951f66ab20d#overview


STEPS
47302
MEMORY
7471
PEDERSEN_BUILTIN
14
RANGE_CHECK_BUILTIN
8515

The function is not that complex and only updates a single storage slot, but this gas is insane...