starkware-libs / cairo

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

bug: Crash in Cairo compiler(WASM-Cairo) with Rust Analyzer's Salsa. #6629

Open cryptonerdcn opened 6 days ago

cryptonerdcn commented 6 days ago

Bug Report

Cairo version:

2.8.0

Current behavior:

I am cryptonerdcn, author of WASM-Cairo. After this PR(https://github.com/starkware-libs/cairo/pull/6173/), the WASM-Cairo version Cairo compiler will crash when the code uses println!.

Expected behavior:

Steps to reproduce:

Checkout this branch: https://github.com/cryptonerdcn/cairo/tree/tmp/test_PR6173

Build:

cargo build -p cairo-compile --target wasm32-wasi --release

Then run:

wasmtime target/wasm32-wasi/release/cairo-compile.wasm -- --single-file ./test.cairo --input-program-string "fn run_tests() {assert(bool::True(()), 1);}"

The code will run normally.

But if you run:

wasmtime target/wasm32-wasi/release/cairo-compile.wasm -- --single-file ./test.cairo --input-program-string 'fn main(){println!("Hello, World!");}'

Will crash with this output:

thread 'main' panicked at crates/cairo-lang-filesystem/src/db.rs:263:42:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error: failed to run main module `target/wasm32-wasi/release/cairo-compile.wasm`

Caused by:
    0: failed to invoke command default
    1: error while executing at wasm backtrace:
           0: 0xd9f647 - cairo_compile-c6d1fb3c515bbdf8.wasm!__rust_start_panic
           1: 0xd9f1ac - cairo_compile-c6d1fb3c515bbdf8.wasm!rust_panic
           2: 0xd9f011 - cairo_compile-c6d1fb3c515bbdf8.wasm!std::panicking::rust_panic_with_hook::h2ed697044fa66de8
           3: 0xd9e213 - cairo_compile-c6d1fb3c515bbdf8.wasm!std::panicking::begin_panic_handler::{{closure}}::hd3fce676619be24b
           4: 0xd9e152 - cairo_compile-c6d1fb3c515bbdf8.wasm!std::sys::backtrace::__rust_end_short_backtrace::hf4c64ea258ae1491
           5: 0xd9e961 - cairo_compile-c6d1fb3c515bbdf8.wasm!rust_begin_unwind
           6: 0xdad54b - cairo_compile-c6d1fb3c515bbdf8.wasm!core::panicking::panic_fmt::hc988c56e45068cc1
           7: 0xdaebca - cairo_compile-c6d1fb3c515bbdf8.wasm!core::panicking::panic::ha419388cb8858206
           8: 0xdb4f6e - cairo_compile-c6d1fb3c515bbdf8.wasm!core::option::unwrap_failed::hd2c54ff58c095093
           9: 0xd2d54b - cairo_compile-c6d1fb3c515bbdf8.wasm!<cairo_lang_filesystem::db::PrivRawFileContentQuery as salsa::plumbing::QueryFunction>::execute::h6a4ba9a620df9e8f
          10: 0xd3cb5d - cairo_compile-c6d1fb3c515bbdf8.wasm!salsa::derived::slot::Slot<Q,MP>::execute::h93cf29e363a57fb5
          11: 0xd461a2 - cairo_compile-c6d1fb3c515bbdf8.wasm!salsa::derived::slot::Slot<Q,MP>::read::hb4cfcd1267751b4b
          12: 0xd58af0 - cairo_compile-c6d1fb3c515bbdf8.wasm!<salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::fetch::h7b64e0a63ba03251
          13: 0xd2cabd - cairo_compile-c6d1fb3c515bbdf8.wasm!<DB as cairo_lang_filesystem::db::FilesGroup>::priv_raw_file_content::__shim::hf205c6964704c2a0
          14: 0xaf674 - cairo_compile-c6d1fb3c515bbdf8.wasm!<DB as cairo_lang_filesystem::db::FilesGroup>::priv_raw_file_content::h69edc544eb992d80
          15: 0xd3922b - cairo_compile-c6d1fb3c515bbdf8.wasm!salsa::derived::slot::Slot<Q,MP>::execute::h7a6b639d7748a528
          16: 0xd44929 - cairo_compile-c6d1fb3c515bbdf8.wasm!salsa::derived::slot::Slot<Q,MP>::read::h30efa2f370fa82dc
          17: 0xd592ad - cairo_compile-c6d1fb3c515bbdf8.wasm!<salsa::derived::DerivedStorage<Q,MP> as salsa::plumbing::QueryStorageOps<Q>>::fetch::hcb03a297a96605fb
          18: 0xd2cb18 - cairo_compile-c6d1fb3c515bbdf8.wasm!<DB as cairo_lang_filesystem::db::FilesGroup>::file_content::__shim::h9b6a688c1b074702
          19: 0xb4bbd - cairo_compile-c6d1fb3c515bbdf8.wasm!cairo_lang_compiler::diagnostics::DiagnosticsReporter::check::h06f99b6f1f97dc7c
          20: 0xb5861 - cairo_compile-c6d1fb3c515bbdf8.wasm!cairo_lang_compiler::wasm_cairo_interface::compile_cairo_project_with_input_string::ha556e57b43481fe8
          21: 0x104e8 - cairo_compile-c6d1fb3c515bbdf8.wasm!cairo_compile::main::h9f346a7530e0cff8
          22: 0xc58b - cairo_compile-c6d1fb3c515bbdf8.wasm!std::sys::backtrace::__rust_begin_short_backtrace::h0ff002016a0950bc
          23: 0xc514 - cairo_compile-c6d1fb3c515bbdf8.wasm!std::rt::lang_start::{{closure}}::hfb743f751a76a5a5
          24: 0xd95828 - cairo_compile-c6d1fb3c515bbdf8.wasm!std::rt::lang_start_internal::heff804dabc0ba4b9
          25: 0x136a8 - cairo_compile-c6d1fb3c515bbdf8.wasm!__main_void
          26: 0x837f - cairo_compile-c6d1fb3c515bbdf8.wasm!_start
    2: wasm trap: wasm `unreachable` instruction executed

Related code:

The previous PR https://github.com/starkware-libs/cairo/pull/6245/ (fb3d11c7ebf4dfeb04cf86e94da0d3a4867c5ba9 ) works normally. So I am sure the problem is from https://github.com/starkware-libs/cairo/pull/6173/ (3be48966e2b710888c0075dd642630c755e8f12f).

Other information:

cryptonerdcn commented 6 days ago

Relative issue: https://github.com/starkware-libs/cairo/issues/6171

orizi commented 6 days ago

Moving to a newer salsa version was required for making the language server viable for normal user usage.

Having a wasm build for a fork of the compiler wasn't part of the considerations.

@ArielElp

cryptonerdcn commented 6 days ago

Having a wasm build for a fork of the compiler wasn't part of the considerations.

Thanks for your reply! @orizi Yes I know, so I did it alone for a long time. Maybe it's salsa's bug(because it is pre-release?), and WASM-Cairo users don't require language server features, I think I can make an old salsa version for it. Do you have any advice for it?

orizi commented 6 days ago

In any case - from looking at the failure stack - it seems like a salsa issue.

have you tried rebasing over the latest on main?

mkaput commented 6 days ago

@cryptonerdcn you could give https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html a try to forcefully compile with a stable salsa release, but I recall there were some source-wise differences in cycles handling (and there are definitely breaking changes regarding cancellation, but that one is scoped to LS only)

I am currently trying to debug this also

orizi commented 6 days ago

Do you have any advice for it?

Well - you can see in the PR the adaptations we did to handle the changes, so the opposite will be required for a revert, but do note we needed to do some logical adaptations to handle the difference between these (their cycle handling logic is somewhat different) - which i assume you really won't want to do.

mkaput commented 6 days ago

@cryptonerdcn to me it looks like one of the unwraps here is failing: https://github.com/starkware-libs/cairo/commit/e7d5aa90ba7910bd4009580ed860f45aa8218ded#diff-04d31e27b9f6f114064562aa726da3f89794fc84843b002862ead0d5832a25ecR255-R267

I have analysed the stack trace you got vs the actual source that is built, and it looks like some stuff was aggressively inlined by Rust between these two frames:

           8: 0xdb4f6e - cairo_compile-c6d1fb3c515bbdf8.wasm!core::option::unwrap_failed::hd2c54ff58c095093
           9: 0xd2d54b - cairo_compile-c6d1fb3c515bbdf8.wasm!<cairo_lang_filesystem::db::PrivRawFileContentQuery as salsa::plumbing::QueryFunction>::execute::h6a4ba9a620df9e8f

The <cairo_lang_filesystem::db::PrivRawFileContentQuery as salsa::plumbing::QueryFunction>::execute function directly calls priv_raw_file_content function that provides implementation for that query, and it appears that it was fully inlined along with your read_file_on_disk_or_wasm that it calls (which explains why they're not visible in stack trace).

cryptonerdcn commented 6 days ago

@mkaput Thank you! Because wasm can not really read a file from disk, the read_file_on_disk_or_wasm is used to read the files embedded into the wasm package(Basically, the .cairo files in the corelib ). So maybe the problem is salsa needs to access sth. in the disk but it can't?

mkaput commented 6 days ago

So maybe the problem is salsa needs to access sth. in the disk but it can't?

Nope, Salsa shouldn't be doing any I/O. Instead, my guess is that it changes some order of operations or smth, that is causing read_file_on_disk_or_wasm to fail. I suggest putting some logs & replacing these unwraps with expect with distinct messages to investigate which call precisely is failing, as there are many there.

cryptonerdcn commented 6 days ago

Good news: As @mkaput said, "there are definitely breaking changes regarding cancellation, but that one is scoped to LS only". If I use salsa = "0.16.1" and revert other changes of PR(https://github.com/starkware-libs/cairo/pull/6173), at least the compiler can work fine (Based on Cairo 2.8.2, sha: 6679166d823395a8bf54a592be192b92dc8a3fcd). Cause @orizi said "it seems like a salsa issue.", I will temporarily use this solution to release a new version of WASM-Cairo to support Cairo 2.8.2 because Shinigami needs it (https://github.com/keep-starknet-strange/shinigami/issues/240). Thanks for your help again! @orizi @mkaput