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

libiconv is still unconditionally linked to all binaries on macOS #2870

Closed
winterqt opened this issue Aug 10, 2022 · 4 comments · Fixed by #2944
Closed

libiconv is still unconditionally linked to all binaries on macOS #2870

winterqt opened this issue Aug 10, 2022 · 4 comments · Fixed by #2944
Labels
C-bug Category: bug O-macos

Comments

@winterqt
Copy link

$ rustc -Vv
rustc 1.64.0-nightly (93ffde6f0 2022-07-23)
binary: rustc
commit-hash: 93ffde6f04d3d24327a4e17a2a2bf4f63c246235
commit-date: 2022-07-23
host: aarch64-apple-darwin
release: 1.64.0-nightly
LLVM version: 14.0.6
$ cat main.rs
fn main() {}
$ rustc main.rs
$ otool -L main
main:
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
	/usr/lib/libresolv.9.dylib (compatibility version 1.0.0, current version 1.0.0)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)

The libc version in this compiler's std is 0.2.126, which includes #2150, which should have fixed this.

@JohnTitor
Copy link
Member

The problem is that I don't have a macOS machine to debug, cc @goffrie (again).

@JohnTitor JohnTitor added C-bug Category: bug O-macos labels Aug 11, 2022
@bjorn3
Copy link
Member

bjorn3 commented Aug 17, 2022

I don't think rustc cares about if functions in an extern block are used, but unconditionally considers the library referenced by the extern block used.

@goffrie
Copy link
Contributor

goffrie commented Aug 17, 2022

Interesting, I'm pretty sure you're correct. I thought I tested #2150 manually but I might have fooled myself.

In that case I'm not sure that we can actually do anything. libiconv is part of the macOS base system at least so it shouldn't pose a problem at runtime.

winterqt added a commit to winterqt/nixpkgs that referenced this issue Sep 7, 2022
Rust binaries are unconditionally linked to libiconv on Darwin (see rust-lang/libc#2870). We already add it as a dependency in `buildRustPackage`, so let's go a step further and propagate it.
@thomcc
Copy link
Member

thomcc commented Oct 7, 2022

It poses a problem in several cases any build system that tries to accurately track dependencies or linkage has to deal with this -- you see this with the Nix discussion that seems to have referenced the issue. It causes various conflicts with C code occasionally, slows down linking/loading/startup/dlsym/etc, and so on. It would also cause massive issues for all Rust programs if libiconv ever bumped to version 8.0.0 (it's on 7.0.0), or if it bumped the soname to libiconv.3. I also think the fact that it isn't re-exported from libSystem.B.{tbd,dylib} is likely deliberate (probably due to the size -- all those conversions are not free).

Additionally, it's also essentially pointless -- nobody uses the libc bindings for these functions. There are zero hits on libc::iconv language:Rust in github's code search. Grepping for iconv language:Rust shows a number of people declaring their own bindings to the iconv functions rather than using the ones from libc. Many of these have the #[link(name = "iconv")] they'd need, and the ones that don't... well, I don't think we want to guarantee that we pull in libraries you don't use.

Concretely: std does/should not guarantee that it will any particular C library transitively, and while in this case you could make the argument that libc should... in cases like this, I'm not sure I agree. It's not like we link with all of the other things in $sdkroot/usr/lib on macOS, nor even all the other things in $sdkroot/usr/lib that are covered by POSIX, nor even everything in $sdkroot/usr/lib that is both covered by POSIX and declared in this crate.

I suppose ideally, there might be a #[link] modifier for "only link this if the things in this extern block are referenced". I dunno if that's very easy to do in the compiler, though, since that's only really known by the linker.

I thought it was weirdly needed for unwinding or something, but it's not, so it's pointless to pull this into all Rust programs. As a first step, I'll put up #[cfg_attr(not(feature = "rustc-dep-of-std"), link_name = "iconv")], which should be less controversial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug O-macos
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants