[regression] parallel LTO breaks linking because it internalizes weak symbols

[japaric]

STR

With LTO

$ git clone https://github.com/japaric/embedded-demo

$ cd embedded-demo

$ xargo rustc --release -v -- -C lto -Z print-link-args
error: linking with `arm-none-eabi-ld` failed: exit code: 1
  |
  = note: "arm-none-eabi-ld" (..)
  = note: /home/japaric/tmp/cortex-m-quickstart/target/thumbv7m-none-eabi/release/deps/demo-cbfeda05d006136a.stm32f103xx0-3aeab9466184a3fc8b7128118183614f.rs.rcgu.o: In function `WWDG':
          stm32f103xx0-3aeab9466184a3fc8b7128118183614f.rs:(.text+0x0): undefined reference to `DEFAULT_HANDLER'

Linker arguments:

"arm-none-eabi-ld"
"-L"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.aligned0-82013031e0adce00680863804baa6d8a.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.bare_metal0-808e09050a3d8f805631ff0277b6d5f4.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.core0-5fd795c6d6efb3b3ac7a93ecfc07e7e8.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.cortex_m0-eef2b118572823d6b94a55e824ed5889.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.cortex_m_rt0-a8ed1a542842e08c4e593519be42c026.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.demo0-bda6ff7720758957fa6c47e3e0108053.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.r00-d69ab77365edbeda5c83b8029590a7b6.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.stm32f103xx0-b5844958e960b076c42287432ab6341a.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.vcell0-17ed04a11350e05da18a7742432984b.rs.rcgu.o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87.volatile_register0-fb687636cdd4964c5e2d4a17ac1819ef.rs.rcgu.o"
"-o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-cc40ab28f2256f87"
"--gc-sections"
"-L"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps"
"-L"
"/home/japaric/tmp/embedded-demo/target/release/deps"
"-L"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/build/cortex-m-rt-8aa404db3cc9f96b/out"
"-L"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib"
"-Bstatic"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib/libcompiler_builtins-e9756467978f74f8.rlib"
"-Tlink.x"
"-Bdynamic"

Without LTO

$ xargo rustc --release -- -Z print-link-args && echo OK
OK

Linker arguments:

"arm-none-eabi-ld"
"-L"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-7be3323439223b43.demo0.rcgu.o"
"-o"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/demo-7be3323439223b43"
"--gc-sections"
"-L"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps"
"-L"
"/home/japaric/tmp/embedded-demo/target/release/deps"
"-L"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/build/cortex-m-rt-8aa404db3cc9f96b/out"
"-L"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib"
"-Bstatic"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libstm32f103xx-0620e682f6df62cd.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libcortex_m_rt-637507d2b1db7555.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libr0-a7dcce0e9369941f.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libcortex_m-690455d6e615b2d4.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libvolatile_register-45547ab3b9a80964.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libvcell-89a145f57bb869ee.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libbare_metal-ddaad95c84c49e08.rlib"
"/home/japaric/tmp/embedded-demo/target/thumbv7m-none-eabi/release/deps/libaligned-61d763743c0a5839.rlib"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib/libcore-2fea0bb2226aafe6.rlib"
"/home/japaric/.xargo/lib/rustlib/thumbv7m-none-eabi/lib/libcompiler_builtins-e9756467978f74f8.rlib"
"-Tlink.x"
"-Bdynamic"

Note the order in which objects (crates) are passed to the linker between an LTO and a non-LTO build. The LTO build more or less reverses the order.

stm32f103xx has an undefined symbol, DEFAULT_HANDLER, which is provided by its dependency cortex-m-rt. The LTO build fails to link because cortex-m-rt appears before stm32f103xx in the list of linker arguments.

---

With the old, non-parallel LTO there was only a single object so there was no linking problem.

cc @alexcrichton Is there an option to get the old, non-parallel LTO on a recent nightly? I tried codegen-units = 1 and I still got parallel LTO.

[alexcrichton]

@japaric still digesting, but -Z thinlto=no can turn off the parallel version on nightly

[alexcrichton]

Hm ok this is surprising! I thought linkers didn't care about ordering of objects, just of libraries...

