Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cryptonerdcn opened this issue Nov 12, 2024 · 10 comments
Open
Labels
bug Something isn't working

Comments

@cryptonerdcn
Copy link

Bug Report

Cairo version:

2.8.0

Current behavior:

I am cryptonerdcn, author of WASM-Cairo. After this PR(#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 #6245 (fb3d11c ) works normally. So I am sure the problem is from #6173 (3be4896).

Other information:

@cryptonerdcn cryptonerdcn added the bug Something isn't working label Nov 12, 2024
@cryptonerdcn cryptonerdcn changed the title bug: bug: Crash in Cairo compiler(WASM-Cairo) with Rust Analyzer's Salsa. Nov 12, 2024
@cryptonerdcn
Copy link
Author

Relative issue: #6171

@orizi
Copy link
Collaborator

orizi commented Nov 12, 2024

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
Copy link
Author

cryptonerdcn commented Nov 12, 2024

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
Copy link
Collaborator

orizi commented Nov 12, 2024

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
Copy link
Member

mkaput commented Nov 12, 2024

@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
Copy link
Collaborator

orizi commented Nov 12, 2024

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
Copy link
Member

mkaput commented Nov 12, 2024

@cryptonerdcn to me it looks like one of the unwraps here is failing: e7d5aa9#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
Copy link
Author

cryptonerdcn commented Nov 12, 2024

@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
Copy link
Member

mkaput commented Nov 12, 2024

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
Copy link
Author

cryptonerdcn commented Nov 12, 2024

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(#6173), at least the compiler can work fine (Based on Cairo 2.8.2, sha: 6679166).
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 (keep-starknet-strange/shinigami#240).
Thanks for your help again! @orizi @mkaput

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants