[regression] parallel LTO breaks linking because it internalizes weak symbols #46543
Comments
alexcrichton
added
the
regression-from-stable-to-nightly
|
@japaric still digesting, but |
|
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 |
|
@alexcrichton $ arm-none-eabi-nm -C "demo-cc40ab28f2256f87.cortex_m_rt0-a8ed1a542842e08c4e593519be42c026.rs.rcgu.o"
00000000 W BUS_FAULT
00000000 t DEFAULT_HANDLER
U _ebssThe 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 |
It certainly matters, at least for static libraries. |
|
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? |
|
@alexcrichton // 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" {} |
|
Removing |
|
@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? |
|
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? |
|
Ok the first test case reduced a bit is: |
|
The |
If I make DEFAULT_HANDLER part of the public API ( |
japaric
changed the title
|
@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 I didn't spend too much time trying to rationalize |
|
I believe #46549 fixes the weak linkage issue |
In rust-lang#46382 the logic around linkage preservation with ThinLTO ws tweaked but the loop that registered all otherwise exported GUID values as "don't internalize me please" was erroneously too conservative and only asking "external" linkage items to not be internalized. Instead we actually want the inversion of that condition, everything *without* "local" linkage to be internalized. This commit updates the condition there, adds a test, and... Closes rust-lang#46543
rustc: Further tweak linkage in ThinLTO In #46382 the logic around linkage preservation with ThinLTO ws tweaked but the loop that registered all otherwise exported GUID values as "don't internalize me please" was erroneously too conservative and only asking "external" linkage items to not be internalized. Instead we actually want the inversion of that condition, everything *without* "local" linkage to be internalized. This commit updates the condition there, adds a test, and... Closes #46543
STR
With LTO
Linker arguments:
Without LTO
Linker arguments:
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.
stm32f103xxhas an undefined symbol,DEFAULT_HANDLER, which is provided by its dependencycortex-m-rt. The LTO build fails to link becausecortex-m-rtappears beforestm32f103xxin 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 = 1and I still got parallel LTO.The text was updated successfully, but these errors were encountered: