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

std support for wasm32 panic=unwind #121438

Merged
merged 5 commits into from
Mar 11, 2024

Conversation

coolreader18
Copy link
Contributor

Tracking issue: #118168

This adds std support for -Cpanic=unwind on wasm, and with it slightly more fleshed out rustc support. Now, the stable default is still panic=abort without exception-handling, but if you -Zbuild-std with RUSTFLAGS=-Cpanic=unwind, you get wasm exception-handling try/catch blocks in the binary:

#[no_mangle]
pub fn foo_bar(x: bool) -> *mut u8 {
    let s = Box::<str>::from("hello");
    maybe_panic(x);
    Box::into_raw(s).cast()
}

#[inline(never)]
#[no_mangle]
fn maybe_panic(x: bool) {
    if x {
        panic!("AAAAA");
    }
}
;; snip...
(try $label$5
 (do
  (call $maybe_panic
   (local.get $0)
  )
  (br $label$1)
 )
 (catch_all
  (global.set $__stack_pointer
   (local.get $1)
  )
  (call $__rust_dealloc
   (local.get $2)
   (i32.const 5)
   (i32.const 1)
  )
  (rethrow $label$5)
 )
)
;; snip...

@rustbot
Copy link
Collaborator

rustbot commented Feb 22, 2024

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 22, 2024
@coolreader18
Copy link
Contributor Author

Also, though this is more of a future concern, idk if a panic=unwind std would be able to run nicely in a panic=abort binary atm - if the user is trying to keep backwards compatibility to wasm MVP, llvm or something would have to strip the try instructions from the end result.

