Skip to content

Commit

Permalink
Stabilize native library modifier syntax and the whole-archive modi…
Browse files Browse the repository at this point in the history
…fier specifically
  • Loading branch information
petrochenkov committed Mar 30, 2022
1 parent bb5c437 commit 1004783
Show file tree
Hide file tree
Showing 33 changed files with 270 additions and 165 deletions.
8 changes: 0 additions & 8 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
if attr.has_name(sym::link) {
for nested_meta in attr.meta_item_list().unwrap_or_default() {
if nested_meta.has_name(sym::modifiers) {
gate_feature_post!(
self,
native_link_modifiers,
nested_meta.span(),
"native link modifiers are experimental"
);

if let Some(modifiers) = nested_meta.value_str() {
for modifier in modifiers.as_str().split(',') {
if let Some(modifier) = modifier.strip_prefix(&['+', '-']) {
Expand All @@ -412,7 +405,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
gate_modifier!(
"bundle" => native_link_modifiers_bundle
"verbatim" => native_link_modifiers_verbatim
"whole-archive" => native_link_modifiers_whole_archive
"as-needed" => native_link_modifiers_as_needed
);
}
Expand Down
103 changes: 62 additions & 41 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1844,7 +1844,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>(
// This change is somewhat breaking in practice due to local static libraries being linked
// as whole-archive (#85144), so removing whole-archive may be a pre-requisite.
if sess.opts.debugging_opts.link_native_libraries {
add_local_native_libraries(cmd, sess, codegen_results);
add_local_native_libraries(cmd, sess, codegen_results, crate_type);
}

// Upstream rust libraries and their nobundle static libraries
Expand Down Expand Up @@ -2016,6 +2016,16 @@ fn add_order_independent_options(
add_rpath_args(cmd, sess, codegen_results, out_filename);
}

// A dylib may reexport symbols from the linked rlib or native static library.
// Even if some symbol is reexported it's still not necessarily counted as used and may be
// dropped, at least with `ld`-like ELF linkers. So we have to link some rlibs and static
// libraries as whole-archive to avoid losing reexported symbols.
// FIXME: Find a way to mark reexported symbols as used and avoid this use of whole-archive.
fn default_to_whole_archive(sess: &Session, crate_type: CrateType, cmd: &dyn Linker) -> bool {
crate_type == CrateType::Dylib
&& !(sess.target.limit_rdylib_exports && cmd.exported_symbol_means_used_symbol())
}

/// # Native library linking
///
/// User-supplied library search paths (-L on the command line). These are the same paths used to
Expand All @@ -2029,6 +2039,7 @@ fn add_local_native_libraries(
cmd: &mut dyn Linker,
sess: &Session,
codegen_results: &CodegenResults,
crate_type: CrateType,
) {
let filesearch = sess.target_filesearch(PathKind::All);
for search_path in filesearch.search_paths() {
Expand All @@ -2046,14 +2057,18 @@ fn add_local_native_libraries(
codegen_results.crate_info.used_libraries.iter().filter(|l| relevant_lib(sess, l));

let search_path = OnceCell::new();
let mut last = (NativeLibKind::Unspecified, None);
let mut last = (None, NativeLibKind::Unspecified, None);
for lib in relevant_libs {
let Some(name) = lib.name else {
continue;
};

// Skip if this library is the same as the last.
last = if (lib.kind, lib.name) == last { continue } else { (lib.kind, lib.name) };
last = if (lib.name, lib.kind, lib.verbatim) == last {
continue;
} else {
(lib.name, lib.kind, lib.verbatim)
};

let verbatim = lib.verbatim.unwrap_or(false);
match lib.kind {
Expand All @@ -2064,15 +2079,19 @@ fn add_local_native_libraries(
NativeLibKind::Framework { as_needed } => {
cmd.link_framework(name, as_needed.unwrap_or(true))
}
NativeLibKind::Static { bundle: None | Some(true), .. }
| NativeLibKind::Static { whole_archive: Some(true), .. } => {
cmd.link_whole_staticlib(
name,
verbatim,
&search_path.get_or_init(|| archive_search_paths(sess)),
);
NativeLibKind::Static { whole_archive, .. } => {
if whole_archive == Some(true)
|| (whole_archive == None && default_to_whole_archive(sess, crate_type, cmd))
{
cmd.link_whole_staticlib(
name,
verbatim,
&search_path.get_or_init(|| archive_search_paths(sess)),
);
} else {
cmd.link_staticlib(name, verbatim)
}
}
NativeLibKind::Static { .. } => cmd.link_staticlib(name, verbatim),
NativeLibKind::RawDylib => {
// FIXME(#58713): Proper handling for raw dylibs.
bug!("raw_dylib feature not yet implemented");
Expand Down Expand Up @@ -2197,34 +2216,37 @@ 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 {
let mut last = None;
let mut last = (None, NativeLibKind::Unspecified, None);
for lib in &codegen_results.crate_info.native_libraries[&cnum] {
let Some(name) = lib.name else {
continue;
};
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 {
last = if (lib.name, lib.kind, lib.verbatim) == last {
continue;
}

if let Some(static_lib_name) = lib.name {
if let NativeLibKind::Static { bundle: Some(false), whole_archive } =
lib.kind
} else {
(lib.name, lib.kind, lib.verbatim)
};

if let NativeLibKind::Static { bundle: Some(false), whole_archive } =
lib.kind
{
let verbatim = lib.verbatim.unwrap_or(false);
if whole_archive == Some(true)
|| (whole_archive == None
&& default_to_whole_archive(sess, crate_type, cmd))
{
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;
cmd.link_whole_staticlib(
name,
verbatim,
search_path.get_or_init(|| archive_search_paths(sess)),
);
} else {
cmd.link_staticlib(name, verbatim);
}
}
}
Expand Down Expand Up @@ -2282,15 +2304,10 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
let cratepath = &src.rlib.as_ref().unwrap().0;

let mut link_upstream = |path: &Path| {
// If we're creating a dylib, then we need to include the
// whole of each object in our archive into that artifact. This is
// because a `dylib` can be reused as an intermediate artifact.
//
// Note, though, that we don't want to include the whole of a
// compiler-builtins crate (e.g., compiler-rt) because it'll get
// repeatedly linked anyway.
// We don't want to include the whole compiler-builtins crate (e.g., compiler-rt)
// regardless of the default because it'll get repeatedly linked anyway.
let path = fix_windows_verbatim_for_gcc(path);
if crate_type == CrateType::Dylib
if default_to_whole_archive(sess, crate_type, cmd)
&& codegen_results.crate_info.compiler_builtins != Some(cnum)
{
cmd.link_whole_rlib(&path);
Expand Down Expand Up @@ -2401,7 +2418,7 @@ fn add_upstream_native_libraries(
sess: &Session,
codegen_results: &CodegenResults,
) {
let mut last = (NativeLibKind::Unspecified, None);
let mut last = (None, NativeLibKind::Unspecified, None);
for &cnum in &codegen_results.crate_info.used_crates {
for lib in codegen_results.crate_info.native_libraries[&cnum].iter() {
let Some(name) = lib.name else {
Expand All @@ -2412,7 +2429,11 @@ fn add_upstream_native_libraries(
}

// Skip if this library is the same as the last.
last = if (lib.kind, lib.name) == last { continue } else { (lib.kind, lib.name) };
last = if (lib.name, lib.kind, lib.verbatim) == last {
continue;
} else {
(lib.name, lib.kind, lib.verbatim)
};

let verbatim = lib.verbatim.unwrap_or(false);
match lib.kind {
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_codegen_ssa/src/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ pub trait Linker {
fn no_crt_objects(&mut self);
fn no_default_libraries(&mut self);
fn export_symbols(&mut self, tmpdir: &Path, crate_type: CrateType, symbols: &[String]);
fn exported_symbol_means_used_symbol(&self) -> bool {
true
}
fn subsystem(&mut self, subsystem: &str);
fn group_start(&mut self);
fn group_end(&mut self);
Expand Down Expand Up @@ -724,6 +727,10 @@ impl<'a> Linker for GccLinker<'a> {
}
}

fn exported_symbol_means_used_symbol(&self) -> bool {
self.sess.target.is_like_windows || self.sess.target.is_like_osx
}

fn subsystem(&mut self, subsystem: &str) {
self.linker_arg("--subsystem");
self.linker_arg(&subsystem);
Expand Down Expand Up @@ -1471,6 +1478,10 @@ impl<'a> Linker for L4Bender<'a> {
return;
}

fn exported_symbol_means_used_symbol(&self) -> bool {
false
}

fn subsystem(&mut self, subsystem: &str) {
self.cmd.arg(&format!("--subsystem {}", subsystem));
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/accepted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ declare_features! (
/// Allows patterns with concurrent by-move and by-ref bindings.
/// For example, you can write `Foo(a, ref b)` where `a` is by-move and `b` is by-ref.
(accepted, move_ref_pattern, "1.49.0", Some(68354), None),
/// Allows specifying modifiers in the link attribute: `#[link(modifiers = "...")]`
(accepted, native_link_modifiers, "1.61.0", Some(81490), None),
/// Allows specifying the whole-archive link modifier
(accepted, native_link_modifiers_whole_archive, "1.61.0", Some(81490), None),
/// Allows using `#![no_std]`.
(accepted, no_std, "1.6.0", None, None),
/// Allows defining identifiers beyond ASCII.
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,16 +446,12 @@ declare_features! (
(active, must_not_suspend, "1.57.0", Some(83310), None),
/// Allows using `#[naked]` on functions.
(active, naked_functions, "1.9.0", Some(32408), None),
/// Allows specifying modifiers in the link attribute: `#[link(modifiers = "...")]`
(active, native_link_modifiers, "1.53.0", Some(81490), None),
/// Allows specifying the as-needed link modifier
(active, native_link_modifiers_as_needed, "1.53.0", Some(81490), None),
/// Allows specifying the bundle link modifier
(active, native_link_modifiers_bundle, "1.53.0", Some(81490), None),
/// Allows specifying the verbatim link modifier
(active, native_link_modifiers_verbatim, "1.53.0", Some(81490), None),
/// Allows specifying the whole-archive link modifier
(active, native_link_modifiers_whole_archive, "1.53.0", Some(81490), None),
/// Allow negative trait implementations.
(active, negative_impls, "1.44.0", Some(68318), None),
/// Allows the `!` type. Does not imply 'exhaustive_patterns' (below) any more.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_llvm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![feature(nll)]
#![feature(native_link_modifiers)]
#![cfg_attr(bootstrap, feature(native_link_modifiers))]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]

// NOTE: This crate only exists to allow linking on mingw targets.
Expand Down
42 changes: 40 additions & 2 deletions compiler/rustc_metadata/src/native_libs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,18 @@ impl<'tcx> ItemLikeVisitor<'tcx> for Collector<'tcx> {

// Do this outside the above loop so we don't depend on modifiers coming
// after kinds
if let Some(item) = items.iter().find(|item| item.has_name(sym::modifiers)) {
let mut modifiers_count = 0;
for item in items.iter().filter(|item| item.has_name(sym::modifiers)) {
if let Some(modifiers) = item.value_str() {
modifiers_count += 1;
let span = item.name_value_literal_span().unwrap();
let mut has_duplicate_modifiers = false;
for modifier in modifiers.as_str().split(',') {
let (modifier, value) = match modifier.strip_prefix(&['+', '-']) {
Some(m) => (m, modifier.starts_with('+')),
None => {
// Note: this error also excludes the case with empty modifier
// string, like `modifiers = ""`.
sess.span_err(
span,
"invalid linking modifier syntax, expected '+' or '-' prefix \
Expand All @@ -143,6 +148,9 @@ impl<'tcx> ItemLikeVisitor<'tcx> for Collector<'tcx> {

match (modifier, &mut lib.kind) {
("bundle", NativeLibKind::Static { bundle, .. }) => {
if bundle.is_some() {
has_duplicate_modifiers = true;
}
*bundle = Some(value);
}
("bundle", _) => {
Expand All @@ -153,9 +161,17 @@ impl<'tcx> ItemLikeVisitor<'tcx> for Collector<'tcx> {
);
}

("verbatim", _) => lib.verbatim = Some(value),
("verbatim", _) => {
if lib.verbatim.is_some() {
has_duplicate_modifiers = true;
}
lib.verbatim = Some(value);
}

("whole-archive", NativeLibKind::Static { whole_archive, .. }) => {
if whole_archive.is_some() {
has_duplicate_modifiers = true;
}
*whole_archive = Some(value);
}
("whole-archive", _) => {
Expand All @@ -168,6 +184,9 @@ impl<'tcx> ItemLikeVisitor<'tcx> for Collector<'tcx> {

("as-needed", NativeLibKind::Dylib { as_needed })
| ("as-needed", NativeLibKind::Framework { as_needed }) => {
if as_needed.is_some() {
has_duplicate_modifiers = true;
}
*as_needed = Some(value);
}
("as-needed", _) => {
Expand All @@ -190,12 +209,22 @@ impl<'tcx> ItemLikeVisitor<'tcx> for Collector<'tcx> {
}
}
}
if has_duplicate_modifiers {
let msg =
"same modifier is used multiple times in a single `modifiers` argument";
sess.span_err(item.span(), msg);
}
} else {
let msg = "must be of the form `#[link(modifiers = \"...\")]`";
sess.span_err(item.span(), msg);
}
}

if modifiers_count > 1 {
let msg = "multiple `modifiers` arguments in a single `#[link]` attribute";
sess.span_err(m.span, msg);
}

// In general we require #[link(name = "...")] but we allow
// #[link(wasm_import_module = "...")] without the `name`.
let requires_name = kind_specified || lib.wasm_import_module.is_none();
Expand Down Expand Up @@ -349,6 +378,15 @@ impl Collector<'_> {
.drain_filter(|lib| {
if let Some(lib_name) = lib.name {
if lib_name.as_str() == passed_lib.name {
// FIXME: This whole logic is questionable, whether modifiers are
// involved or not, library reordering and kind overriding without
// explicit `:rename` in particular.
if lib.has_modifiers() || passed_lib.has_modifiers() {
self.tcx.sess.span_err(
self.tcx.def_span(lib.foreign_module.unwrap()),
"overriding linking modifiers from command line is not supported"
);
}
if passed_lib.kind != NativeLibKind::Unspecified {
lib.kind = passed_lib.kind;
}
Expand Down
Loading

0 comments on commit 1004783

Please sign in to comment.