@japaric if you take the failing linker command line and use the -Wl,-( and -Wl,-) (I think that's the syntax?) arguments around the object files (to tell the linker to consider them as a group) does that work? I also wouldn't be surprised if a symbol was accidentally stripped...

[japaric]

@alexcrichton --start-group / --end-group (I'm using ld, not gcc) doesn't work. The symbol is in libcortex_m_rt.rcgu.o though but it became non-global (local?) for some reason:

$ arm-none-eabi-nm -C "demo-cc40ab28f2256f87.cortex_m_rt0-a8ed1a542842e08c4e593519be42c026.rs.rcgu.o"
00000000 W BUS_FAULT
00000000 t DEFAULT_HANDLER
         U _ebss

The symbol is global (but weak) in the rlib:

$ arm-none-eabi-nm -C libcortex_m_rt-637507d2b1db7555.rlib
00000000 W BUS_FAULT
00000000 W DEFAULT_HANDLER
         U _ebss

[japaric]

I thought linkers didn't care about ordering of objects, just of libraries...

It certainly matters, at least for static libraries. .a and .rlib are just archives of objects (.o).

[alexcrichton]

Ok interesting! I think that's the bug then (ThinLTO can be a little aggressive about internalizing by accident sometimes).

Do you think you'd be able to minimize this a bit to where it's just a symbol being internalized by accident by the LTO passes?

[japaric]

@alexcrichton linkage = "weak" is required to trigger the problem. Repro below:

// file: foo/src/lib.rs

#![feature(linkage)]
#![no_std]

#[linkage = "weak"] // <- it works without this
#[no_mangle]
pub extern "C" fn FOO() -> i32 {
    0
}

// file: bar/src/lib.rs
// Cargo.toml
// [dependencies]
// foo = { path = "../foo" }

#![no_std]

extern crate foo;

extern "C" {
    fn FOO() -> i32;
}

pub fn bar() -> i32 {
    unsafe { FOO() }
}

// file: app/src/main.rs
// Cargo.toml
// [dependencies]
// bar = { path = "../bar" }

#![feature(lang_items)]
#![feature(start)]
#![no_std]

extern crate bar;

use core::ptr;

#[lang = "eh_personality"]
fn eh_personality() {}

#[lang = "panic_fmt"]
fn panic_fmt() {}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
    let x = bar::bar();
    // prevent LLVM from optimizing the call to `bar` away
    unsafe {
        ptr::read_volatile(&x);
    }
    0
}

#[link(name = "c")]
extern "C" {}

$ cd app
$ cargo rustc --release -- -C lto
(..)
  = note: /home/japaric/tmp/app/target/release/deps/app-fb503aa189655f1d.app0-fe76e448b3289da3a00eec0c3c849ae7.rs.rcgu.o: In function `main':
          app0-fe76e448b3289da3a00eec0c3c849ae7.rs:(.text.main+0x2): undefined reference to `FOO'
          collect2: error: ld returned 1 exit status

[japaric]

Removing #[linkage = "weak"] doesn't fix the problem I originally reported. Adding --start-group / --end-group to that still doesn't fix the problem: DEFAULT_HANDLER is still non-global (internal) after parallel LTO.

[alexcrichton]

@japaric aha weak linkage would definitely do it! I was able to reproduce locally with weak linkage and then it was fixed with the weak linkage removed.

Although for you removing the weak linkage didn't fix it?

[alexcrichton]

Oh I see, maybe there's two bugs at play then? Would it be possible to minimize with the weak linkage removed from the original case as well?

[alexcrichton]

Ok the first test case reduced a bit is:

$ cat foo.rs
#![feature(linkage)]

pub mod foo {
    #[linkage = "weak"] // <- it works without this
    #[no_mangle]
    pub extern "C" fn FOO() -> i32 {
        0
    }
}

mod bar {
    extern "C" {
        fn FOO() -> i32;
    }

    pub fn bar() -> i32 {
        unsafe { FOO() }
    }
}

fn main() {
    bar::bar();
}
$ rustc +nightly foo.rs -O
$ rustc +nightly foo.rs -O -C codegen-units=8
error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-L" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "foo.foo0-317d481089b8c8fe83113de504472633.rs.rcgu.o" "foo.foo1-317d481089b8c8fe83113de504472633.rs.rcgu.o" "foo.foo2-317d481089b8c8fe83113de504472633.rs.rcgu.o" "-o" "foo" "foo.crate.allocator.rcgu.o" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-nodefaultlibs" "-L" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,-Bstatic" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-c0e00a209103bd4d.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc_jemalloc-d1820c7fa19979f2.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-7c5a3388e5631edc.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-da40b64aab02e5af.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc_system-c0187bacb4a8469f.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-5ddf3ebd487fc468.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-ccc7a59de56dc2df.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd_unicode-4e8d8bf9e4eb1cb3.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-a9c6d56995f719c6.rlib" "/home/alex/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-2280e0ce1fded772.rlib" "-Wl,-Bdynamic" "-l" "dl" "-l" "rt" "-l" "pthread" "-l" "pthread" "-l" "gcc_s" "-l" "c" "-l" "m" "-l" "rt" "-l" "pthread" "-l" "util" "-l" "util"
  = note: foo.foo2-317d481089b8c8fe83113de504472633.rs.rcgu.o:foo2-317d481089b8c8fe83113de504472633.rs:function foo::main::hdc141899c6b702c7: error: undefined reference to 'FOO'
          collect2: error: ld returned 1 exit status
          

error: aborting due to previous error

[alexcrichton]

The LLVMRustPrepareThinLTOResolveWeak pass here is internalizing FOO. I never did think that pass was correct... My guess is that there's probably some trickery which I didn't copy from LLVM correctly or needs to be tweaked.

[japaric]

Although for you removing the weak linkage didn't fix it?

If I make DEFAULT_HANDLER part of the public API (pub and visible/reachable) and remove weak linkage then it works / links. (It seems that weak linkage makes the symbol global even if it's not part of the public API.) I can live with the DEFAULT_HANDLER change (I can doc(hidden) the function) but I can't drop the weak linkage because that's a feature.

[alexcrichton]

@japaric yeah makes total sense to me, the regression in handling of weak linkage is definitely a bug to fix!

I think the need for "make it all the way public" is because #[linkage] the attribute doesn't play that well into which symbols are exported and whatnot. I think this worked with LTO in one module because it happened to match up but with LTO in multiple modules the matchup isn't happening and the symbol is lost.

I didn't spend too much time trying to rationalize #[linkage] (in general, not specifically weak) with ThinLTO as it's an unstable feature, but I'm always game for tweaks!