extern "C" {
/// LLVM lowers this intrinsic to the `throw` instruction.
#[link_name = "llvm.wasm.throw"]
fn wasm_throw(tag: i32, ptr: *mut u8) -> !;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a new rust intrinsic which lowers to this LLVM intrinsic when using the LLVM backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I wanna put it in stdarch, alongside the other wasm instructions. A full extern "rust-intrinsic" intrinsic shouldn't be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Feb 22, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 22, 2024

This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp.

@coolreader18
Copy link
Contributor Author

Ok, now wasi uses libunwind, while -unknown still uses wasm.throw directly. It does seem like the scheme that LLVM implements is a tooling convention (so, not just LLVM's specific implementation) but it does say it's for C++ specifically, and says other languages should have distinct tags. However, doing that means (unless I'm reading the exception-handling proposal wrong) that exceptions caught in a different language can't be cleaned up. But maybe that's fine, for now.

@coolreader18
Copy link
Contributor Author

coolreader18 commented Feb 22, 2024

Oh, actually, if we had a distinct tag for panics it would just mean that a C++ exception would skip catch_unwind. Which is fine, if I'm reading the C-unwind rfc correctly.

As noted in the summary, if a Rust frame containing a pending catch_unwind call is unwound by a foreign exception, the behavior is undefined for now.

If we really want to process a C++ exception with catch_unwind in the future, we could just implement the itanium wasm EH scheme in library/unwind and codegen a catch $cpp_exc_tag for r#try. Once llvm supports that.

@coolreader18
Copy link
Contributor Author

coolreader18 commented Feb 22, 2024

Actually, knowing that, do you think wasi even needs llvm's libunwind? We can handle it just fine ourselves, obviously, and it will indeed cause UB at catch_unwind, but that's fine, and that'll be fixable if we want to once LLVM lets you specify other tags.

@coolreader18
Copy link
Contributor Author

I've convinced myself I think, lol

library/unwind/src/wasm.rs Outdated Show resolved Hide resolved
@cuviper
Copy link
Member

cuviper commented Mar 1, 2024

I'm not clear, is this waiting on the stdarch change, or vice versa?

@coolreader18
Copy link
Contributor Author

Vice versa - the stdarch PR will fail until this is in nightly.

@cuviper
Copy link
Member

cuviper commented Mar 4, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2024

📌 Commit c7fcf43 has been approved by cuviper

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 4, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 5, 2024
…r=cuviper

std support for wasm32 panic=unwind

Tracking issue: rust-lang#118168

This adds std support for `-Cpanic=unwind` on wasm, and with it slightly more fleshed out rustc support. Now, the stable default is still panic=abort without exception-handling, but if you `-Zbuild-std` with `RUSTFLAGS=-Cpanic=unwind`, you get wasm exception-handling try/catch blocks in the binary:

```rust
#[no_mangle]
pub fn foo_bar(x: bool) -> *mut u8 {
    let s = Box::<str>::from("hello");
    maybe_panic(x);
    Box::into_raw(s).cast()
}

#[inline(never)]
#[no_mangle]
fn maybe_panic(x: bool) {
    if x {
        panic!("AAAAA");
    }
}
```
```wat
;; snip...
(try $label$5
 (do
  (call $maybe_panic
   (local.get $0)
  )
  (br $label$1)
 )
 (catch_all
  (global.set $__stack_pointer
   (local.get $1)
  )
  (call $__rust_dealloc
   (local.get $2)
   (i32.const 5)
   (i32.const 1)
  )
  (rethrow $label$5)
 )
)
;; snip...
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#121280 (Implement MaybeUninit::fill{,_with,_from})
 - rust-lang#121438 (std support for wasm32 panic=unwind)
 - rust-lang#121658 (Hint user to update nightly on ICEs produced from outdated nightly)
 - rust-lang#121959 (Removing absolute path in proc-macro)
 - rust-lang#121961 (add test for rust-lang#78894 rust-lang#71450)
 - rust-lang#121975 (hir_analysis: enums return `None` in `find_field`)
 - rust-lang#121978 (Fix duplicated path in the "not found dylib" error)
 - rust-lang#121991 (Merge impl_trait_in_assoc_types_defined_by query back into `opaque_types_defined_by`)
 - rust-lang#122016 (will_wake tests fail on Miri and that is expected)
 - rust-lang#122018 (only set noalias on Box with the global allocator)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
…kingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#116791 (Allow codegen backends to opt-out of parallel codegen)
 - rust-lang#116793 (Allow targets to override default codegen backend)
 - rust-lang#117458 (LLVM Bitcode Linker: A self contained linker for nvptx and other targets)
 - rust-lang#119385 (Fix type resolution of associated const equality bounds (take 2))
 - rust-lang#121438 (std support for wasm32 panic=unwind)
 - rust-lang#121893 (Add tests (and a bit of cleanup) for interior mut handling in promotion and const-checking)
 - rust-lang#122080 (Clarity improvements to `DropTree`)
 - rust-lang#122152 (Improve diagnostics for parenthesized type arguments)
 - rust-lang#122166 (Remove the unused `field_remapping` field from `TypeLowering`)
 - rust-lang#122249 (interpret: do not call machine read hooks during validation)
 - rust-lang#122299 (Store backtrace for `must_produce_diag`)
 - rust-lang#122318 (Revision-related tweaks for next-solver tests)
 - rust-lang#122320 (Use ptradd for vtable indexing)
 - rust-lang#122328 (unix_sigpipe: Replace `inherit` with `sig_dfl` in syntax tests)
 - rust-lang#122330 (bootstrap readme: fix, improve, update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1279830 into rust-lang:master Mar 11, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
Rollup merge of rust-lang#121438 - coolreader18:wasm32-panic-unwind, r=cuviper

std support for wasm32 panic=unwind

Tracking issue: rust-lang#118168

This adds std support for `-Cpanic=unwind` on wasm, and with it slightly more fleshed out rustc support. Now, the stable default is still panic=abort without exception-handling, but if you `-Zbuild-std` with `RUSTFLAGS=-Cpanic=unwind`, you get wasm exception-handling try/catch blocks in the binary:

```rust
#[no_mangle]
pub fn foo_bar(x: bool) -> *mut u8 {
    let s = Box::<str>::from("hello");
    maybe_panic(x);
    Box::into_raw(s).cast()
}

#[inline(never)]
#[no_mangle]
fn maybe_panic(x: bool) {
    if x {
        panic!("AAAAA");
    }
}
```
```wat
;; snip...
(try $label$5
 (do
  (call $maybe_panic
   (local.get $0)
  )
  (br $label$1)
 )
 (catch_all
  (global.set $__stack_pointer
   (local.get $1)
  )
  (call $__rust_dealloc
   (local.get $2)
   (i32.const 5)
   (i32.const 1)
  )
  (rethrow $label$5)
 )
)
;; snip...
```
@coolreader18 coolreader18 deleted the wasm32-panic-unwind branch October 8, 2024 20:45
tgross35 added a commit to tgross35/rust that referenced this pull request Oct 12, 2024
… r=bjorn3

Use throw intrinsic from stdarch in wasm libunwind

Tracking issue: rust-lang#118168

This is a very belated followup to rust-lang#121438; now that rust-lang/stdarch#1542 is merged, we can use the intrinsic exported from `core::arch` instead of defining it inline. I also cleaned up the cfgs a bit and added a more detailed comment.
tgross35 added a commit to tgross35/rust that referenced this pull request Oct 13, 2024
… r=bjorn3

Use throw intrinsic from stdarch in wasm libunwind

Tracking issue: rust-lang#118168

This is a very belated followup to rust-lang#121438; now that rust-lang/stdarch#1542 is merged, we can use the intrinsic exported from `core::arch` instead of defining it inline. I also cleaned up the cfgs a bit and added a more detailed comment.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 13, 2024
Rollup merge of rust-lang#131418 - coolreader18:wasm-exc-use-stdarch, r=bjorn3

Use throw intrinsic from stdarch in wasm libunwind

Tracking issue: rust-lang#118168

This is a very belated followup to rust-lang#121438; now that rust-lang/stdarch#1542 is merged, we can use the intrinsic exported from `core::arch` instead of defining it inline. I also cleaned up the cfgs a bit and added a more detailed comment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants