Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of +whole-archive native link modifier. #88161

Merged
merged 3 commits into from
Sep 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 49 additions & 10 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use regex::Regex;
use tempfile::Builder as TempFileBuilder;

use std::ffi::OsString;
use std::lazy::OnceCell;
use std::path::{Path, PathBuf};
use std::process::{ExitStatus, Output, Stdio};
use std::{ascii, char, env, fmt, fs, io, mem, str};
Expand Down Expand Up @@ -254,6 +255,19 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>(
// metadata of the rlib we're generating somehow.
for lib in codegen_results.crate_info.used_libraries.iter() {
match lib.kind {
NativeLibKind::Static { bundle: None | Some(true), whole_archive: Some(true) }
if flavor == RlibFlavor::Normal =>
{
// Don't allow mixing +bundle with +whole_archive since an rlib may contain
// multiple native libs, some of which are +whole-archive and some of which are
// -whole-archive and it isn't clear how we can currently handle such a
// situation correctly.
// See https://github.com/rust-lang/rust/issues/88085#issuecomment-901050897
sess.err(
"the linking modifiers `+bundle` and `+whole-archive` are not compatible \
with each other when generating rlibs",
);
}
NativeLibKind::Static { bundle: None | Some(true), .. } => {}
NativeLibKind::Static { bundle: Some(false), .. }
| NativeLibKind::Dylib { .. }
Expand Down Expand Up @@ -1222,6 +1236,7 @@ pub fn archive_search_paths(sess: &Session) -> Vec<PathBuf> {
sess.target_filesearch(PathKind::Native).search_path_dirs()
}

#[derive(PartialEq)]
enum RlibFlavor {
Normal,
StaticlibBase,
Expand Down Expand Up @@ -2001,7 +2016,7 @@ fn add_local_native_libraries(
let relevant_libs =
codegen_results.crate_info.used_libraries.iter().filter(|l| relevant_lib(sess, l));

let search_path = archive_search_paths(sess);
let search_path = OnceCell::new();
let mut last = (NativeLibKind::Unspecified, None);
for lib in relevant_libs {
let name = match lib.name {
Expand All @@ -2023,7 +2038,11 @@ fn add_local_native_libraries(
}
NativeLibKind::Static { bundle: None | Some(true), .. }
| NativeLibKind::Static { whole_archive: Some(true), .. } => {
cmd.link_whole_staticlib(name, verbatim, &search_path);
cmd.link_whole_staticlib(
name,
verbatim,
&search_path.get_or_init(|| archive_search_paths(sess)),
);
}
NativeLibKind::Static { .. } => cmd.link_staticlib(name, verbatim),
NativeLibKind::RawDylib => {
Expand Down Expand Up @@ -2116,6 +2135,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
}

let mut compiler_builtins = None;
let search_path = OnceCell::new();

for &cnum in deps.iter() {
if group_start == Some(cnum) {
Expand Down Expand Up @@ -2149,16 +2169,35 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
// external build system already has the native dependencies defined, and it
// will provide them to the linker itself.
if sess.opts.debugging_opts.link_native_libraries {
// Skip if this library is the same as the last.
let mut last = None;
for lib in &codegen_results.crate_info.native_libraries[&cnum] {
if lib.name.is_some()
&& relevant_lib(sess, lib)
&& matches!(lib.kind, NativeLibKind::Static { bundle: Some(false), .. })
&& last != lib.name
{
cmd.link_staticlib(lib.name.unwrap(), lib.verbatim.unwrap_or(false));
last = lib.name;
if !relevant_lib(sess, lib) {
// Skip libraries if they are disabled by `#[link(cfg=...)]`
continue;
}

// Skip if this library is the same as the last.
if last == lib.name {
continue;
}

if let Some(static_lib_name) = lib.name {
if let NativeLibKind::Static { bundle: Some(false), whole_archive } =
lib.kind
{
let verbatim = lib.verbatim.unwrap_or(false);
if whole_archive == Some(true) {
cmd.link_whole_staticlib(
static_lib_name,
verbatim,
search_path.get_or_init(|| archive_search_paths(sess)),
);
} else {
cmd.link_staticlib(static_lib_name, verbatim);
}

last = lib.name;
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![feature(box_patterns)]
#![feature(try_blocks)]
#![feature(in_band_lifetimes)]
#![feature(once_cell)]
#![feature(nll)]
#![feature(associated_type_bounds)]
#![recursion_limit = "256"]
Expand Down
39 changes: 39 additions & 0 deletions src/test/run-make/native-link-modifier-whole-archive/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# ignore-cross-compile -- compiling C++ code does not work well when cross-compiling

# This test case makes sure that native libraries are linked with --whole-archive semantics
# when the `-bundle,+whole-archive` modifiers are applied to them.
#
# The test works by checking that the resulting executables produce the expected output,
# part of which is emitted by otherwise unreferenced C code. If +whole-archive didn't work
# that code would never make it into the final executable and we'd thus be missing some
# of the output.

-include ../../run-make-fulldeps/tools.mk

all: $(TMPDIR)/$(call BIN,directly_linked) $(TMPDIR)/$(call BIN,indirectly_linked) $(TMPDIR)/$(call BIN,indirectly_linked_via_attr)
$(call RUN,directly_linked) | $(CGREP) 'static-initializer.directly_linked.'
$(call RUN,indirectly_linked) | $(CGREP) 'static-initializer.indirectly_linked.'
$(call RUN,indirectly_linked_via_attr) | $(CGREP) 'static-initializer.native_lib_in_src.'

# Native lib linked directly into executable
$(TMPDIR)/$(call BIN,directly_linked): $(call NATIVE_STATICLIB,c_static_lib_with_constructor)
$(RUSTC) directly_linked.rs -Z unstable-options -l static:+whole-archive=c_static_lib_with_constructor

# Native lib linked into RLIB via `-l static:-bundle,+whole-archive`, RLIB linked into executable
$(TMPDIR)/$(call BIN,indirectly_linked): $(TMPDIR)/librlib_with_cmdline_native_lib.rlib
$(RUSTC) indirectly_linked.rs

# Native lib linked into RLIB via #[link] attribute, RLIB linked into executable
$(TMPDIR)/$(call BIN,indirectly_linked_via_attr): $(TMPDIR)/libnative_lib_in_src.rlib
$(RUSTC) indirectly_linked_via_attr.rs

# Native lib linked into rlib with via commandline
$(TMPDIR)/librlib_with_cmdline_native_lib.rlib: $(call NATIVE_STATICLIB,c_static_lib_with_constructor)
$(RUSTC) rlib_with_cmdline_native_lib.rs -Z unstable-options --crate-type=rlib -l static:-bundle,+whole-archive=c_static_lib_with_constructor

# Native lib linked into rlib via `#[link()]` attribute on extern block.
$(TMPDIR)/libnative_lib_in_src.rlib: $(call NATIVE_STATICLIB,c_static_lib_with_constructor)
$(RUSTC) native_lib_in_src.rs --crate-type=rlib

$(TMPDIR)/libc_static_lib_with_constructor.o: c_static_lib_with_constructor.cpp
$(call COMPILE_OBJ_CXX,$@,$<)
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#include <cstdio>

// Since this is a global variable, its constructor will be called before
// main() is executed. But only if the object file containing it actually
// gets linked into the executable.
struct Foo {
Foo() {
printf("static-initializer.");
fflush(stdout);
}
} FOO;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
use std::io::Write;

fn main() {
print!("directly_linked.");
std::io::stdout().flush().unwrap();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extern crate rlib_with_cmdline_native_lib;

fn main() {
rlib_with_cmdline_native_lib::hello();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
extern crate native_lib_in_src;

fn main() {
native_lib_in_src::hello();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(native_link_modifiers_bundle)]
#![feature(native_link_modifiers_whole_archive)]
#![feature(native_link_modifiers)]

use std::io::Write;

#[link(name = "c_static_lib_with_constructor",
kind = "static",
modifiers = "-bundle,+whole-archive")]
extern {}

pub fn hello() {
print!("native_lib_in_src.");
std::io::stdout().flush().unwrap();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
use std::io::Write;

pub fn hello() {
print!("indirectly_linked.");
std::io::stdout().flush().unwrap();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// compile-flags: -Zunstable-options --crate-type rlib
// build-fail
// error-pattern: the linking modifiers `+bundle` and `+whole-archive` are not compatible with each other when generating rlibs

#![feature(native_link_modifiers)]
#![feature(native_link_modifiers_bundle)]
#![feature(native_link_modifiers_whole_archive)]

#[link(name = "mylib", kind = "static", modifiers = "+bundle,+whole-archive")]
extern "C" { }

fn main() { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error: the linking modifiers `+bundle` and `+whole-archive` are not compatible with each other when generating rlibs

error: could not find native static library `mylib`, perhaps an -L flag is missing?

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Mixing +bundle and +whole-archive is not allowed

// compile-flags: -l static:+bundle,+whole-archive=mylib -Zunstable-options --crate-type rlib
// build-fail
// error-pattern: the linking modifiers `+bundle` and `+whole-archive` are not compatible with each other when generating rlibs

fn main() { }
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error: the linking modifiers `+bundle` and `+whole-archive` are not compatible with each other when generating rlibs

error: could not find native static library `mylib`, perhaps an -L flag is missing?

error: aborting due to 2 previous errors