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

dylib shared libraries will not make public symbols that may be necessary to link inlined code #65610

Open
nagisa opened this issue Oct 19, 2019 · 14 comments
Assignees
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented Oct 19, 2019

When a dylib crate has a public inline(always) functions in it that use, as an implementation, other, private functions, using these public functions from other crates will fail with linkage errors because we fail to "export" the private functions.

An example project can be seen here. In this example the driver dylib crate privately externs a symbol from a static C library and uses it to implement an inline(always) interface/wrapper. The user crate then attempts to use the inline(always) wrapper, but linking fails with an error such as this:

/tmp/user/driver/src/lib.rs:8: undefined reference to `private_extern_symbol'

When inspecting the libdriver.so we can see that the extern symbol does indeed exist but is "private":

0000000000001100 t private_extern_symbol

I think we might be un-exporting items too aggressively here. cc @michaelwoerister @oli-obk

Blocks #55617

Regression from 1.36.0 (example builds successfully) to 1.37 (example fails to build).

@nagisa nagisa added A-linkage Area: linking into static, shared libraries and binaries T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Oct 19, 2019
@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Oct 19, 2019
@michaelwoerister
Copy link
Member

Yeah, I can imagine that that doesn't work. Maybe #59752 is related?

@nagisa
Copy link
Member Author

nagisa commented Oct 23, 2019

Did a rustc bisect and it is indeed dbebcee (#59752)

@nagisa
Copy link
Member Author

nagisa commented Oct 23, 2019

cc @Zoxc

@Zoxc
Copy link
Contributor

Zoxc commented Oct 23, 2019

You haven't told rustc where that symbol comes from. Try adding #[link(name = "foo", kind = "static")] on the extern block.

@nagisa
Copy link
Member Author

nagisa commented Oct 23, 2019

Adding the attribute helped, however these symbols are originally linked via command line arguments that cc crate emits to cargo from the build.rs script. Why the two ways to link in stuff have different behaviour?

@Zoxc
Copy link
Contributor

Zoxc commented Oct 24, 2019

When you link the library on the command line, rustc doesn't know which symbols are in it, so it won't export any.

@nagisa
Copy link
Member Author

nagisa commented Oct 24, 2019

Okay, that’s somewhat bad, then, because libraries that rely exclusively on arguments emitted by cc for linking purposes are very abound in the ecosystem.

We should look at updating the cc’s documentation to include information about needing the #[link] attribute regardless. cc @alexcrichton

@Zoxc
Copy link
Contributor

Zoxc commented Oct 24, 2019

We could perhaps just consider all reachable extern fns to have static linkage and export them. I'm not sure if there's any problems with that though...

@alexcrichton
Copy link
Member

I don't think you need the kind on the block since the kind is specified on the command line and that overrides what the block says, but yes adding this documentation to cc seems fine.

@pnkfelix pnkfelix self-assigned this Dec 5, 2019
nagisa added a commit to rust-lang/stacker that referenced this issue Mar 29, 2020
As of recently it is no longer sufficient to just build the `libX.a`
with the cc crate in build.rs. Due to
rust-lang/rust#65610 it is also necessary to
specify the fact of linkage in the source for it to work in all
scenarios. The most frustrating part is that it only fails when shared
libraries are in the equation, which is a comparatively rare use-case in
Rust-land.
nagisa added a commit to rust-lang/stacker that referenced this issue Mar 29, 2020
As of recently it is no longer sufficient to just build the `libX.a`
with the cc crate in build.rs. Due to
rust-lang/rust#65610 it is also necessary to
specify the fact of linkage in the source for it to work in all
scenarios. The most frustrating part is that it only fails when shared
libraries are in the equation, which is a comparatively rare use-case in
Rust-land.
@o0Ignition0o
Copy link
Contributor

@rustbot modify labels to +I-prioritize

@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 27, 2020
@retep998
Copy link
Member

Summary of current situation:

In order for a symbol pulled in from a native library to be exported from a dylib, Rust has to know that the symbol comes from a static library. This is currently determined by the #[link] attribute attached to the extern block containing those symbols. The kind specified, either via the attribute directly or overridden by a build script later, will determine whether to export the symbol but also has some other effects:

  • kind=dylib. Symbols pulled in will not get exported from dylib crates. On Windows dllimport will be applied to those symbols.
  • kind=static. Symbols pulled in will get exported from dylib crates. On Windows dllimport will not be applied to those symbols. Notably, if the native library is linked via a dependency of the dylib crate and not the dylib directly, then it will be unable to link to static libraries provided by the system.
  • kind=static-nobundle. Similar to kind=static in that symbols will get exported from dylib crates and dllimport will not be applied, but it does always work with static system libraries.
  • No kind specified. A weird case where symbols pulled in will not get exported from dylib crates but also dllimport will not be applied to those symbols. When trying to link to static system libraries, due to kind=static-nobundle being unstable, people will often just not specify the kind so that the library can be found by the linker, and dllimport will not be applied, however this runs into the problem stated in this issue where the symbol is not exported from dylib crates.

Therefore to solve this problem we need to:

  • Add documentation stressing the importance of attaching #[link] attributes to extern blocks and specifying the kind.
  • Stabilize kind=static-nobundle in some form.

@nagisa
Copy link
Member Author

nagisa commented May 28, 2020

I’ve seen this mistake one too many times. And it tends to remain that way because just -lbanana on CLI usually works. A compiler warning would be a great way to do the stressing. For instance:

warning: -lbanana was specified on the command line, but no extern block refers to it
help: its a problem because…, see [this link] to learn more.

possibly could be a clippy lint too.

@retep998
Copy link
Member

retep998 commented May 28, 2020

Note that if symbols are spread across multiple extern blocks, having to annotate each of them with the appropriate #[link] attribute can cause significant repetition in the linker command line. This is the reason I don't specify #[link] on extern blocks in winapi even though I should in order to gain the benefits of dllimport: #38460

Also note that on Windows it is possible for a library to have both static and dynamic symbols.

Perhaps it may be worthwhile to allow us to specify whether a symbol comes from a static library or a dynamic library without having to name it. Allowing #[link(kind="static")] with no name specified could be very useful for crate authors to ensure their symbols are exported from dylibs or have the correct dllimport annotations without running into that linker repetition issue.

@spastorino
Copy link
Member

Assigning P-medium as discussed as part of the Prioritization Working Group process and removing I-prioritize.

@spastorino spastorino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries C-bug Category: This is a bug. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants