Stabilize native library modifier syntax and the whole-archive modifier specifically

[petrochenkov]

Stabilization report: #93901 (comment)

cc #81490

[rust-highfive]

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

Stabilization Report

This PR partially stabilizes RFC 2951: "Linking modifiers for native libraries" (tracking issue #83507).
The RFC was implemented about 9 months ago in #83507.

What is stabilized:

• Generic native library modifier syntax both on command line and in #[link] attributes.
• One of the modifiers - whole-archive.

Generic modifier syntax

In #[link] attributes a new field is accepted - #[link(modifiers = "+foo,-bar")].
On command line the -l option is extended with modifiers, which can only be specified if the library kind is also specified - -l static:+foo,-bar=mylib.
The modifiers string is a non-empty comma-separated list of modifiers prepended with + or -, as specified in the RFC.

Multiple modifiers fields in a single #[link] attribute are not stabilized.
Multiple uses of the same modifier in a single attribute or option (e.g. +foo,-foo or +foo,+foo) are also not stabilized.
See the question (1) below for why I'm not stabilizing these.

whole-archive modifier

This is the only modifier that is being stabilized at this time.

Examples: #[link(name = "mylib", kind = "static", modifiers = "+whole-archive")], -l static:+whole-archive=mylib.

This modifier is only compatible with the static linking kind.
Using any other kind will result in a compiler error.

+whole-archive means that the static library is linked as a whole archive without throwing any object files away.

This modifier translates to --whole-archive for ld-like linkers, to /WHOLEARCHIVE for link.exe, and to -force_load for ld64.
The modifier does nothing for linkers that don't support it.

The default for this modifier is -whole-archive.
NOTE: The default may currently be different when building dylibs for some targets, but it is not guaranteed.
See the question (2) below for the details.

Test cases

• src/test/ui/native-library-link-flags/*
• src/test/ui/feature-gates/feature-gate-native_link_modifiers*
• src/test/run-make/native-link-modifier-whole-archive

Documentation

rust-lang/reference#1170

Questions requiring attention

(1) Modifier overriding

rustc has an obscure feature of overriding some data in #[link] attributes for the currently compiled crate using -l options on command line.
I dislike how it is currently implemented and would like to try changing the behavior to

• avoid reordering the libraries, and
• avoid overriding anything unless explicitly requested with the :RENAME part of the -l option.

That's why I left a FIXME in compiler/rustc_metadata/src/native_libs.rs and also kept any kind of #[link]/-l data overriding involving modifiers unstable, including the "+foo,-foo == -foo" stuff.

(2) Default value for the whole-archive modifier

The default for whole-archive is currently inconsistent.
It is usually false, but it's true when we are linking a +bundle static library directly (not through a rlib) into an executable or dylib.
These "rules" were never specified anywhere, and were changed sometimes, e.g. in 2020 whole-archive started being correctly supported on MSVC targets (#72785), but it actually caused one known regression (FaultyRAM/windres-rs#15) due to the compiler using link_whole_staticlib over-eagerly.

Ideally the default should be always false.
In this PR I switch it to false, because users now have a stable way to set whole-archive to true when necessary (which should be a rare case).

There's one problem however.
When we are building a dylib, it may reexport symbols from static libraries using extern {} blocks, see src\test\ui\abi\cross-crate\auxiliary\anon-extern-mod-cross-crate-1.rs for an example.
The problem is that the reexport by itself is not counted as a use (at least with ld-like ELF linkers), so that symbol is dropped when we are linking the dylib despite being actually necessary.
One blunt way to address this issue is to link the static library as whole-archive, then the exported symbols will also be included for sure.
This is what the current compiler behavior actually attempted to achieve, I think, it was just doing it not very correctly, with both false-positives and false-negatives.

I want to avoid breaking this dylib use case in this PR, because even if the user applied explicit +whole-archive to address such a regression, it would also be a blunt solution and not the best fix.
The best fix is to automatically mark exported symbols as used in rustc, because rustc have all the information necessary for that.
However, until that fix is implemented, rustc can use the blunt solution of using whole-archive by default when linking dylibs on some targets (the default_to_whole_archive changes in compiler\rustc_codegen_ssa\src\back\link.rs).
This solution is not used on all targets though, on Windows, for example, exported symbols are counted as used, so whole-archive is not used by default there (the exported_symbol_means_used_symbol changes in compiler\rustc_codegen_ssa\src\back\linker.rs).

(3) Consistency with --extern crate modifiers

Extern crates passed on command line can also be enhanced with modifiers on nightly - --extern foo,bar:my_crate=PATH (implemented in #67074).
There's no dedicated tracking issue for this command line syntax yet.

The currently supported modifiers are:

• priv, for Tracking issue for RFC 1977: public & private dependencies #44663
• noprelude, for -Z build-std in cargo

But it may make sense to add more of these in the future, e.g.

• prefer-dynamic, for [WIP] parameterize -C prefer-dynamic #88101
• or maybe even whole-archive for rlibs too

It would be nice to harmonize this unstable syntax with the native library modifier syntax, for example

• use + and - for boolean values - --extern +foo,-bar:my_crate=PATH,
• and maybe move the modifiers to the left of my_crate? --extern my_crate:+foo,-bar=PATH

The current syntax reference:

# Native lib modifiers
-l [KIND[:MODIFIERS]=]NAME[:RENAME]
# Rust crate modifiers
--extern [MODIFIERS:]NAME[=PATH]

[michaelwoerister]

Great to see this moving forward!

Will there be a stable way to achieve the effect of -bundle,+whole-archive after this PR?

[petrochenkov]

@michaelwoerister

Will there be a stable way to achieve the effect of -bundle,+whole-archive after this PR?

No, but I plan to prepare a stabilization PR for the bundle modifier next.

[pnkfelix]

This is really great work, thank you @petrochenkov

[pnkfelix]

Based on my reading of the "Questions requiring attention", I don't think any of them would block the stabilization proposed here. Its more stuff we need to think about as we move forward beyond this.

So, under that assumption, I'm going to propose FCP. But I want everyone to also look carefully at that stabilization report and think about it before checking your box!

[pnkfelix]

@rfcbot fcp merge

[rfcbot]

Team member @pnkfelix has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Aaron1011
• @cjgillot
• @davidtwco
• @eddyb
• @estebank
• @lcnr
• @matthewjasper
• @michaelwoerister
• @nagisa
• @nikomatsakis
• @oli-obk
• @petrochenkov
• @pnkfelix
• @wesleywiser

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[wesleywiser]

Questions requiring attention

(2) Default value for the whole-archive modifier

I want to avoid breaking this dylib use case in this PR, because even if the user applied explicit +whole-archive to address such a regression, it would also be a blunt solution and not the best fix.
...
However, until that fix is implemented, rustc can use the blunt solution of using whole-archive by default when linking dylibs on some targets (the default_to_whole_archive changes in compiler\rustc_codegen_ssa\src\back\link.rs).

I agree with the motivation to avoid breaking that use case, but I wonder if we think there is much risk that users will come to rely on dylibs being linked whole-archive in this instance?

[petrochenkov]

I wonder if we think there is much risk that users will come to rely on dylibs being linked whole-archive in this instance

There's some additional risk - non-bundled staticlib dependencies will be linked as whole-archive, but those cases are not stable yet (due to -bundle/no-bundle), so it's not a problem until the bundle modifier is stabilized.

However, some of that risk is already here because bundled staticlib dependencies (aka all the stable ones) are already linked as whole-archive and it's possible that someone already depends on it.
I hope that it won't be a problem in practice because Rust dylibs are not recommended for general use.

[petrochenkov]

non-bundled staticlib dependencies will be linked as whole-archive

We could leave this case as is though, in that case dylibs reexporting something from non-bundled dependencies will continue being broken by default (I suspect we have some issues about this on our tracker).

[wesleywiser]

That seems reasonable then. Thanks!

[aadog]

yes

[aadog]

[aadog]

cargo.exe build --color=always --message-format=json-diagnostic-rendered-ansi --package frida-shared

[petrochenkov]

@aadog
The workaround in this case is to disable printing of cargo:rustc-link-lib=static=frida-core from build.rs.

It may happen implicitly (https://docs.rs/cc/1.0.73/src/cc/lib.rs.html#1017-1018) in https://docs.rs/cc/1.0.73/cc/struct.Build.html#method.compile
In that case it can be disabled with cc.cargo_metadata(false), then you'll have to re-add cargo:rustc-link-search=native=... manually.

Ideally this could also be fixed by using rust-lang/cc-rs#671, but cc-rs is currently unmaintained unfortunately.

[aadog]

@petrochenkov
I just started learning rust, and it seems that I encountered a lot of difficulties in the process. My purpose is to statically compile c to a dylib and re-export symbols

[aadog]

@petrochenkov If it is such a simple function how should I operate with a minimal example

[aadog]

Export all symbols, or, specify a list of symbols to export

[petrochenkov]

@aadog
For questions unrelated to native library modifiers specifically I suggest asking people on https://users.rust-lang.org instead of here.

[aadog]

Is this feature based on rlib rather than cdylib?

[jsgf]

I know this is way too late, and probably the wrong place to raise this objection but I see the whole #[link] mechanism as a mistake and I'd really rather see steps towards deprecating it than extending it.

Fundamentally, what #[link] controls is out of the scope of rustc, and Rust in general, since it's primarily about how non-Rust dependencies are linked. This is the perview of the build system rather than the compiler, and as such I'd prefer these kinds of annotations be moved to Cargo, or purely restricted to build-script<=>cargo. That is, there should be no in-source #[link] annotations.

At least we have -Zlink-native-libraries=no to override #[link] in the cases where it is unwanted (though I think I want to change it to ignore #[link] directives at link time, not at compilation time).

#73632 (comment) is some more general context of where I'm coming from.

[petrochenkov]

@jsgf
The question of modifiers is pretty orthogonal to the "#[link] vs build system" question, they just support both ways indiscriminately.
The "build system" alternative means -l options passed to rustc (if rustc is used for linking at all), and the modifiers are supported for this alternative.

At least we have -Zlink-native-libraries=no to override #[link] in the cases where it is unwanted (though I think I want to change it to ignore #[link] directives at link time, not at compilation time).

What do you mean?
-Zlink-native-libraries=no already works at link time.