Skip to content

Commit

Permalink
Auto merge of rust-lang#97033 - nbdd0121:unwind3, r=Amanieu
Browse files Browse the repository at this point in the history
Remove libstd's calls to `C-unwind` foreign functions

Remove all libstd and its dependencies' usage of `extern "C-unwind"`.

This is a prerequiste of a WIP PR which will forbid libraries calling `extern "C-unwind"` functions to be compiled in `-Cpanic=unwind` and linked against `panic_abort` (this restriction is necessary to address soundness bug rust-lang#96926).
Cargo will ensure all crates are compiled with the same `-Cpanic` but the std is only compiled `-Cpanic=unwind` but needs the ability to be linked into `-Cpanic=abort`.

Currently there are two places where `C-unwind` is used in libstd:
* `__rust_start_panic` is used for interfacing to the panic runtime. This could be `extern "Rust"`
* `_{rdl,rg}_oom`: a shim `__rust_alloc_error_handler` will be generated by codegen to call into one of these; they can also be `extern "Rust"` (in fact, the generated shim is used as `extern "Rust"`, so I am not even sure why these are not, probably because they used to `extern "C"` and was changed to `extern "C-unwind"` when we allow alloc error hooks to unwind, but they really should just be using Rust ABI).

For dependencies, there is only one `extern "C-unwind"` function call, in `unwind` crate. This can be expressed as a re-export.

More dicussions can be seen in the Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/210922-project-ffi-unwind/topic/soundness.20in.20mixed.20panic.20mode

`@rustbot` label: T-libs F-c_unwind
  • Loading branch information
bors committed May 19, 2022
2 parents e6327bc + fbb3c19 commit 50872bd
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 9 deletions.
4 changes: 2 additions & 2 deletions library/alloc/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,13 +398,13 @@ pub mod __alloc_error_handler {

// if there is no `#[alloc_error_handler]`
#[rustc_std_internal_symbol]
pub unsafe extern "C-unwind" fn __rdl_oom(size: usize, _align: usize) -> ! {
pub unsafe fn __rdl_oom(size: usize, _align: usize) -> ! {
panic!("memory allocation of {size} bytes failed")
}

// if there is an `#[alloc_error_handler]`
#[rustc_std_internal_symbol]
pub unsafe extern "C-unwind" fn __rg_oom(size: usize, align: usize) -> ! {
pub unsafe fn __rg_oom(size: usize, align: usize) -> ! {
let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
extern "Rust" {
#[lang = "oom"]
Expand Down
2 changes: 1 addition & 1 deletion library/panic_abort/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub unsafe extern "C" fn __rust_panic_cleanup(_: *mut u8) -> *mut (dyn Any + Sen

// "Leak" the payload and shim to the relevant abort on the platform in question.
#[rustc_std_internal_symbol]
pub unsafe extern "C-unwind" fn __rust_start_panic(_payload: *mut &mut dyn BoxMeUp) -> u32 {
pub unsafe fn __rust_start_panic(_payload: *mut &mut dyn BoxMeUp) -> u32 {
// Android has the ability to attach a message as part of the abort.
#[cfg(target_os = "android")]
android::android_set_abort_message(_payload);
Expand Down
2 changes: 1 addition & 1 deletion library/panic_unwind/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub unsafe extern "C" fn __rust_panic_cleanup(payload: *mut u8) -> *mut (dyn Any
// Entry point for raising an exception, just delegates to the platform-specific
// implementation.
#[rustc_std_internal_symbol]
pub unsafe extern "C-unwind" fn __rust_start_panic(payload: *mut &mut dyn BoxMeUp) -> u32 {
pub unsafe fn __rust_start_panic(payload: *mut &mut dyn BoxMeUp) -> u32 {
let payload = Box::from_raw((*payload).take_box());

imp::panic(payload)
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extern "C" {
}

#[allow(improper_ctypes)]
extern "C-unwind" {
extern "Rust" {
/// `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).
Expand Down
5 changes: 1 addition & 4 deletions library/unwind/src/libunwind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,7 @@ if #[cfg(not(all(target_os = "ios", target_arch = "arm")))] {
pub fn _Unwind_SjLj_RaiseException(e: *mut _Unwind_Exception) -> _Unwind_Reason_Code;
}

#[inline]
pub unsafe fn _Unwind_RaiseException(exc: *mut _Unwind_Exception) -> _Unwind_Reason_Code {
_Unwind_SjLj_RaiseException(exc)
}
pub use _Unwind_SjLj_RaiseException as _Unwind_RaiseException;
}
} // cfg_if!

Expand Down

0 comments on commit 50872bd

Please sign in to comment.