Improve handling of dllexport #27416
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
alexcrichton
force-pushed
the
fix-dll-export
branch
from
c33318a to
de5e33f
Compare
👍 Also for that matter, how much does this decrease the size of the DLLs bundled with the Rust distribution? |
|
☔ The latest upstream changes (presumably #27210) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
force-pushed
the
fix-dll-export
branch
from
de5e33f to
2b3708e
Compare
|
☔ The latest upstream changes (presumably #27458) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
force-pushed
the
fix-dll-export
branch
from
2b3708e to
80936de
Compare
|
Rebased, ping r? @brson
Yeah right now we don't have a great distinction between "dylib later linked into a Rust project" and "dylib only used to link into an external C-like project", so we have to continue to pessimistically export everything. If we add this distinction, however, then we could get the same wins on dylibs as we're getting on executables. I'd also recommend avoiding editing github comments, unfortunately no notifications are sent out after an edit... |
|
This will break dylibs on windows that, like rustllvm, export native symbols, right? On stable it also leaves no recourse for fixing the breakage. I wonder if this comes up anywhere in the wild. Maybe some of Cargo's bindings? |
|
Thankfully this doesn't actually break any of them that weren't already broken. If the native library is compiled with LLVM doesn't support a mode of being built with In short, this doesn't actually change the story for any native library linked into an MSVC binary today. It still sucks that you have to remember to |
|
@alexcrichton If we really want some wins with dynamic libraries we should respect the distinction between |
|
@bors r+ |
|
📌 Commit 80936de has been approved by |
|
@retep998 that would only help binary size, right? I'm not too too worried about it because Rust dlls are so uncommon. This is another case where a distinction between a DLL-used-with-a-C-api and a DLL-used-as-a-rust-dependency would be useful, however. |
|
@alexcrichton It would help binary size when you are distributing DLLs with a Rust binary where nobody will link to those DLLs. For example the DLLs sitting next to |
|
Sure, but this is nothing that has to do specifically with Windows, this is a problem that plagues all platforms. |
|
☔ The latest upstream changes (presumably #27338) made this pull request unmergeable. Please resolve the merge conflicts. |
To correctly reexport statically included libraries from a DLL on Windows, the compiler will soon need to have knowledge about what symbols are statically included and which are not. To solve this problem a new unstable `#[linked_from]` attribute is being added and recognized on `extern` blocks to indicate which native library the symbols are coming from. The compiler then keeps track of what the set of FFI symbols are that are included statically. This information will be used in a future commit to configure how we invoke the linker on Windows.
Rust's current compilation model makes it impossible on Windows to generate one object file with a complete and final set of dllexport annotations. This is because when an object is generated the compiler doesn't actually know if it will later be included in a dynamic library or not. The compiler works around this today by flagging *everything* as dllexport, but this has the drawback of exposing too much. Thankfully there are alternate methods of specifying the exported surface area of a dll on Windows, one of which is passing a `*.def` file to the linker which lists all public symbols of the dynamic library. This commit removes all locations that add `dllexport` to LLVM variables and instead dynamically generates a `*.def` file which is passed to the linker. This file will include all the public symbols of the current object file as well as all upstream libraries, and the crucial aspect is that it's only used when generating a dynamic library. When generating an executable this file isn't generated, so all the symbols aren't exported from an executable. To ensure that statically included native libraries are reexported correctly, the previously added support for the `#[linked_from]` attribute is used to determine the set of FFI symbols that are exported from a dynamic library, and this is required to get the compiler to link correctly.
alexcrichton
force-pushed
the
fix-dll-export
branch
from
80936de to
e648c96
Compare
These two commits are aimed at "fixing" our usage of `dllexport` in the compiler. Currently we blanket apply this attribute to *everything* public in a crate, but this ends up with a few downsides: * Executables are larger than the should be as a result of thinking they should export everything * Native libraries aren't handled correctly because technically a statically included native library should be exported from a DLL in some cases. * Symbols don't actually need to be exported if they never end up in a DLL. The first commit adds a new unstable attribute, `#[linked_from]`, which is a way to tell the compiler what native library a block of symbols comes from. This is used to inform the compiler what set of native libraries are statically included in the rlib (or other output). This information is later used to export them from a DLL if necessary. Currently this is only used in a few places (such as the LLVM bindings) to get the compiler to link correctly. The second commit stops adding `dllexport` to all items in LLVM and instead explicitly telling the linker what symbols should be exported. We only need to do this when building a dynamic library, and otherwise we can avoid adding `dllexport` or telling the linker about exported symbols. As a testament to this change, the size of "Hello World" on MSVC drops from 1.2MB to 67KB as a result of this patch. This is because the linker can much more aggressively remove unused code. These commits do not yet attempt to fix our story with `dllimport`, and I'll leave that to a future PR and issue, for now though I'm going to say that this Closes #7196
These two commits are aimed at "fixing" our usage of
dllexportin the compiler. Currently we blanket apply this attribute to everything public in a crate, but this ends up with a few downsides:The first commit adds a new unstable attribute,
#[linked_from], which is a way to tell the compiler what native library a block of symbols comes from. This is used to inform the compiler what set of native libraries are statically included in the rlib (or other output). This information is later used to export them from a DLL if necessary. Currently this is only used in a few places (such as the LLVM bindings) to get the compiler to link correctly.The second commit stops adding
dllexportto all items in LLVM and instead explicitly telling the linker what symbols should be exported. We only need to do this when building a dynamic library, and otherwise we can avoid addingdllexportor telling the linker about exported symbols.As a testament to this change, the size of "Hello World" on MSVC drops from 1.2MB to 67KB as a result of this patch. This is because the linker can much more aggressively remove unused code.
These commits do not yet attempt to fix our story with
dllimport, and I'll leave that to a future PR and issue, for now though I'm going to say that thisCloses #7196