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

rust_begin_unwind required for panic=abort #38281

Closed
phil-opp opened this issue Dec 10, 2016 · 20 comments
Closed

rust_begin_unwind required for panic=abort #38281

phil-opp opened this issue Dec 10, 2016 · 20 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug.

Comments

@phil-opp
Copy link
Contributor

Since a few nightlies, the compiler requires a rust_begin_unwind symbol again even though my no_std crate uses panic=abort:

In function `core::panicking::panic_fmt::hb1bd46c16a8d9cdb':
core.cgu-0.rs:(.text._ZN4core9panicking9panic_fmt17hb1bd46c16a8d9cdbE+0x38): undefined reference to `rust_begin_unwind'

The big problem is that defining a rust_begin_unwind function in Rust gives an error, too:

error: symbol `rust_begin_unwind` is already defined

I only works if I add a rust_begin_unwind function to a C file that is linked with the Rust crate.

@japaric
Copy link
Member

japaric commented Dec 10, 2016

STR

$ cargo new --bin app && cd $_

$ edit Cargo.toml && tail -n2 $_
[profile.dev]
panic = "abort"

$ edit src/main.rs && cat $_
#![feature(lang_items)]
#![no_main]
#![no_std]

#[no_mangle]
pub fn _start() -> ! {
    panic!()
}

#[lang = "panic_fmt"]
extern "C" fn panic_fmt() -> ! {
    loop {}
}

$ cargo rustc -- -C link-arg=-nostartfiles
error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/japaric/.multirust/toolchains/nightly-2016-12-06-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "/home/japaric/tmp/app/target/debug/deps/app-55df21ded2024f97.0.o" "-o" "/home/japaric/tmp/app/target/debug/deps/app-55df21ded2024f97" "-Wl,--gc-sections" "-pie" "-nodefaultlibs" "-L" "/home/japaric/tmp/app/target/debug/deps" "-L" "/home/japaric/.multirust/toolchains/nightly-2016-12-06-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "-Wl,-Bdynamic" "/home/japaric/.multirust/toolchains/nightly-2016-12-06-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-1357b93f.rlib" "-nostartfiles"
  = note: /home/japaric/.multirust/toolchains/nightly-2016-12-06-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-1357b93f.rlib(core-1357b93f.0.o): In function `core::panicking::panic_fmt':
/buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/libcore/num/bignum.rs:489: undefined reference to `rust_begin_unwind'
collect2: error: ld returned 1 exit status


error: aborting due to previous error

error: Could not compile `app`.

$ nm -C target/debug/deps/app*.0.o
0000000000000000 t rust_begin_unwind
0000000000000000 N __rustc_debug_gdb_scripts_section__
0000000000000000 T _start
0000000000000000 r str.0
0000000000000000 r str.1
0000000000000000 d app::_start::_MSG_FILE_LINE::h21fcbd8366a7e1b6
                 U core::panicking::panic::h194ce5d68a8f28a1
0000000000000000 t drop::h8b70c8901008835d

Bisecting:

BAD: rustc 1.15.0-nightly (daf8c1dfc 2016-12-05)

GOOD: rustc 1.15.0-nightly (ebeee0e27 2016-12-04)

AFAICT, the difference is that rust_begin_unwind is never a extern/global
symbol in recent nightlies like it used to be:

# with GOOD rustc
$ cargo rustc -- --emit=obj

$ nm -C ./target/debug/deps/app-*.o
0000000000000000 T rust_begin_unwind
0000000000000000 N __rustc_debug_gdb_scripts_section__
0000000000000000 T _start
0000000000000000 r str.0
0000000000000000 r str.1
0000000000000000 d app::_start::_MSG_FILE_LINE::h0939c9c7dafe8539
                 U core::panicking::panic::h194ce5d68a8f28a1
0000000000000000 t drop::he2bf721309cf573a

Changing the extern "C" fn to public doesn't help:

 #[lang = "panic_fmt"]
