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

libstd.dylib always depends on liballoc_jemalloc (and libpanic_unwind) #43637

Closed
RalfJung opened this issue Aug 4, 2017 · 21 comments
Closed
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Aug 4, 2017

I am trying to use xargo to build a libstd without jemalloc. (That should enable cross-compiling libstd without cross-compiling any C code.)

I have the following Xargo.toml:

[dependencies]
std = {features = ["panic_abort", "force_alloc_system"]}

Now xargo build -v shows:

...
     Running `rustc --crate-name std /home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/lib.rs --crate-type dylib --crate-type rlib --emit=dep-info,link -C prefer-dynamic -C opt-level=3 --cfg 'feature="force_alloc_system"' --cfg 'feature="panic_abort"' -C metadata=f2ad81e4aae1da77 --out-dir /tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps -L dependency=/tmp/xargo.j52xO5VAig3n/target/release/deps --extern rustc_tsan=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/librustc_tsan-b7ca73d041615ffd.rlib --extern alloc=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/liballoc-210591387bfdd319.rlib --extern panic_abort=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/libpanic_abort-2640770c1084be03.rlib --extern core=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/libcore-c98bfbbe49faadd5.rlib --extern std_unicode=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/libstd_unicode-c551e0da6902f65e.rlib --extern libc=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/liblibc-a777dc7e09579a84.rlib --extern unwind=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/libunwind-bfa0f42576422ec8.rlib --extern compiler_builtins=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/libcompiler_builtins-77a37c3c1af692d9.rlib --extern rustc_asan=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/librustc_asan-1b93e3a38c735df5.rlib --extern collections=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/libcollections-9a131e253f0f0440.rlib --extern rand=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/librand-5667d73b4da05dc4.rlib --extern alloc_system=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/liballoc_system-f1890d93ca3ab3ce.rlib --extern rustc_msan=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/librustc_msan-e33a58e222e716d7.rlib --extern rustc_lsan=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/deps/librustc_lsan-ad0c635ae24b3237.rlib --sysroot /home/r/.xargo/HOST -l dl -l rt -l pthread -L native=/tmp/xargo.j52xO5VAig3n/target/x86_64-unknown-linux-gnu/release/build/compiler_builtins-7685fddded0d8535/out`
error[E0463]: can't find crate for `alloc_jemalloc`

error: aborting due to previous error

error: Could not compile `std`.

This shows that the correct feature flags are passed to rustc. However, for some reason, rustc behaves as if libstd contained extern crate alloc_jemalloc, which it does not. Is there magic going on here that adds alloc_jemalloc nevertheless? I did some grepping of the code but found nothing.

(EDIT: There seems to be similar magic behavior for panic_unwind.)

If somehow it is not possible to get rid of this magic "link against liballoc_jemalloc", then (a) libstd/Cargo.toml should be changed to reflect this fact by always depending on liballoc_jemalloc, and (b) I hope there's a chance we can get a feature flag on liballoc_jemalloc itself to enable "dummy_jemalloc" mode (currently controlled by build.rs), and have libstd set that flag when force_alloc_system is set.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

Looking at all this feature flag stuff, I get more and more confused. For example, libstd does debug-jemalloc = ["alloc_jemalloc/debug"], which I thought would enable the debug flag on alloc_jemalloc, but then liballoc_jemalloc/build.rs uses cfg!(feature = "debug-jemalloc")... so does this directly access libstd' flags after all?

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

Cc @oli-obk

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

I pushed some experiments with the feature flags to https://github.com/RalfJung/rust/tree/force_alloc_system (diff: rust-lang:eae446c...RalfJung:2b4b3f0). Note that I have no idea what I am doing here.

But anyway, with that patch, xargo can actually successfully build libstd with the Xargo.toml given above.

EDIT: However, cross-compilation still requires a C compiler due to compiler_builtins. :/

@oli-obk
Copy link
Contributor

oli-obk commented Aug 4, 2017

cc @aidanhs

@aidanhs
Copy link
Member

aidanhs commented Aug 4, 2017

Another victim of the tantalising feature name of force_alloc_system :( So, first off - force_alloc_system was not not created for this reason and shouldn't be used here (it's just a workaround for a scenario related to local rebuilds).

Setting the lib and exe default allocator crates in a target specification may have some relevance. #43524 (cc @vorner) is a recent issue talking about something similar.

