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

from nightly-2021-04-26 started linking unneeded ws2_32.dll for x86_64-pc-windows-msvc target #85441

Closed
klensy opened this issue May 18, 2021 · 2 comments · Fixed by #89472
Closed
Labels
A-linkage Area: linking into static, shared libraries and binaries O-windows Operating system: Windows

Comments

@klensy
Copy link
Contributor

klensy commented May 18, 2021

For empty.rs:

fn main(){}

rustc +nightly-2021-04-25-x86_64-pc-windows-msvc empty.rs -O
don't use ws2_32.dll

 KERNEL32.dll
 VCRUNTIME140.dll
 api-ms-win-crt-runtime-l1-1-0.dll
 api-ms-win-crt-math-l1-1-0.dll
 api-ms-win-crt-stdio-l1-1-0.dll
 api-ms-win-crt-locale-l1-1-0.dll
 api-ms-win-crt-heap-l1-1-0.dll

rustc +nightly-2021-04-26-x86_64-pc-windows-msvc empty.rs -O
use ws2_32.dll

 WS2_32.dll
 KERNEL32.dll
 VCRUNTIME140.dll
 api-ms-win-crt-runtime-l1-1-0.dll
 api-ms-win-crt-math-l1-1-0.dll
 api-ms-win-crt-stdio-l1-1-0.dll
 api-ms-win-crt-locale-l1-1-0.dll
 api-ms-win-crt-heap-l1-1-0.dll

Can be checked for example via dumpbin.exe /dependents empty.exe

regressed between 42816d6...3709ae3, i'm thinking about #84115

Thing that imported from ws2_32 is WSACleanup

@rustbot label +O-windows +A-linkage

@rustbot rustbot added A-linkage Area: linking into static, shared libraries and binaries O-windows Operating system: Windows labels May 18, 2021
@klensy
Copy link
Contributor Author

klensy commented May 21, 2021

@CDirkx

@CDirkx
Copy link
Contributor

CDirkx commented May 21, 2021

Yeah that makes sense that it is from #84115.

pub fn cleanup() {
if INIT.is_completed() {
// only close the socket interface if it has actually been started
unsafe {
c::WSACleanup();
}
}
}

cleanup always called, which makes Rust link to WSACleanup from ws2_32.dll, even if no networking code is ever used. I wonder if extracting WSACleanup into a seperate non-inlined function would be enough:

pub fn cleanup() { 
    // only perform cleanup if network functionality was actually initialized
    if INIT.is_completed() { 
        _cleanup();
    }
}

// avoid linking to `WSACleanup` if it is never used by letting this function to be optimized away
#[inline(never)]
fn _cleanup() {
    unsafe { 
        // close the socket interface
        c::WSACleanup();
    }
}

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 6, 2021
Only register `WSACleanup` if `WSAStartup` is actually ever called

Fix for rust-lang#85441.

Because `WSACleanup` appears in `cleanup` currently `WS2_32.dll` is always linked, even if no network functionality is ever used.
To prevent this, `WSACleanup` has to only appear in `init`, hence the workaround of registering it in a static.

If anyone knows a cleaner solution, let me know.
nagisa added a commit to nagisa/rust that referenced this issue Oct 2, 2021
On MinGW toolchains the various features (such as function sections)
necessary to eliminate dead function references are disabled due to
various bugs. This means that the windows sockets library will most
likely remain linked to any mingw toolchain built program that also
utilizes libstd.

That said, I made an attempt to also enable `function-sections` and
`--gc-sections` during my experiments, but the symbol references
remained, sadly.
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 4, 2021
Only register `WSACleanup` if `WSAStartup` is actually ever called

See rust-lang#85595

Fixes rust-lang#85441
@bors bors closed this as completed in e021a10 Oct 4, 2021
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 O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants