Whole archive modifier for native libraries is not propagated to final output.

[gcoakes]

With the additional of RFC 2951 (tracking: #81490, implementation: #83507), linking modifiers such as +whole-archive are now possible to use for native libraries. However, +whole-archive is not propagated to the final output of the build. I've minimally reproduced this issue as best I could here. It can probably be minimized more with raw calls to rustc, but I'm not familiar enough to do so right now.

Essentially, I have a static library which provides constructor and destructor functions which must be included in the final output.

I tried this code:

foo.c

#include <stdio.h>
void __attribute__((constructor)) construct();
void __attribute__((destructor)) destruct();
void construct()
{
    printf("Constructor.\n");
}
void destruct()
{
    printf("Destructor.\n");
}

build.rs

fn main() {
    cc::Build::new()
        .file("src/foo.c")
        .cargo_metadata(false)
        .static_flag(true)
        .compile("foo");
    println!(
        "cargo:rustc-link-search=native={}",
        std::env::var("OUT_DIR").unwrap()
    );
    println!("cargo:rustc-link-lib=static:+whole-archive=foo");
}

lib.rs

pub fn it_linked() {}

main.rs

fn main() {
    indirect::it_linked();
    println!("Hello, indirect!");
}

I expected to see this happen:

$ RUSTFLAGS="-Z unstable-options" cargo run --bin indirect
   Compiling cc v1.0.69
   Compiling direct v0.1.0 (/home/gcoakes/Documents/Source/minimal-whole-archive/indirect)
    Finished dev [unoptimized + debuginfo] target(s) in 1.73s
     Running `target/debug/indirect`
Constructor.
Hello, indirect!
Destructor.
$ nm target/debug/indirect | grep 'construct\|destruct'
0000000000007ab5 T construct
0000000000007acc T destruct

Instead, this happened:

$ RUSTFLAGS="-Z unstable-options" cargo run --bin indirect
   Compiling indirect v0.1.0 (/home/gcoakes/Documents/Source/minimal-whole-archive/indirect)
    Finished dev [unoptimized + debuginfo] target(s) in 0.68s
     Running `target/debug/indirect`
Hello, indirect!
$ nm target/debug/indirect | grep 'construct\|destruct'

Resolution

I've done a bit of code inspection to determine where the linking issue actually is. I think it comes down to the link_upstream closure in rustc_codegen_ssa::src::back::link::add_upstream_rust_crates::add_static_crate. It should be adjusted to inspect the crates native libraries and determine if any of them have +whole-archive. If one or more do, then it should run link_whole_rlib on the crate's rlib instead of link_rlib.

Meta

rustc --version --verbose:

rustc 1.56.0-nightly (8007b506a 2021-08-14)
binary: rustc
commit-hash: 8007b506ac5da629f223b755f5a5391edd5f6d01
commit-date: 2021-08-14
host: x86_64-unknown-linux-gnu
release: 1.56.0-nightly
LLVM version: 12.0.1

[petrochenkov]

whole-archive is currently correctly propagated for static:-bundle (aka static-nobundle) libraries, libraries bundled into rlibs however lose their identity entirely and can only be linked as that rlib.

If we want to support that use case, rlibs will need to stop being "just static libraries" containing object files, they'll need to maintain some kind of internal structure to be able to be broken into parts with those parts being passed to linker separately.

Short term we probably need to make static:+whole-archive an error (at least when building a rlib) to avoid confusing people, and recommend migrating to static:-bundle,+whole-archive.

[gcoakes]

@petrochenkov, I just tried adding -bundle to the build.rs from the description, and I'm still seeing the same behavior. Am I missing something? Also, I wasn't expecting the rlib to be passed separately from the native library. I was expecting the whole rlib would be +whole-archive since one of it's dependencies was.

[michaelwoerister]

I can reproduce the issue as described:

• If compiled with cargo:rustc-link-lib=static:+bundle,+whole-archive=foo then libfoo_sys.rlib will contain foo.o but it is linked without --whole-archive (and thus foo.o won't be linked in).
• If compiled with cargo:rustc-link-lib=static:-bundle,+whole-archive=foo (i.e. no bundling) then libfoo_sys.rlib will not contain foo.o (as expected) but libfoo.a also is not linked with --whole-archive.

That it does not work for the no-bundle case seems to be a bug in add_upstream_native_libraries where the whole_archive setting is not taken into account:

rust/compiler/rustc_codegen_ssa/src/back/link.rs

Lines 2355 to 2363 in aa8f27b

	NativeLibKind::Static { bundle: Some(false), .. } => {
	// Link "static-nobundle" native libs only if the crate they originate from
	// is being linked statically to the current crate. If it's linked dynamically
	// or is an rlib already included via some other dylib crate, the symbols from
	// native libs will have already been included in that dylib.
	if data[cnum.as_usize() - 1] == Linkage::Static {
	cmd.link_staticlib(name, verbatim)
	}
	}

Changing this to the following makes the no-bundle case work for me locally:

NativeLibKind::Static { bundle: Some(false), whole_archive } => {
    // Link "static-nobundle" native libs only if the crate they originate from
    // is being linked statically to the current crate.  If it's linked dynamically
    // or is an rlib already included via some other dylib crate, the symbols from
    // native libs will have already been included in that dylib.
    if data[cnum.as_usize() - 1] == Linkage::Static {
        if whole_archive == Some(true) {
            cmd.link_whole_staticlib(name, verbatim, &search_path);
        } else {
            cmd.link_staticlib(name, verbatim)
        }
    }
}

[michaelwoerister]

@petrochenkov, I plan to make a PR with the above fix and some regression tests. However, the fix only covers the -bundle case. What do we want to do for the +bundle case? I see a few options:

1. Disallow combining +bundle with +whole-archive
2. Try to emulate the before of +bundle,+whole-archive somehow (e.g. by extracting the object files of the static lib and linking them directly).
3. Leave things as they are but maybe emit a warning +whole-archive can't be respected.
4. Follow @gcoakes's suggestion and make RLIBs containing any bundled, whole-archive dependencies also be linked with --whole-archive.

Are there any better solutions?

[petrochenkov]

Follow @gcoakes's suggestion and make RLIBs containing any bundled, whole-archive dependencies also be linked with --whole-archive.

I don't think this is a right thing to do - a rlib can include its own code and multiple native libraries with different values of the whole-archive flag, why should a single +whole-archive "win" over the rest of the rlib?

Disallow combining +bundle with +whole-archive

Leave things as they are but maybe emit a warning +whole-archive can't be respected.

These looks like good near term solutions.

Try to emulate the before of +bundle,+whole-archive somehow (e.g. by extracting the object files of the static lib and linking them directly).

I'd be interested in seeing a design doc describing how to build a rlib in a way allowing to "disassemble" it into parts and link those parts separately.
(It may e.g. turn out that it's viable to "bundle" native libraries by just keeping them as separate files somewhere near the main rlib file.)

It would be useful not only for whole-archive, but also for link time cfg, for example. Seems like an MCP material.

[michaelwoerister]

Disallow combining +bundle with +whole-archive

How would this look from a backwards-compatibility point of view? It should be fine because +whole-archive isn't stable anyway, right?

[petrochenkov]

It should be fine because +whole-archive isn't stable anyway, right?

Yes.

[gcoakes]

@michaelwoerister, the -bundle,+whole-archive case is considered a bug, right? Just want to clarify because I think that's what I should have been doing from the beginning, but I just didn't know about it before it was mentioned here.

[michaelwoerister]

@gcoakes, yes that case is clearly just a bug in the implementation. I'm working on a fix.