Failing that, there is a bug related to builds without jemalloc (fixed at some point by #43589), but that only affects stage0 builds. I've never used xargo, but a quick grep indicates that it implicitly does a stage0 compilation, so you could try applying that patch and see what happens.

I'm not yet sure how the allocator crate specs and the patch above will interact, but I'm sure it will be exciting to find out.

I have no idea about compiler builtins unfortunately. Possibly a question to ask on the xargo repo, as they might have more experience.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 4, 2017

So, first off - force_alloc_system was not not created for this reason and shouldn't be used here

I know. I read your other post about it. I just wanted to see whether we can build upon it.

We'll retry without force_alloc_system after your PR goes through.

@aidanhs
Copy link
Member

aidanhs commented Aug 4, 2017

@oli-obk ah sorry, since jemalloc broke I've said this in a lot of places so I've forgotten who it's been said to. Apart from anything else though, it's actually a no-op right now because it was removed in the global allocator PR - my patch would make it actually do something again.

@shepmaster shepmaster added A-build C-feature-request Category: A feature request, i.e: not implemented / a PR. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 4, 2017
@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

Another victim of the tantalising feature name of force_alloc_system :( So, first off - force_alloc_system was not not created for this reason and shouldn't be used here (it's just a workaround for a scenario related to local rebuilds).

I see. That may explain some things, but it does not explain where the magic dependencies come from -- compiling libstd literally has rustc look for liballoc_jemalloc and panic_unwind even though there is no "extern crate".

However, why should libstd be forced to depend on jemalloc on stage1+?

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

I've never used xargo, but a quick grep indicates that it implicitly does a stage0 compilation, so you could try applying that patch and see what happens.

How is that determined? I started at the rustc calls that are actually run in the end, but they don't seem to specify a stage. Thinking about it, probably there are environment variables as well... makes me wonder how I could see what these look like.

@oli-obk
Copy link
Contributor

oli-obk commented Aug 4, 2017

Related #37975

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

@eddyb helped with the magic dependencies, and it turns out the reason is that libstd is built as a dylib which requires it to get a panic and allocation library, which is how these additional dependencies appear.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 4, 2017

I tried to fix this by making building libstd as dylib optional (from what I understand, that shouldn't usually be needed for xargo use cases?), but I am not able to change the crate type depending on a feature. I can set the crate type inside lib.rs and then using cfg_attr, but then the build fails:

   Compiling rustc_errors v0.0.0 (file:///home/r/src/rust/rustc.2/src/librustc_errors)
error: cannot satisfy dependencies so `std` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `core` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `rand` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `alloc` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `std_unicode` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `alloc_system` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `libc` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `unwind` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `compiler_builtins` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `alloc_jemalloc` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: cannot satisfy dependencies so `panic_unwind` only shows up once
  |
  = help: having upstream crates all available in one format will likely make this go away

error: aborting due to 11 previous errors

error: Could not compile `rustc_errors`.

Caused by:
  process didn't exit successfully: `/home/r/src/rust/rustc.2/build/bootstrap/debug/rustc --crate-name rustc_errors src/librustc_errors/lib.rs --error-format json --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=8861e0bae2d701bf -C extra-filename=-8861e0bae2d701bf --out-dir /home/r/src/rust/rustc.2/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/home/r/src/rust/rustc.2/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/home/r/src/rust/rustc.2/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps --extern serialize=/home/r/src/rust/rustc.2/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-7949b697b8b9ae91.so --extern serialize=/home/r/src/rust/rustc.2/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libserialize-7949b697b8b9ae91.rlib --extern syntax_pos=/home/r/src/rust/rustc.2/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax_pos-60df18e62faffff4.so` (exit code: 101)
warning: build failed, waiting for other jobs to finish...
error: build failed

@eddyb
Copy link
Member

eddyb commented Aug 4, 2017

@RalfJung Ahh my bad, I guess you can't do it like this, because Cargo needs to know.

@aidanhs
Copy link
Member

aidanhs commented Aug 5, 2017

However, why should libstd be forced to depend on jemalloc on stage1+?

  1. stage0 compiles the stage1 libstd.so. stage0 may or may not have jemalloc (and if it has it, it will default to it), and we may or may not want to link jemalloc into libstd. To sidestep handling the four possibilities here, we just always use the system allocator, which is guaranteed to be available.
  2. stage0 compiles the rest of stage1 rustc
  3. stage1 compiles the stage1 rlibs (including allocator crates as appropriate for the configure option we've chosen)
  4. stage1 compiles the stage2 libstd.so. At this point ourdefault preference of allocator has been compiled into rustc (e.g. defaulting to jemalloc, or system if jemalloc is disabled) and we know that whatever allocator crate we want to use by default is available because we just compiled it in step 3, so we don't need to think at all about allocators any more, we just use the default

How is that determined? I started at the rustc calls that are actually run in the end, but they don't seem to specify a stage.

Sorry, my comment sounds way more authoritative than I actually felt. I was basing it on https://github.com/japaric/xargo/blob/b67abf8c1ad5e129618117715a10acf8aacadf6d/src/sysroot.rs#L298-L307 which appears to pick 0 by default, but a more careful examination doesn't show this being used anywhere. My mistake!

@RalfJung
Copy link
Member Author

RalfJung commented Aug 5, 2017

The stage1 libstd.rlib and libstd.so are built by different compilers?!? Okay THAT is unexpected.

But anyway the main reason libstd needs an allocator dependency is that it is a dylib, not just an rlib, and that's the source of my confusion. Ideally there would be a way to compile it as rlib-only. However, beyond that, it does seem reasonable to me to be able to configure which allocator and panic handler is used by the dylib. For the panic handler, that can already be controlled via Cargo.toml panic = "abort", but for the allocator, I don't think there is such a switch. A libstd feature would make sense. Something exactly like force_system_allocator indeed, except that it should only affect the dylib, not the rlib...

@eddyb
Copy link
Member

eddyb commented Aug 5, 2017

The stage1 libstd.rlib and libstd.so are built by different compilers

I don't think that's what @aidanhs is saying, I think you can s/libstd.so/libstd.{so,rlib}.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 5, 2017

I guess I misunderstood these two:

stage0 compiles the stage1 libstd.so

stage1 compiles the stage1 rlibs

I thought rlibs included libstd.rlib. Seems it does not.

@eddyb
Copy link
Member

eddyb commented Aug 5, 2017

@RalfJung libstd is in there but specifically it's also a dylib so it gets special treatment.

@RalfJung RalfJung changed the title libstd tries to link to liballoc_jemalloc even when it is not enabled libstd.dylib always depends on liballoc_jemalloc Aug 5, 2017
@RalfJung
Copy link
Member Author

RalfJung commented Aug 5, 2017

Sorry, my comment sounds way more authoritative than I actually felt. I was basing it on https://github.com/japaric/xargo/blob/b67abf8c1ad5e129618117715a10acf8aacadf6d/src/sysroot.rs#L298-L307 which appears to pick 0 by default, but a more careful examination doesn't show this being used anywhere. My mistake!

Ah, these are xargo stages that have nothing to do with bootstrap stages. See https://github.com/japaric/xargo#multi-stage-builds for more information.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 5, 2017

If I understood things correctly, the problem here is the following:

  • libstd is built both as rlib and as dylib. The dylib is needed for procedural macros. The rlib is used for normal builds.
  • dylibs need to pick a concrete allocator and panic handler. (Not sure why that is the case, but it is what happens.) This adds two dependencies to libstd which are not caused by "extern crate", and which are not accounted for by Cargo.toml, confusing xargo.
  • Cargo provides a way to control the panic handler, so while irritating, we can actually suppress the magic dependency on panic_unwind by setting panic = "abort".
  • However, there is nothing like that for the allocator. The global allocator is controlled by an attribute that must exist no more than once in the entire crate tree. If it doesn't exist, it falls back to the default for this target. The libstd.rlib obviously should not contain this attribute or else it would fix the allocator for everyone, defeating the purpose of pluggable allocators. Currently, however, this means that libstd.dylib also cannot contain this attribute. As a consequence, libstd.dylib always uses the default allocator for the target, and this cannot be controlled by e.g. xargo.

Please correct me if this is wrong.

Based on this, the ideal solution for me seems to be to provide some way for libstd.dylib to control its allocator independent of the target default, and without affecting the libstd.rlib.

Is it possible to make the items of a crate different for the rlib and dylib version of the crate, respectively? If not, the problem outlined above would be a limitation affecting all crates that are both rlib and dylib. (I suppose that's a rare combination.) I don't expect this to be possible, but who knows. ;)
In the absence of this option, the only alternative I can think of it to not make libstd itself a dylib, but instead make libstd rlib-only, and have another crate (libstd_proc or so?) which is dylib-only and used by procedural macros. Then libstd_proc could (optionally, controlled by features) set a particular allocator.

@RalfJung RalfJung changed the title libstd.dylib always depends on liballoc_jemalloc libstd.dylib always depends on liballoc_jemalloc (and libpanic_unwind) Aug 19, 2017
@alexcrichton
Copy link
Member

This is a pretty old issue and fundamentally isn't really solvable in isolation, so I'm going to close this in favor of #36963 which we can probably do on the nearer horizon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants