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

It's impossible to write a panic crate for no_std #48661

Closed
glandium opened this issue Mar 2, 2018 · 10 comments
Closed

It's impossible to write a panic crate for no_std #48661

glandium opened this issue Mar 2, 2018 · 10 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@glandium
Copy link
Contributor

glandium commented Mar 2, 2018

(Originally filed as rust-lang/cargo#5106, but I think it's actually a rustc issue)

I've created a workspace and have multiple crates, all of which are no_std and need a panic_fmt implementation, so I created a separate crate that can be shared for that. Then I built a cdylib which built and linked fine, but failed at runtime with "undefined symbol: rust_begin_unwind".

Here's how to reproduce (on Linux, but I expect this should fail the same on all platforms):

$ cargo new --lib bar
$ cd bar
$ cargo new --lib panic
$ cat > panic/src/lib.rs <<EOF
#![no_std]
#![feature(lang_items)]

#[cfg(not(test))]
#[lang = "panic_fmt"]
#[no_mangle]
pub extern "C" fn panic_fmt(_: core::fmt::Arguments, _: &'static str, _: u32, _: u32) -> ! {
    loop {}
}
EOF
$ cat >> Cargo.toml <<EOF
panic = { path = "panic" }

[profile.dev]
panic = "abort"

[profile.release]
panic = "abort"

[lib]
crate-type = ["cdylib"]
EOF
$ cat > src/lib.rs <<EOF
#![no_std]
extern crate panic;

#[no_mangle]
pub fn foo() -> ! {
    panic!("");
}
EOF
$ cargo +nightly build --release
   Compiling bar v0.1.0 (file:///tmp/bar)
    Finished release [optimized] target(s) in 0.11 secs

Note how this all compiled and linked fine. But:

$ objdump -T target/release/libbar.so 

target/release/libbar.so:     file format elf64-x86-64

DYNAMIC SYMBOL TABLE:
0000000000000000  w   D  *UND*	0000000000000000 __cxa_finalize
0000000000000000  w   D  *UND*	0000000000000000 _ITM_registerTMCloneTable
0000000000000000      D  *UND*	0000000000000000 rust_begin_unwind
0000000000000000  w   D  *UND*	0000000000000000 _ITM_deregisterTMCloneTable
0000000000000000  w   D  *UND*	0000000000000000 __gmon_start__
0000000000000550 g    DF .text	000000000000000f foo

rust_begin_unwind is undefined.

The reason this is happening is because nothing is using the symbol in any library that appears before libpanic on the linker command line, and the only thing that does use the symbol is libcore, which comes after. So the linker happily removes it.

If I force libpanic to be added last on the linker command line, it works:

$ RUSTFLAGS="-C link-arg=target/release/deps/libpanic-3af7779d45985ae9.rlib" cargo +nightly build --release
   Compiling bar v0.1.0 (file:///tmp/bar)
    Finished release [optimized] target(s) in 0.11 secs

$ objdump -T target/release/libbar.so 

target/release/libbar.so:     file format elf64-x86-64

DYNAMIC SYMBOL TABLE:
0000000000000000  w   D  *UND*	0000000000000000 __cxa_finalize
0000000000000000  w   D  *UND*	0000000000000000 _ITM_registerTMCloneTable
0000000000000000  w   D  *UND*	0000000000000000 _ITM_deregisterTMCloneTable
0000000000000000  w   D  *UND*	0000000000000000 __gmon_start__
0000000000000620 g    DF .text	0000000000000002 rust_begin_unwind
0000000000000550 g    DF .text	000000000000000f foo

Relatedly, this makes me think cargo should probably add -Wl,-z,defs:

$ RUSTFLAGS="-C link-arg=-Wl,-z,defs" cargo +nightly build --release
   Compiling bar v0.1.0 (file:///tmp/bar)
error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/glandium/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/tmp/bar/target/release/deps/bar.bar0-b17b6035155785f4e639ba83018b4206.rs.rcgu.o" "-o" "/tmp/bar/target/release/deps/libbar.so" "-Wl,--version-script=/tmp/rustc.vWTWjhp8JGrI/list" "-Wl,--gc-sections" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-nodefaultlibs" "-L" "/tmp/bar/target/release/deps" "-L" "/home/glandium/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/tmp/bar/target/release/deps/libpanic-3af7779d45985ae9.rlib" "/home/glandium/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-80b45058e2fc80f9.rlib" "-shared" "-Wl,-z,defs" "-Wl,-Bdynamic"
  = note: /home/glandium/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-80b45058e2fc80f9.rlib(core-80b45058e2fc80f9.core0.rcgu.o): In function `core::panicking::panic_fmt':
          /checkout/src/libcore/num/bignum.rs:263: undefined reference to `rust_begin_unwind'
          collect2: error: ld returned 1 exit status
@glandium
Copy link
Contributor Author

glandium commented Mar 2, 2018

FWIW, LTO works around the issue.

@glandium
Copy link
Contributor Author

glandium commented Mar 2, 2018

Another work around: using the dylib crate-type instead of cdylib, because of:

if crate_type == config::CrateTypeDylib &&
trans.crate_info.compiler_builtins != Some(cnum) {
cmd.link_whole_rlib(&fix_windows_verbatim_for_gcc(&dst));

But this doesn't help for the bin crate-type, which actually fails to link because of the missing rust_begin_unwind symbol.

All in all, I just wish there was a way for me to make that library appear after libcore on the linker command.

@glandium
Copy link
Contributor Author

glandium commented Mar 2, 2018

Another workaround: building libpanic as both a rlib and a staticlib, and add:

#[link(name = "panic", kind = "static")]
extern "C" {}

in the same file that contains extern crate panic, and add -L target/release to the linker flags, which I guess is only possible with a build script...

@glandium
Copy link
Contributor Author

glandium commented Mar 2, 2018

This build.rs allows me to build as a workaround:

use std::env;
use std::path::Path;

fn main() {
    let out_dir = env::var_os("OUT_DIR").unwrap();
    let out_dir = Path::new(&out_dir).parent().unwrap().parent().unwrap().parent().unwrap();
    println!("cargo:rustc-link-search=native={}", out_dir.to_str().unwrap());
    println!("cargo:rustc-link-lib=static=panic");
}

The triple parent() is kind of awful, I wish cargo exposed its base target directory instead.

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 6, 2018
@japaric
Copy link
Member

japaric commented Mar 15, 2018

I don't remember the differences between cdylbs and plain executables but this sounds like #47074 / #18807, which has a proposed rustc fix in #47074 (comment). @glandium could you try wrapping the linker arguments in --start-group / --end-group?

@glandium
Copy link
Contributor Author

It works, but only if libcore and libpanic are between the same --start-group/--end-group. That doesn't help with the second half of #44489 (comment) though.

@japaric
Copy link
Member

japaric commented Mar 15, 2018

It works, but only if libcore and libpanic are between the same --start-group/--end-group

Yes, that's what #47074 (comment) proposes to do.

FWIW, you may be able to avoid the linking errors if you disable both incr comp and parallel codegen in your dev profile (IDK if you also get the error using the release profile). We have run into this problem in Cortex-M land because the cortex-m-rt provides a panic implementation (the panic_fmt lang item) and both disabling incr comp and parallel codegen fixes the linking issue in must, but not all, cases.

@glandium
Copy link
Contributor Author

@alexcrichton Sadly, #49316 didn't fix this :(

alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 4, 2018
It turns out that the support in rust-lang#49316 wasn't enough to handle all cases
notably the example in rust-lang#48661. The underlying bug was connected to panic=abort
where lang items were listed in the `missing_lang_items` sets but didn't
actually exist anywhere.

This caused the linker backend to deduce that start-group/end-group wasn't
needed because not all items were defined. Instead the missing lang items that
don't actually need to have a definition are filtered out and not considered for
the start-group/end-group arguments

Closes rust-lang#48661
@alexcrichton
Copy link
Member

I believe this should be fixed by #49672

bors added a commit that referenced this issue Apr 7, 2018
…haelwoerister

Fix another circular deps link args issue

It turns out that the support in #49316 wasn't enough to handle all cases
notably the example in #48661. The underlying bug was connected to panic=abort
where lang items were listed in the `missing_lang_items` sets but didn't
actually exist anywhere.

This caused the linker backend to deduce that start-group/end-group wasn't
needed because not all items were defined. Instead the missing lang items that
don't actually need to have a definition are filtered out and not considered for
the start-group/end-group arguments

Closes #48661
@glandium
Copy link
Contributor Author

glandium commented Apr 8, 2018

I can confirm this is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants