linking of libtest failed #64872
Comments
jonas-schievink
added
A-libtest
|
When replacing getopts/src/lib.rs with |
|
It fails to link on macOS. It works fine on Linux however. I haven't tested it on Windows. |
|
@rustbot modify labels: +O-macos |
|
triage: P-high for now (until I can prove to myself it deserves lower priority). Removing I-nominated label. Assigning to self. cc @alexcrichton and @michaelwoerister |
|
Would it be possible to get a reduced test case that involves just poking around rustc to work with? |
|
@alexcrichton #64872 (comment) has been my reduced test case for now. I tried to reproduce it with a standalone example, but I failed. I may be able to reduce it more in a few days, when I get to use a macOS again. |
|
Ah true yeah, but I'm also curious if this can be reproduced without using the cranelift backend, because it seems like this could be a bug in that backend vs in the common shared code? |
|
It can be reproduced without the cranelift backend. I thought that it was a bug in the cranelift backend at first, but when I couldn't find a difference between cg_llvm and cg_clif, I tried to use cg_llvm, which failed too. Running |
|
Ok I want to poke around, but it doesn't currently reproduce because patches fail. Would it be possible to fix that and/or try to reduce further to not require a bunch of rustup stuff and patching sysroot files directly? |
|
I am not sure if those patches are actually required to reproduce the problem. I am currently working on fixing the patches though. |
|
Pushed a new set of patches. |
|
The patches are not necessary. I have uploaded a repro with just the necessary Cargo.toml and a script to fetch the sysroot src at https://github.com/bjorn3/rust_issue_64872_repro. |
|
Sorry I do not have time to continue to investigate this and help minifying, but wanted to be sure to say so. |
|
No problem. I will try to minify it myself once I have time to do so. |
|
nominating for discussion: I want T-compiler to consider whether, in the short term, we should revert PR #64324 as a way to address this bug. (It seems to me like the risks of linker errors due to mixed optimization levels are less than the risks presented here in this ticket, and that is an argument for reverting #64324 in the short term.) In the meantime I will continue to investigate the bug independently, but the higher level bit of whether to revert seems worth discussion. |
|
discussed in yesterdays T-compiler meeting. There was some support for hypothetically reverting PR #64324 in the short term. But there was also concern that no one would do the follow-up work afterward, and also that it would be a shame to lose the various improvements we got from PR #64324 Overall we decided to leave this nominated to be revisited next week, and in the meantime I will try to minimize the bug and maybe identify what its root cause is. |
|
I spent some time today working to minimize the input that stills sees a linker failure. Here's what I've gotten down to so far: // libtest/src/lib.rs
#![crate_name = "test"]
extern crate getopts;// getopts/src/lib.rs
#[derive(Debug)]
enum Name { _Short(u32) }// libstd/lib.rs
#![no_std]
#![feature(needs_panic_runtime)]
#![needs_panic_runtime]
#![feature(lang_items)]
#![feature(rustc_attrs)]
#![feature(alloc_error_handler)]
#![feature(allocator_internals)]
#![default_lib_allocator]
extern crate alloc as alloc_crate;
use core::panic::PanicInfo;
#[panic_handler]
pub fn rust_begin_panic(_info: &PanicInfo<'_>) -> ! {
loop { }
}
pub mod prelude {
pub mod v1 {
pub use core::prelude::v1::{Debug};
}
}
use alloc_crate::alloc::Layout;
#[alloc_error_handler]
pub fn rust_oom(_layout: Layout) -> ! { loop { } }
#[rustc_std_internal_symbol]
pub unsafe extern fn __rdl_alloc(_size: usize, _align: usize) -> *mut u8 { loop { } }
#[rustc_std_internal_symbol]
pub unsafe extern fn __rdl_dealloc(_ptr: *mut u8,
_size: usize,
_align: usize) { loop { } }
#[rustc_std_internal_symbol]
pub unsafe extern fn __rdl_realloc(_ptr: *mut u8,
_old_size: usize,
_align: usize,
_new_size: usize) -> *mut u8 { loop { } }
#[rustc_std_internal_symbol]
pub unsafe extern fn __rdl_alloc_zeroed(_size: usize, _align: usize) -> *mut u8 { loop { } }(And I confirmed that this is still tickling the same bug, because when I change the |
|
More data: I've reduced all of
It seems like the problem here manifests itself when both if you eliminate that commonality, then the linking proceeds successfully. This makes me hypothesize that the bug here might be something where the compilation of the downstream crate (
|
|
@pnkfelix Can you take a look at the symbols defined in the upstream crate? Is the missing function present there (but maybe as a local symbol). |
This sounds like |
|
@michaelwoerister here is some For this reduction, the symbol we are looking for is |
|
@michaelwoerister wrote:
I don't think this explains it. I tried removing |
|
There was further discussion with @michaelwoerister and @alexcrichton on zulip. I think I'll have a patch up today that should address the problem here, hopefully without causing any problems anywhere else. |
|
Removing nomination label since it seems like this is in hand. |
|
By the way, @alexcrichton and I managed to construct a very nice minimized version of the bug here: https://gist.github.com/alexcrichton/b3ba3becdf2009973270d2aef3453670 |
they will be statically linked. In the particular case of rust-lang#64872, we get burned by an attempt to link to a monomorphization in libcore.rlib that ends up not being exported by libstd.dylib. (For that scenario, the attempt to lookup the linkage returns `None`, which led us terribly astray.) (That scenario is encoded in a test in a follow-on commit.) Also added some `log::info` output for the `None` case, because I want easy access in all of our builds to inspect what goes on in this logic. In response to review feedback, I had tried to revise the fix to be more nuanced in handling of `None` (i.e., the case which I've previously asserted to be "unknown linkage"). Alex's take on matters is that we should use the output crate type to determine what format the dependency here will have. However, further testing showed that approach to be flawed. So I added debugflag: `-Z share-generics-for-unknown-linkage`, to make it easy to recover the earlier behavior (which I nonetheless assert to be buggy in general), and use that flag to keep one of our codegen tests working.
pnkfelix
added
the
regression-from-stable-to-beta
|
(based on my further testing, this is readily reproducible without using any unstable features, and thus is a stable-to-beta regression. See discussion in #65781 ) |
Probably regressed in #64324 (Fix mixing crates with different
share_generics)The text was updated successfully, but these errors were encountered: