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

miri no longer builds after rust-lang/rust#86155 #87778

Closed
rust-highfive opened this issue Aug 4, 2021 · 4 comments · Fixed by #87849
Closed

miri no longer builds after rust-lang/rust#86155 #87778

rust-highfive opened this issue Aug 4, 2021 · 4 comments · Fixed by #87849
Assignees
Labels
A-miri Area: The miri tool C-bug Category: This is a bug.

Comments

@rust-highfive
Copy link
Collaborator

Hello, this is your friendly neighborhood mergebot.
After merging PR #86155, I observed that the tool miri has failing tests.
A follow-up PR to the repository https://github.com/rust-lang/miri is needed to fix the fallout.

cc @alexcrichton, do you think you would have time to do the follow-up work?
If so, that would be great!

@rust-highfive rust-highfive added A-miri Area: The miri tool C-bug Category: This is a bug. labels Aug 4, 2021
@alexcrichton
Copy link
Member

I think this is what's necessary to fix the issue, but I'll test more tomorrow when a nightly with #86155 is released

diff --git a/tests/run-pass/function_calls/exported_symbol_good_unwind.rs b/tests/run-pass/function_calls/exported_symbol_good_unwind.rs
index 3dd3b8f2..71b799a1 100644
--- a/tests/run-pass/function_calls/exported_symbol_good_unwind.rs
+++ b/tests/run-pass/function_calls/exported_symbol_good_unwind.rs
@@ -2,16 +2,10 @@
 // found in this form" errors works without `-C prefer-dynamic` (`panic!` calls foreign function
 // `__rust_start_panic`).
 // no-prefer-dynamic
-#![feature(c_unwind, unboxed_closures, unwind_attributes)]
+#![feature(c_unwind, unboxed_closures)]
 
 use std::panic;
 
-#[no_mangle]
-#[unwind(allowed)]
-extern "C" fn good_unwind_allowed() {
-    panic!();
-}
-
 #[no_mangle]
 extern "C-unwind" fn good_unwind_c() {
     panic!();
@@ -29,11 +23,6 @@ extern "rust-call" fn good_unwind_rust_call(_: ()) -> ! {
 }
 
 fn main() -> ! {
-    extern "C" {
-        #[unwind(allowed)]
-        fn good_unwind_allowed();
-    }
-    panic::catch_unwind(|| unsafe { good_unwind_allowed() }).unwrap_err();
     extern "C-unwind" {
         fn good_unwind_c();
     }
diff --git a/tests/run-pass/function_calls/exported_symbol_unwind_allowed.rs b/tests/run-pass/function_calls/exported_symbol_unwind_allowed.rs
index 0e4ec573..fd5ec494 100644
--- a/tests/run-pass/function_calls/exported_symbol_unwind_allowed.rs
+++ b/tests/run-pass/function_calls/exported_symbol_unwind_allowed.rs
@@ -1,5 +1,4 @@
-// compile-flags: -Zmiri-disable-abi-check
-#![feature(unwind_attributes, c_unwind)]
+#![feature(c_unwind)]
 
 #[no_mangle]
 extern "C-unwind" fn unwind() {
@@ -7,8 +6,7 @@ extern "C-unwind" fn unwind() {
 }
 
 fn main() {
-    extern "C" {
-        #[unwind(allowed)]
+    extern "C-unwind" {
         fn unwind();
     }
     unsafe { unwind() }

@ghost
Copy link

ghost commented Aug 5, 2021

There are more failures in compile-fail tests (which are not run when run-pass tests fail):

failures:
    [compile-fail] compile-fail/abort-terminator.rs
    [compile-fail] compile-fail/function_calls/exported_symbol_bad_unwind2.rs
    [compile-fail] compile-fail/function_calls/exported_symbol_bad_unwind3.rs
    [compile-fail] compile-fail/panic/bad_miri_start_panic.rs
    [compile-fail] compile-fail/panic/panic_abort1.rs
    [compile-fail] compile-fail/panic/panic_abort2.rs
    [compile-fail] compile-fail/panic/panic_abort3.rs
    [compile-fail] compile-fail/panic/panic_abort4.rs
  • compile-fail/abort-terminator.rs and tests/compile-fail/function_calls/exported_symbol_bad_unwind3.rs fail with error[E0557]: feature has been removed.
  • Revision definition of tests/compile-fail/function_calls/exported_symbol_bad_unwind2.rs fails with abnormal termination: the program aborted execution -- unwinding past a #[rustc_allocator_nounwind] function seems to abort instead of triggering UB now.
  • tests/compile-fail/panic/bad_miri_start_panic.rs does not trigger the unwinding past a stack frame that does not allow unwinding error anymore. Adding #![feature(c_unwind)] fixes it.
  • All compile-fail/panic/panic_abort*.rs tests fail with this 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
    
    Which seems to be an ABI mismatch between panic_abort and std:
    pub unsafe extern "C" fn __rust_start_panic(_payload: *mut &mut dyn BoxMeUp) -> u32 {

    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;
    }

    Opened Use C-unwind ABI for __rust_start_panic in panic_abort #87787 to fix that.

@alexcrichton
Copy link
Member

Ah ok in that case I think I unfortunately don't think I have time to personally look more into this.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 6, 2021
Use `C-unwind` ABI for `__rust_start_panic` in `panic_abort`

The function originally has `C` ABI but is called using `C-unwind` ABI in `std`:
https://github.com/rust-lang/rust/blob/d4ad1cfc63ba5824196bfb2370451ddb5af2e020/library/std/src/panicking.rs#L49-L54
Which might be [problematic](rust-lang/miri#1745 (comment)) and triggers this [Miri error](rust-lang#87778 (comment)):
```
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.
@RalfJung
Copy link
Member

RalfJung commented Aug 7, 2021

The fix is ready at rust-lang/miri#1868, but bors is broken so we can't land it.

bors added a commit to rust-lang/miri that referenced this issue Aug 7, 2021
@RalfJung RalfJung mentioned this issue Aug 7, 2021
@bors bors closed this as completed in 57d8747 Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-miri Area: The miri tool C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants