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

LTO causes undefined references to core::panicking::panic #79

Closed
FenrirWolf opened this issue Sep 30, 2016 · 14 comments · Fixed by #80
Closed

LTO causes undefined references to core::panicking::panic #79

FenrirWolf opened this issue Sep 30, 2016 · 14 comments · Fixed by #80

Comments

@FenrirWolf
Copy link

FenrirWolf commented Sep 30, 2016

Update

After #80, this only occurs with profile.dev (+debug-assertions) + LTO. To fix this we'll have to create newtypes over primitives types that intrinsics::abort() on bad inputs (division by zero) and overflow.


Getting a funky error when attempting to use rustc-builtins for nintendo 3DS shenanigans instead the prebuilt copy of libcompiler-rt that we've been using up to this point:

   Compiling ctru-rs v0.4.0 (https://github.com/FenrirWolf/ctru-rs?branch=builtins#27eb3caf)
   Compiling rustc_builtins v0.1.0 (https://github.com/japaric/rustc-builtins.git#69e93de9)
   Compiling alloc_system3ds v0.1.0 (https://github.com/rust3ds/alloc_system3ds?rev=da38c94#da38c941)
   Compiling ctru-sys v0.2.0 (https://github.com/FenrirWolf/ctru-rs?branch=builtins#27eb3caf)
   Compiling rust3ds-template v0.1.0 (file:///home/fenrir/projects/rust3ds-template)
error: linking with `arm-none-eabi-gcc` failed: exit code: 1
  |
  = note: "arm-none-eabi-gcc" "-specs=3dsx.specs" "-march=armv6k" "-mtune=mpcore" "-mfloat-abi=hard" "-mtp=soft" "-L" "/home/fenrir/.xargo/lib/rustlib/3ds/lib" "/home/fenrir/projects/rust3ds-template/target/3ds/release/rust3ds_template.0.o" "-o" "/home/fenrir/projects/rust3ds-template/target/3ds/release/rust3ds_template.elf" "-Wl,--gc-sections" "-nodefaultlibs" "-L" "/home/fenrir/projects/rust3ds-template/target/3ds/release/deps" "-L" "/opt/devkitPro/libctru/lib" "-L" "/home/fenrir/.xargo/lib/rustlib/3ds/lib" "-Wl,-Bstatic" "-Wl,-Bdynamic" "/tmp/rustc.n3iL9DJW0uUT/libctru-56422efc011c55b9.rlib" "/tmp/rustc.n3iL9DJW0uUT/librustc_builtins-5c8a87f356eb958c.rlib" "-lc" "-lm" "-lsysbase" "-lc" "-lsysbase" "-lc"
  = note: /tmp/rustc.n3iL9DJW0uUT/librustc_builtins-5c8a87f356eb958c.rlib(rustc_builtins-5c8a87f356eb958c.0.o): In function `__udivmodsi4':
rustc_builtins.cgu-0.rs:(.text.__udivmodsi4+0xb4): undefined reference to `core::panicking::panic::h6d47421a09de1633'
/tmp/rustc.n3iL9DJW0uUT/librustc_builtins-5c8a87f356eb958c.rlib(rustc_builtins-5c8a87f356eb958c.0.o): In function `__udivmoddi4':
rustc_builtins.cgu-0.rs:(.text.__udivmoddi4+0x304): undefined reference to `core::panicking::panic::h6d47421a09de1633'
rustc_builtins.cgu-0.rs:(.text.__udivmoddi4+0x30c): undefined reference to `core::panicking::panic::h6d47421a09de1633'
rustc_builtins.cgu-0.rs:(.text.__udivmoddi4+0x314): undefined reference to `core::panicking::panic::h6d47421a09de1633'
/tmp/rustc.n3iL9DJW0uUT/librustc_builtins-5c8a87f356eb958c.rlib(rustc_builtins-5c8a87f356eb958c.0.o): In function `__aeabi_uidiv':
rustc_builtins.cgu-0.rs:(.text.__aeabi_uidiv+0xa0): undefined reference to `core::panicking::panic::h6d47421a09de1633'
/tmp/rustc.n3iL9DJW0uUT/librustc_builtins-5c8a87f356eb958c.rlib(rustc_builtins-5c8a87f356eb958c.0.o):rustc_builtins.cgu-0.rs:(.text.__divsi3+0x40): more undefined references to `core::panicking::panic::h6d47421a09de1633' follow
collect2: error: ld returned 1 exit status

error: aborting due to previous error

However, the error only occurs if lto = true is set in Cargo.toml. If LTO is disabled, the result is a successful build.

@japaric
Copy link
Member

japaric commented Sep 30, 2016

I couldn't set up the 3ds dev environment but still managed to reproduce this with:

// src/main.rs
#![feature(lang_items)]
#![no_main]
#![no_std]

extern crate rustc_builtins;

use core::ptr;

#[no_mangle]
pub unsafe fn _start() -> u32 {
    let x = ptr::read_volatile(0x0 as *const u32);
    let y = ptr::read_volatile(0x0 as *const u32);

    x / y
}

#[no_mangle]
pub fn __aeabi_unwind_cpp_pr0() {}

#[lang = "panic_fmt"]
extern fn panic_fmt() {}
# Cargo.toml
[package]
name = "foo"
version = "0.1.0"
authors = ["Jorge Aparicio <japaricious@gmail.com>"]

[dependencies]
rustc_builtins = { git = "https://github.com/japaric/rustc-builtins" }

[profile.dev]
panic = "abort"

[profile.release]
panic = "abort"
lto = true
# .cargo/config
[build]
rustflags = ["-C", "link-arg=-nostartfiles"]
// thumbv6m-none-eabi.json
{
    "arch": "arm",
    "data-layout": "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64",
    "executables": true,
    "features": "+strict-align",
    "linker": "arm-none-eabi-gcc",
    "llvm-target": "thumbv6m-none-eabi",
    "os": "none",
    "target-endian": "little",
    "target-pointer-width": "32"
}
$ xargo build --target thumbv6m-none-eabi --release
   Compiling foo v0.1.0 (file:///home/japaric/tmp/foo)
error: linking with `arm-none-eabi-gcc` failed: exit code: 1
  |
  = note: "arm-none-eabi-gcc" "-L" "/home/japaric/.xargo/lib/rustlib/thumbv6m-none-eabi/lib" "/home/japaric/tmp/foo/target/thumbv6m-none-eabi/release/foo.0.o" "-o" "/home/japaric/tmp/foo/target/thumbv6m-none-eabi/release/foo" "-Wl,--gc-sections" "-nodefaultlibs" "-L" "/home/japaric/tmp/foo/target/thumbv6m-none-eabi/release/deps" "-L" "/home/japaric/.xargo/lib/rustlib/thumbv6m-none-eabi/lib" "-Wl,-Bstatic" "-Wl,-Bdynamic" "/tmp/rustc.Jxy0hf5HsfzM/librustc_builtins-5c8a87f356eb958c.rlib" "-nostartfiles"
  = note: /tmp/rustc.Jxy0hf5HsfzM/librustc_builtins-5c8a87f356eb958c.rlib(rustc_builtins-5c8a87f356eb958c.0.o): In function `__aeabi_uidiv':
rustc_builtins.cgu-0.rs:(.text.__aeabi_uidiv+0xde): undefined reference to `core::panicking::panic::h2a0ea99cd46c9ef6'
collect2: error: ld returned 1 exit status


error: aborting due to previous error

error: Could not compile `foo`.

To learn more, run the command again with --verbose.
error: `cargo` process didn't exit successfully

@japaric
Copy link
Member

japaric commented Sep 30, 2016

So, what I think is happening is that, because core participates in LTO but rustc_builtins doesn't, LLVM looks at _start and says "oh, this can't panic let's drop the panicking::panic symbol". After LTO, the object looks like this

(this is foo.0.o)

00000000 <_start>:
   0:   b580            push    {r7, lr}
   2:   2100            movs    r1, #0
   4:   6808            ldr     r0, [r1, #0]
   6:   6809            ldr     r1, [r1, #0]
   8:   f7ff fffe       bl      0 <__aeabi_uidiv>
   c:   bd80            pop     {r7, pc}

Disassembly of section .text.__aeabi_unwind_cpp_pr0:

00000000 <__aeabi_unwind_cpp_pr0>:
   0:   4770            bx      lr

Disassembly of section .text.rust_begin_unwind:

00000000 <rust_begin_unwind>:
   0:   4770            bx      lr

Note that there is an undefined reference to __aeabi_uidiv (this is expected).

But then when linking happens, the linker links in the __aeabi_uidiv symbol but that symbol depends on panicking::panic and there is no other object that provides it! The core crate has the panicking::panic symbol but that crate doesn't participate in the linking process because it already was LTOed.

@japaric
Copy link
Member

japaric commented Sep 30, 2016

I think what we could do is:

  • Have rustc_builtins participate in LTO. But than would bring back the infinite recursive call problem with __aeabi_memcpy.
  • Don't use anything that's in core in the rustc_builtins crate. This is a non-starter.
  • Perhaps, have rustc_builtins participate in LTO but isolate the symbols that have the infinite recursive call problem into another crate that doesn't participate in LTO. rustc_builtins would depend on that other crate.
  • Don't use panic! in rustc_builtins. That would fix this particular case but there would still be problems with other symbols defined in core that get used in rustc_builtins but that don't get used in the top-crate/executable.

cc @alexcrichton

@Amanieu
Copy link
Member

Amanieu commented Sep 30, 2016

Another alternative: move the memcpy stuff (including the aeabi aliases) to a separate no_builtin crate.

@japaric
Copy link
Member

japaric commented Sep 30, 2016

@Amanieu yeah, that's the third alternative I mentioned 😄.

@alexcrichton
Copy link
Member

Morally I feel like "Don't use anything that's in core in the rustc_builtins crate" is the correct approach here because that's what the dependencies actually look like. Perhaps though we could tweak this to "Don't use any symbol that's in core in the rustc_builtins crate".

That is, almost all of libcore is tagged with #[inline], and we're already looking at symbols in the compiled output, so perhaps we could assert that there are no undefined symbols that we don't know about?

@japaric
Copy link
Member

japaric commented Sep 30, 2016

@alexcrichton

$ nm -u target/debug/librustc_builtins.rlib

rustc_builtins.0.o:
                 U rust_eh_personality
                 U _Unwind_Resume
                 U _ZN4core9panicking5panic17h1a2d1a6b50eaa468E

$ nm -u target/release/librustc_builtins.rlib

rustc_builtins.0.o:
                 U _ZN4core9panicking5panic17h1a2d1a6b50eaa468E

Should we stop using panic! in our implementation and switch to compilerrt_abort?

On that note, the libcompiler-rt.a that we used to distribute is filled with undefined compilerrt_abort_impl symbols. I don't know how that used to work.

@alexcrichton
Copy link
Member

Yeah in my mind this crate should never panic, but if it must be fallible it's safe to tear down everything.

@japaric
Copy link
Member

japaric commented Sep 30, 2016

Yeah in my mind this crate should never panic

Some of the implementations have branches that panic for stuff like division by zero but we should never reach those paths because Rust would trigger a panic before those intrinsics are called (I can't where in the compiler this behavior is implemented).

I think the only way to reach those branches is to directly call the intrinsic (unsafe) with input = 0 or use the intrinsics::unchecked_div in the standard library. unchecked_div says that it's UB to call it with denominator = 0. Does that give us permission to replace the panics in our division by zero branches with intrinsics::unreachable?

@alexcrichton
Copy link
Member

I'd prefer to stick with intrinsics::abort for now, but in general I think unreachable should be ok in the long term (once we've verified everything)

japaric pushed a commit that referenced this issue Sep 30, 2016
@japaric japaric mentioned this issue Sep 30, 2016
japaric pushed a commit that referenced this issue Oct 1, 2016
japaric pushed a commit that referenced this issue Oct 6, 2016
japaric pushed a commit to japaric/f3 that referenced this issue Oct 6, 2016
japaric pushed a commit that referenced this issue Oct 7, 2016
japaric pushed a commit that referenced this issue Oct 7, 2016
japaric pushed a commit that referenced this issue Oct 7, 2016
@japaric japaric reopened this Oct 7, 2016
@alexcrichton
Copy link
Member

Some more guards for this were added in #166, so I think this is resolved.

@BartMassey
Copy link

This seems to have regressed again. ssh://git@gitlab.com/BartMassey/mixy on Linux/x86-64, remove the -O flag in the Makefile. Shall I file a separate issue for the regression?

@alexcrichton
Copy link
Member

@BartMassey yes please do!

@BartMassey
Copy link

@alexcrichton Issue #245. I think it's probably all my fault, but I'm not sure exactly what to do, so…

paoloteti added a commit to paoloteti/compiler-builtins that referenced this issue Sep 14, 2018
Temporary workaround for the well known
"the undefined references problem for debug-assertions+lto" (rust-lang#79)
paoloteti added a commit to paoloteti/compiler-builtins that referenced this issue Sep 14, 2018
Temporary workaround for the well known
"the undefined references problem for debug-assertions+lto" (rust-lang#79)
paoloteti added a commit to paoloteti/compiler-builtins that referenced this issue Sep 14, 2018
Temporary workaround for the well known
"undefined references problem for debug-assertions+lto" (rust-lang#79)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants