Skip to content

Use C-unwind ABI for __rust_start_panic in panic_abort#87787

Merged
bors merged 1 commit intomasterfrom
unknown repository
Aug 6, 2021
Merged

Use C-unwind ABI for __rust_start_panic in panic_abort#87787
bors merged 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Aug 5, 2021

The function originally has C ABI but is called using C-unwind ABI in std:

rust/library/std/src/panicking.rs

Lines 49 to 54 in d4ad1cf

extern "C-unwind" {
/// `payload` is passed through another layer of raw pointers as `&mut dyn Trait` is not
/// FFI-safe. `BoxMeUp` lazily performs allocation only when needed (this avoids allocations
/// when using the "abort" panic runtime).
fn __rust_start_panic(payload: *mut &mut dyn BoxMeUp) -> u32;
}

Which might be problematic and triggers this Miri error:

error: Undefined Behavior: calling a function with ABI C using caller ABI C-unwind
   --> /home/hyd-dev/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:672:9
    |
672 |         __rust_start_panic(obj)
    |         ^^^^^^^^^^^^^^^^^^^^^^^ calling a function with ABI C using caller ABI C-unwind
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

Changing the ABI of the function to C-unwind fixes the error above.

@rust-highfive
Copy link
Contributor

rust-highfive commented Aug 5, 2021

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 5, 2021
This was referenced Aug 5, 2021
Closed
Merged
@RalfJung
Copy link
Member

RalfJung commented Aug 6, 2021

This sounds like Miri could accept it (though I am not sure of that), but I also don't think there is any reason to use a different ABI string on the two sides here. Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 6, 2021

📌 Commit 7520ea9 has been approved by RalfJung

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2021
@JohnTitor JohnTitor mentioned this pull request Aug 6, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2021
@bors
Auto merge of rust-lang#87822 - JohnTitor:rollup-kxojii0, r=JohnTitor
5ad7389 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#85807 (bootstrap: Disable initial-exec TLS model on powerpc)
 - rust-lang#87761 (Fix overflow in rustc happening if the `err_count()` is reduced in a stage.)
 - rust-lang#87775 (Add hint for unresolved associated trait items if the trait has a single item)
 - rust-lang#87779 (Remove special case for statement `NodeId` assignment)
 - rust-lang#87787 (Use `C-unwind` ABI for `__rust_start_panic` in `panic_abort`)
 - rust-lang#87809 (Fix typo in the ptr documentation)
 - rust-lang#87816 (Sync rustc_codegen_cranelift)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c5202b3 into rust-lang:master Aug 6, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 6, 2021
@ghost ghost deleted the c-unwind branch August 12, 2021 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.56.0

Development

Successfully merging this pull request may close these issues.

5 participants

@rust-highfive @RalfJung @bors @Mark-Simulacrum @rustbot