-extern "C" fn panic_fmt() -> ! {
+pub extern "C" fn panic_fmt() -> ! {
     loop {}
 }
# with BAD rustc
$ nm -C ./target/debug/deps/app-*.0.o
0000000000000000 t rust_begin_unwind
0000000000000000 N __rustc_debug_gdb_scripts_section__
0000000000000000 T _start
0000000000000000 r str.0
0000000000000000 r str.1
0000000000000000 d app::_start::_MSG_FILE_LINE::hb927abaa970af5cf
                 U core::panicking::panic::h194ce5d68a8f28a1
0000000000000000 t drop::hc6005949ff734e37

cc @michaelwoerister this may have been caused by #38117

@michaelwoerister
Copy link
Member

@japaric That analysis looks pretty correct to me. I suspect that adding #[no_mangle] in addition to pub would solve the problem. But I'll think about what a clean solution would be.

@brson, @alexcrichton: I guess lang-items should always have external linkage?

@alexcrichton
Copy link
Member

@michaelwoerister yeah unfortunately things like rust_begin_unwind have to be external, although ideally one day I'd like to fix that!

If it's not already #[no_mangle] and pub it likely should be though, I'd prefer to avoid special-casing these symbols too much.

@michaelwoerister
Copy link
Member

@alexcrichton OK.

#[no_mangle] is slightly confusing because the actual symbol name can be different from the function name (as in @japaric's example).

Also, should these symbols be re-exported from (c)dylibs? Probably not, right?

@alexcrichton
Copy link
Member

Yeah ideally we wouldn't export rust_begin_unwind from cdylibs, and I don't think it'll be required to link correctly either.

@steveklabnik steveklabnik added the A-linkage Area: linking into static, shared libraries and binaries label Dec 14, 2016
@japaric
Copy link
Member

japaric commented Dec 16, 2016

Sorry to be annoyning but a few people that work with the thumbv*-none-eabi* targets have bumped into this and asked about it on the #rust-embedded IRC channel. Is this likely to be fixed soon?

@michaelwoerister
Copy link
Member

@japaric Have you tried if adding #[no_mangle] helps?

@pftbest
Copy link
Contributor

pftbest commented Dec 16, 2016

@michaelwoerister
Adding #[no_mangle] to panic_fmt helped in my case. So how does this work?
It picks up default panic_fmt when it can't find the custom one?

@michaelwoerister
Copy link
Member

The #[no_mangle] attribute just prevents the compiler from hiding the symbol. The compiler generally will try to hide as many symbols as possible because it can make for smaller and faster code and allows LLVM to do more optimizations. The algorithm that determines what to hide has become a little more aggressive recently, which is why this pops up now.

So there is a workaround (i.e. #[no_mangle]) but it's not quite ideal. I'd rather that the compiler knows about these special symbols (or that we require, with a lint for example, that this lang-item and maybe others, have the right name and annotations).

@pftbest
Copy link
Contributor

pftbest commented Dec 16, 2016

I know what #[no_mangle] does, what I don't know is why do we get 'rust_begin_unwind is missing' error instead of 'panic_fmt is missing' error.

@michaelwoerister
Copy link
Member

Oh, that's because this already has some special handling in the compiler. As far as I can tell, no matter what actual name the function has, if it is marked with #[lang = "panic_fmt"] then it will be assigned "rust_begin_unwind" as symbol name. It's a bit strange.

@pftbest
Copy link
Contributor

pftbest commented Dec 16, 2016

@michaelwoerister
Oh, now it makes sense, thank you for explaining.
I was thinking there is some other version of panic_fmt that gets pulled in and makes calls to rust_begin_unwind 😀

@japaric
Copy link
Member

japaric commented Dec 17, 2016

@michaelwoerister

Have you tried if adding #[no_mangle] helps?

That works, thanks. Is that how this lang item is supposed to be used from now on? Or will the no no_mangle form work again in the future? Mainly want to know if I should go ahead and change all the references to panic_fmt in e.g. copper, f3, etc. right now or do nothing at all 😄.

@michaelwoerister
Copy link
Member

@japaric I think that's pretty much up to the tools and compiler teams to decide. I personally don't mind giving lang-items special treatment, since that allows us to later not require #[no_mangle] or even that they are exported.

dnseitz pushed a commit to dnseitz/RustOS that referenced this issue Jan 8, 2017
(cherry picked from commit 081105bbb60e1c76f222a6c34baac4eec0c83846)
(cherry picked from commit 1acc1ff5806f2fa101f726644d258eb4e4543540)
dnseitz pushed a commit to dnseitz/RustOS that referenced this issue Jan 9, 2017
(cherry picked from commit 081105bbb60e1c76f222a6c34baac4eec0c83846)
(cherry picked from commit 1acc1ff5806f2fa101f726644d258eb4e4543540)
ppannuto added a commit to tock/tock that referenced this issue Feb 2, 2017
This was pretty smooth and painless, mostly fixing up global variable
names to be all caps b/c that's caught now and a few structures got
smaller (whoo!)

One gotcha, the `#[lang=panic_fmt]` now requires you to also add a
`#[no_mangle]` directive, otherwise the symbol will be dropped and
you'll get an undefined reference to `rust_begin_unwind` (which is
what our `panic_fmt` function gets silently renamed to courtesy of
the magic of `#[lang=panic_fmt]`).
More info at rust-lang/rust#38281

An unresolved mystery, imix currently gives this warning:

    warning: function panic_fmt is marked #[no_mangle], but not exported,
    #[warn(private_no_mangle_fns)] on by default
      --> src/io.rs:40:1
       |
    40 | pub unsafe extern "C" fn panic_fmt(args: Arguments, file: &'static str, line: u32) -> ! {
       | ^

Now, of course, removing no_mangle will result in the undefined
reference as described above, with it this at least compiles, unclear
what's different about imix than the other platforms that don't throw
this warning.

Run-tested on hail. Compile-tested on every other platform.
robert-w-gries added a commit to robert-w-gries/rxinu that referenced this issue Mar 5, 2017
* Import compiler builtin crate to fix issue with _umod and _udiv
* Trim unnecessary Makefile variables and flags
* Keep the BLOCK and ALIGN commands in linker.ld for now
* Fix linking error `undefiend reference to rust_begin_unwind`
  * `panic_fmt` was mangled through optimization
  * `panic_fmt` becomes `rust_begin_unwind`
  * Solution: add `#[no_mangle]` before `panic_fmt`
  * [Rust Issue](rust-lang/rust#38281)
@urschrei
Copy link
Contributor

@alexcrichton @michaelwoerister Could someone clarify how to fix this for stable builds?
I haven't defined #![no_main]or #![no_std] for my crate (Cargo.toml), but linking fails on Windows using latest stable.

@alexcrichton
Copy link
Member

@urschrei you may be running into #18807, can you test the workaround mentioned?

@urschrei
Copy link
Contributor

urschrei commented Apr 29, 2017

@alexcrichton Just adding a spare function still fails with the same error. I assume adding it to lib.rs instead of ffi.rs doesn't matter (Also I opened #41609, but lmk if it's a duplicate and I'll close it)

@urschrei
Copy link
Contributor

Oops, turns out it does have to be in lib.rs. Link step passes now.

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 26, 2017
kevinaboos pushed a commit to theseus-os/Theseus that referenced this issue Jul 27, 2017
d-r-q pushed a commit to d-r-q/maroz that referenced this issue Dec 7, 2017
bradjc pushed a commit to tock/libtock-c that referenced this issue May 25, 2018
This was pretty smooth and painless, mostly fixing up global variable
names to be all caps b/c that's caught now and a few structures got
smaller (whoo!)

One gotcha, the `#[lang=panic_fmt]` now requires you to also add a
`#[no_mangle]` directive, otherwise the symbol will be dropped and
you'll get an undefined reference to `rust_begin_unwind` (which is
what our `panic_fmt` function gets silently renamed to courtesy of
the magic of `#[lang=panic_fmt]`).
More info at rust-lang/rust#38281

An unresolved mystery, imix currently gives this warning:

    warning: function panic_fmt is marked #[no_mangle], but not exported,
    #[warn(private_no_mangle_fns)] on by default
      --> src/io.rs:40:1
       |
    40 | pub unsafe extern "C" fn panic_fmt(args: Arguments, file: &'static str, line: u32) -> ! {
       | ^

Now, of course, removing no_mangle will result in the undefined
reference as described above, with it this at least compiles, unclear
what's different about imix than the other platforms that don't throw
this warning.

Run-tested on hail. Compile-tested on every other platform.
gz added a commit to gz/rust-cpuid that referenced this issue Jun 11, 2018
@steveklabnik
Copy link
Member

Ping from triage!

I attempted to update the code:

> cat .\src\main.rs
#![feature(panic_handler)]
#![no_main]
#![no_std]

#[no_mangle]
pub fn _start() -> ! {
    panic!()
}

use core::panic::PanicInfo;

#[panic_handler]
#[no_mangle]
pub fn panic(_info: &PanicInfo) -> ! {
    loop { }
}

but I got a different error, possibly because I'm on windows

>  cargo rustc -- -C link-arg=-nostartfiles
   Compiling app v0.1.0 (file:///C:/Users/steve/tmp/app)
error: linking with `link.exe` failed: exit code: 1561
  |
  = note: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Community\\VC\\Tools\\MSVC\\14.15.26726\\bin\\HostX64\\x64\\link.exe" "/NOLOGO" "/NXCOMPAT" "/LIBPATH:C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "C:\\Users\\steve\\tmp\\app\\target\\debug\\deps\\app-3187309a36d3b090.1aq5w1s18bk2pb4a.rcgu.o" "/OUT:C:\\Users\\steve\\tmp\\app\\target\\debug\\deps\\app-3187309a36d3b090.exe" "/OPT:REF,NOICF" "/DEBUG" "/NATVIS:C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\etc\\intrinsic.natvis" "/NATVIS:C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\etc\\liballoc.natvis" "/NATVIS:C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\etc\\libcore.natvis" "/LIBPATH:C:\\Users\\steve\\tmp\\app\\target\\debug\\deps" "/LIBPATH:C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcore-68de561760a7c421.rlib" "C:\\Users\\steve\\.rustup\\toolchains\\nightly-x86_64-pc-windows-msvc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcompiler_builtins-a0d3327a110151c3.rlib" "-nostartfiles"
  = note: LINK : warning LNK4044: unrecognized option '/nostartfiles'; ignored
          LINK : fatal error LNK1561: entry point must be defined


error: aborting due to previous error

error: Could not compile `app`.

I tried to figure out the equivalent msvc option, but it wasn't clear...

@phil-opp
Copy link
Contributor Author

phil-opp commented Oct 7, 2018

Instead of defining a lang item, the panic_implementation attribute was implemented, which was later stabilized as panic_handler. For some time, the rust_begin_unwind issue still existed with the panic_implementation attribute, but it was fixed before stabilizing.

So this issue was superseded by #51342, which was fixed. Therefore I'm going to close this issue.

@phil-opp phil-opp closed this as completed Oct 7, 2018
pinocchio338 added a commit to pinocchio338/get_cpu_id that referenced this issue Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

8 participants