Linking failures #88
Assignees
Comments
|
So I initially thought this was because opte is not a workspace, and when pulling it in via git it was treating it as one (and that was causing issues), but even after converting opte to a partial workspace (where ddi, opte-core, and opteadm are packages in the workspace), I still hit this linker error (versus when pulling opteadm in directly via a I used Versus the git case: The only difference I can see between the two is that in the good (path) case it has incremental objects for ddi: But there is no such thing for the bad (git) case. This all feels like some inside baseball between cargo/rustc and how it builds up these objects and invokes the linker that is beyond my understanding on how to debug. The real solution to this problem, once again, is that an API layer needs to be defined that insulates a client from all this crap. There is absolutely no reason for This tells |
22 tasks
|
Yep, I can confirm that this allows me to build the sled agent locally, using |
|
So this issue is rearing its ugly head again though in a slightly different context (seems to have nothing to do with how opte is being imported into another project). Like, I know what ld is telling me, but I just don't understand why this has only sort of become a thing until now, and seems to be dependent on mysterious things (which means it's probably a subtle environment thing or a race). |
|
This issue should probably be retitled or a new issue created in its place as the underlying problem seems to be a bit more general than originally described. I'm wrapped up with jury duty right now so don't quite have the time to organize my thoughts like I normally would, but there's two things thoughts I've had thus far.
comit commit |
|
Okay, I have replicated this issue locally by bumping my toolchain on Toolchain I've been building on for last N weeks: Toolchain I updated to: Output of running So at least I have a reliable way to reproduce now and examine locally. My focus (when I get some free time) will be looking at the gcc/ld invocation differences between the two and if there's any difference between the codegen units. It would probably also make sense to set the |
|
We've not hit this in a while. We'll obviously bring it back if anyone sees it again. |
|
Upon bumping the rust toolchain I have hit this issue once again. I also reproduced this locally on my Helios box. An additional data point I noticed:
The rust version being used: |
rzezeski
changed the title
|
By the powers of
/// Add a synthetic object file that contains reference to all symbols that we want to expose to
/// the linker.
///
/// Background: we implement rlibs as static library (archives). Linkers treat archives
/// differently from object files: all object files participate in linking, while archives will
/// only participate in linking if they can satisfy at least one undefined reference (version
/// scripts doesn't count). This causes `#[no_mangle]` or `#[used]` items to be ignored by the
/// linker, and since they never participate in the linking, using `KEEP` in the linker scripts
/// can't keep them either. This causes #47384.
///
/// To keep them around, we could use `--whole-archive` and equivalents to force rlib to
/// participate in linking like object files, but this proves to be expensive (#93791). Therefore
/// we instead just introduce an undefined reference to them. This could be done by `-u` command
/// line option to the linker or `EXTERN(...)` in linker scripts, however they does not only
/// introduce an undefined reference, but also make them the GC roots, preventing `--gc-sections`
/// from removing them, and this is especially problematic for embedded programming where every
/// byte counts.
///
/// This method creates a synthetic object file, which contains undefined references to all symbols
/// that are necessary for the linking. They are only present in symbol table but not actually
/// used in any sections, so the linker will therefore pick relevant rlibs for linking, but
/// unused `#[no_mangle]` or `#[used]` can still be discard by GC sections.
fn add_linked_symbol_object(
cmd: &mut dyn Linker,
sess: &Session,
tmpdir: &Path,
symbols: &[(String, SymbolExportKind)],
) {
if symbols.is_empty() {
return;
}
let Some(mut file) = super::metadata::create_object_file(sess) else {
return;
};
// NOTE(nbdd0121): MSVC will hang if the input object file contains no sections,
// so add an empty section.
if file.format() == object::BinaryFormat::Coff {
file.add_section(Vec::new(), ".text".into(), object::SectionKind::Text);
// We handle the name decoration of COFF targets in `symbol_export.rs`, so disable the
// default mangler in `object` crate.
file.set_mangling(object::write::Mangling::None);
// Add feature flags to the object file. On MSVC this is optional but LLD will complain if
// not present.
let mut feature = 0;
if file.architecture() == object::Architecture::I386 {
// Indicate that all SEH handlers are registered in .sxdata section.
// We don't have generate any code, so we don't need .sxdata section but LLD still
// expects us to set this bit (see #96498).
// Reference: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format
feature |= 1;
}
file.add_symbol(object::write::Symbol {
name: "@feat.00".into(),
value: feature,
size: 0,
kind: object::SymbolKind::Data,
scope: object::SymbolScope::Compilation,
weak: false,
section: object::write::SymbolSection::Absolute,
flags: object::SymbolFlags::None,
});
}
for (sym, kind) in symbols.iter() {
file.add_symbol(object::write::Symbol {
name: sym.clone().into(),
value: 0,
size: 0,
kind: match kind {
SymbolExportKind::Text => object::SymbolKind::Text,
SymbolExportKind::Data => object::SymbolKind::Data,
SymbolExportKind::Tls => object::SymbolKind::Tls,
},
scope: object::SymbolScope::Unknown,
weak: false,
section: object::write::SymbolSection::Undefined,
flags: object::SymbolFlags::None,
});
}
let path = tmpdir.join("symbols.o");
let result = std::fs::write(&path, file.write().unwrap());
if let Err(e) = result {
sess.fatal(&format!("failed to write {}: {}", path.display(), e));
}
cmd.add_object(&path);
}
In short, this has always been a problem with the way I structured things in The solution will involve restructuring how symbols are exposed from |
rzezeski
added a commit
that referenced
this issue
Aug 30, 2022
The crux of the problem was that illumos-ddi-dki was exposing all symbols, all the time. This meant that even when compiled for the purposes of testing it was exposing kernel-only symbols, resulting in ld failing to find them. This was always a problem, but it wasn't exposed until rustc made it's "synthetic object" change: commit 773f533eae25129cea7241b74e54f26ce5eebb62 Author: Gary Guo <[email protected]> Date: Sat Apr 2 22:54:51 2022 +0100 Synthesis object file for `#[used]` and exported symbols This commit contains the following changes: - Update rust toolchain to nightly 2022-07-14. I tried using newer nightly but it resulted in compiler asserts. - Move all illumos-ddi-dki bits into ilumos-sys-hdrs. If you look at the illumos `uts/common/sys` headers, you'll see they contain the DDI types and API; so moving everything under one crate made more sense (and in fact my reason for separating them in the past was a naive attempt at trying to tease apart the types/symbols visible in userspace vs. those only in the kernel -- but clearly I failed at that). - Add a `kernel` feature to illumos-sys-hdrs. This feature enables the exposure of kernel-only types and symbols, and maps to the `_KERNEL` define found in illumos. With that in place it's a matter of wiring up the `kernel` feature in opte as well as xde, so that only the xde crate enables this feature, whereas running `cargo test` in opte should not enable it. - Update the various imports to map to `illumos_sys_hdrs` (that's the reason for the big churn in `ip.rs`, a purely mechanical change).
rzezeski
added a commit
that referenced
this issue
Aug 30, 2022
The crux of the problem was that illumos-ddi-dki was exposing all symbols, all the time. This meant that even when compiled for the purposes of testing it was exposing kernel-only symbols, resulting in ld failing to find them. This was always a problem, but it wasn't exposed until rustc made it's "synthetic object" change: commit 773f533eae25129cea7241b74e54f26ce5eebb62 Author: Gary Guo <[email protected]> Date: Sat Apr 2 22:54:51 2022 +0100 Synthesis object file for `#[used]` and exported symbols This commit contains the following changes: - Update rust toolchain to nightly 2022-07-14. I tried using newer nightly but it resulted in compiler asserts. - Move all illumos-ddi-dki bits into ilumos-sys-hdrs. If you look at the illumos `uts/common/sys` headers, you'll see they contain the DDI types and API; so moving everything under one crate made more sense (and in fact my reason for separating them in the past was a naive attempt at trying to tease apart the types/symbols visible in userspace vs. those only in the kernel -- but clearly I failed at that). - Add a `kernel` feature to illumos-sys-hdrs. This feature enables the exposure of kernel-only types and symbols, and maps to the `_KERNEL` define found in illumos. With that in place it's a matter of wiring up the `kernel` feature in opte as well as xde, so that only the xde crate enables this feature, whereas running `cargo test` in opte should not enable it. - Update the various imports to map to `illumos_sys_hdrs` (that's the reason for the big churn in `ip.rs`, a purely mechanical change).
rzezeski
added a commit
that referenced
this issue
Aug 30, 2022
The crux of the problem was that illumos-ddi-dki was exposing all symbols, all the time. This meant that even when compiled for the purposes of testing it was exposing kernel-only symbols, resulting in ld failing to find them. This was always a problem, but it wasn't exposed until rustc made it's "synthetic object" change: commit 773f533eae25129cea7241b74e54f26ce5eebb62 Author: Gary Guo <[email protected]> Date: Sat Apr 2 22:54:51 2022 +0100 Synthesis object file for `#[used]` and exported symbols This commit contains the following changes: - Update rust toolchain to nightly 2022-07-14. I tried using newer nightly but it resulted in compiler asserts. - Move all illumos-ddi-dki bits into ilumos-sys-hdrs. If you look at the illumos `uts/common/sys` headers, you'll see they contain the DDI types and API; so moving everything under one crate made more sense (and in fact my reason for separating them in the past was a naive attempt at trying to tease apart the types/symbols visible in userspace vs. those only in the kernel -- but clearly I failed at that). - Add a `kernel` feature to illumos-sys-hdrs. This feature enables the exposure of kernel-only types and symbols, and maps to the `_KERNEL` define found in illumos. With that in place it's a matter of wiring up the `kernel` feature in opte as well as xde, so that only the xde crate enables this feature, whereas running `cargo test` in opte should not enable it. - Update the various imports to map to `illumos_sys_hdrs` (that's the reason for the big churn in `ip.rs`, a purely mechanical change).
|
Addressed in 365cfd6. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When using
opteadmas a Git dependency in a project's Cargo.toml, we get linking errors. Things seem to work fine if the dependency is specified as a local, path-dependency. To reproduce, create a new binary:With a
Cargo.tomlthat looks like this:I have the following minimal binary, in `src/main.rs:
Running the package's binary fails at link:
Cloning this repo and specifying it as a path dependency builds just fine. Changing the one dependency in
Cargo.tomlto:we can build and run the binary successfully:
The text was updated successfully, but these errors were encountered: