-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Formal support for linking rlibs using a non-Rust linker #73632
Comments
What you really want is a Related reading at #38913 |
I'm still don't understand what are the requirements exactly, but if rlibs (as they exist now) work for you, but staticlibs don't, then perhaps you need some variation of staticlibs with different bundling behavior for dependencies, rather than rlibs. |
Yep.
I agree - this is exactly what we need. |
@adetaylor
where What the expected result of compiling |
Right now, it would be a The problem comes in this scenario:
In this case, we can't build |
I don't mean right now, I mean what compilation of
|
Exactly! Except that the JSON file probably isn't necessary. Our build system (gn + ninja) works extremely hard to establish static dependency relationships in advance, so that a rebuild builds the minimal required set. We would expect to use those relationships in the subsequent linker invocation unless there were some reason that we needed to query it dynamically. |
This seems like a reasonable feature to me to support. The goal of rlibs was to reserve space for rustc to "do its thing" so we intentionally stabilize some format rather than accidentally do so. I don't think there's a need for a "staticlib-nobundle" because that's basically what an rlib is. While @nagisa is right in that rlibs can have varying contents, I think that it's fine to say for a use case like this you'd pass some flag to rustc saying "make sure I can pass this to the linker". Rustc, after all, goes to great lengths to pass rlibs directly to the linker to make linking fast. I think this could work well perhaps with The gotchas that I could think of are:
Overall I think we could get away with this by carefully designing how a "stable" pass-things-to-the-linker process would work. We could make sure to solve the constraints here while also giving ourselves some leeway so rustc can still change things in the future if needed. It seems worthwhile to me to integrate into other build systems like this, and it's definitely a pain point with rlibs since inception. |
My suspicion is that the best and most holistic implementation of this kind of feature is represented by having a different crate-type alongside EDIT: Regardless of which way it goes, this feels to me like fairly major change in at least stance on what |
Just to say: thanks all for the comments. I'm delighted that this doesn't seem like a completely idiotic request. From our (user's) point of view it is equally easy if this is a We will bear in mind that this seems like a plausible way forward as we continue to think about our linking strategies. We'll go quiet for a bit, but it's possible that in a few months we will pop up here and say "hi, we'd like to implement this, here's an RFC...". Watch this space. Obviously if anyone else gets there first that's even better :) |
@adetaylor may I ask: have you found a workable solution? Or did you have to abandon Rust because of this issue? In my experimentation I have used |
That doesn't work when using liballoc as the allocator shim is only generated when compiling a crate that needs to be linked and even then doesn't get included in the |
@bbjornse We haven't abandoned Rust, but we also haven't made any real progress in this direction. I know several other teams at various organizations are using the approach of building rlibs then passing them all into the final C++ linker. This still works. It still requires nasty hacks as described in this issue description, and could break at any time. |
Interesting, thanks. Could you please explain: what exactly is the allocator shim, and why is it generated rather than part of a library? Does this relate to the Also, do you know if this is the only challenge with the object file approach? Are there other pieces which might be missing?
Thank you for your reply. This is somewhat disheartening. I'm still not abandoning hope for the |
The allocator shim is what forward the allocator calls in liballoc to either a
Correct
Honestly I am not sure.
Neither rlibs, nor |
I have been thinking about the rust compilation model lagely because of this and certain other deficiencies. No concrete actionable improvement yet though. |
Thank you for your explanation. Have I understood correctly that
I realize I don't understand why the rlib cannot include the shim. I see that it's only necessary when producing the final artifact, but I don't see how it would hurt to generate it immediately? (Unless I've misunderstood and |
To be clear, the relation between my questions and this issue is whether I'm not sure what the object file is guaranteed to include and not include, though. Crucially, is it e.g. guaranteed to not include dependencies, thus avoiding diamond dependency problems and being an improvement over the use of |
If you link to a dylib thaf already has an allocator shim, the rlib must not define an allocator shim, but the allocator shim of the dylib must be used. At the time of generating the rlib it is not yet known if it will be linked against a dylib. Only when compiling the final artifact. |
I see, thanks. Is this only when the rlib shim would use the default allocator? Would it hurt to immediately generate the shim for crates which include a If crates require a shim at all, this means they use liballoc, right? And the prebuilt libstd already includes an allocator shim forwarding to the default allocator. How come this works, when another dylib might be providing a global allocator? Could a similar technique, linking with a "default allocator shim" dylib, work in a liballoc +no-std setting? |
When you dynamically link against libstd or any other dylib that uses liballoc, |
RE: --emit=objIt seems to me that the RE: monomorphization concernsI see that @nagisa has raised concerns that monomorphization within In particular, I tried building the following things:
There would be indeed 2 copies of the monomorphized $ nm out/rel/rust_odr_test | grep crate1
00000000001f23d7 T _ZN20rust_odr_test_crate113sub_in_crate117h56a8a4a969227185E
00000000001f2422 T _ZN20rust_odr_test_crate113sub_in_crate117hf5a81c2f183f301cE But this seems fine:
(More concrete and complete example is in a Chromium-specific code review tool: see here) RE: RFCI guess the next step would be introducing an RFC (as suggested above), but before doing that it is probably desirable to
WDYT? |
One problem right now is that But aside from that, I'd agree that crate-type and emit have somewhat overlapping responsibilities (broadly, what kind of file(s) do I want?), but I'd characterize it as "crate-type sets the intrinsic properties of the output file" and "emit selects a number of auxillary outputs which can be cheaply generated along the way". But
I'm curious how this looks with I'm super interested in this effort. Currently Buck uses |
I agree that depending on I don't feel that Reasons I still think
|
Isn't this expected if |
|
Previous work has made `rust_library` operate as a `CcInfo` provider. As far as I can tell this lets the compiler directly consume an rlib rather than a formal static or shared library crate type. This is a fine experiment but is a poor recommendation in the documentation, I would suggest that until rust-lang/rust#73632 is in a better place we dont recommend people use a `rust_library` target directly where `CcInfo` is expected. I found that this failed for me in ways similar to bazelbuild#1238 even without stating a custom allocator. After fixing this for Linux + GCC this then failed in macos *and* windows. It seems that we are jumping the gun slightly and encouraging a behavior that only works with a specific configuration as the default, rather than as something to trial.
I wanted to confirm one thing. In #99429 we may want to change representation of native libraries bundled into UPD: Hmm, it looks like |
+1 for my use case being like this, I want to manage those via Bazel. |
@petrochenkov Yes, that's what I'd expect - the plan here is to make the build system deal with all dependencies uniformly, Rust and non-Rust.
No, I think this is a bug, and I think I got this the wrong way around when I last mentioned it. Right now it has the effect of ignoring the bundled native library references at link time, but they're still included. I think it would be much more useful to make this skip embedding on a crate-by-crate basis, and have a separate option to ignore any bundled libraries at link time. This would allow a more incremental approach of skipping bundled native libraries on a case-by-case basis, but still allow them for libstd (until it has fully specified dependencies). |
Hello! I've posted a pre-RFC for minimal stabilization of the |
As of #86844 (scheduled for the 1.71 release) if you are directly linking the rlibs of the standard library rather than letting rustc handle linking, you will now need to define a static named |
rust-lang/rust#73632 (comment): ``` As of #86844 (scheduled for the 1.71 release) if you are directly linking the rlibs of the standard library rather than letting rustc handle linking, you will now need to define a static named `__rust_no_alloc_shim_is_unstable` which is at least 1 byte big. In addition if you are using `#[global_allocator]`, you must stop defining `__rust_alloc`, `__rust_dealloc`, `__rust_realloc` and `__rust_alloc_zeroed` as they are now directly defined by the `#[global_allocator]` expansion rather than as part of the allocator shim. If you are using the default allocator in libstd you will need to keep defining them though. ``` We use the default allocator, so we can keep using our C++ shims as long as we define this new symbol. [email protected] Bug: 1292038 Change-Id: Ib21c48d8b656155acc0afc6941ed85f819bc9bea Cq-Include-Trybots: luci.chromium.try:android-rust-arm32-rel,android-rust-arm64-dbg,android-rust-arm64-rel,linux-rust-x64-rel,linux-rust-x64-dbg,win-rust-x64-dbg,mac-rust-x64-dbg,win-rust-x64-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4569204 Reviewed-by: Adrian Taylor <[email protected]> Commit-Queue: danakj <[email protected]> Cr-Commit-Position: refs/heads/main@{#1149904}
Is there somewhat future-proof workaround available as part of some open-source project that one could leverage maybe? There are some recipes in this discussion, but it's not clear how one could reconstruct the required "ugly hacks" to get going while we waiting for the right solution to make it to the upstream. I'm building some bindings from Rust to OCaml, everything worked great until I tried to link two such bindings libraries in one binary, which lead me here with a bunch of linker errors at hand... |
Folks using bazel and rules_rust workaround this today, so you can trace back that code or an example link command there to see the result |
The current work around is to build rust rlibs, not staticlibs, and link those as you would .a files. You must explicitly link the stdlib rlibs as well though. |
Is compilation of an empty crate as a staticlib still a decent approach to not hunt individual stdlib rlibs? Bazel rules around that are quite wordy as it seems... Other than that, one needs to parse cargo manifest, build dependency graph, build a list of rlibs required for particular crate, pass that list to the linker? |
AIUI, it's a decent approach if-and-only-if you can consolidate all your Rust bits into a single unified target. Otherwise you run the risk of pulling stuff in twice, which either bloats your binary or causes linker errors depending on how you do it. |
rust-lang/rust#73632 (comment): ``` As of #86844 (scheduled for the 1.71 release) if you are directly linking the rlibs of the standard library rather than letting rustc handle linking, you will now need to define a static named `__rust_no_alloc_shim_is_unstable` which is at least 1 byte big. In addition if you are using `#[global_allocator]`, you must stop defining `__rust_alloc`, `__rust_dealloc`, `__rust_realloc` and `__rust_alloc_zeroed` as they are now directly defined by the `#[global_allocator]` expansion rather than as part of the allocator shim. If you are using the default allocator in libstd you will need to keep defining them though. ``` We use the default allocator, so we can keep using our C++ shims as long as we define this new symbol. [email protected] Bug: 1292038 Change-Id: Ib21c48d8b656155acc0afc6941ed85f819bc9bea Cq-Include-Trybots: luci.chromium.try:android-rust-arm32-rel,android-rust-arm64-dbg,android-rust-arm64-rel,linux-rust-x64-rel,linux-rust-x64-dbg,win-rust-x64-dbg,mac-rust-x64-dbg,win-rust-x64-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4569204 Reviewed-by: Adrian Taylor <[email protected]> Commit-Queue: danakj <[email protected]> Cr-Commit-Position: refs/heads/main@{#1149904} NOKEYCHECK=True GitOrigin-RevId: 579b3dd0ea41a40da8a61ab87a8b0bc39e158998
@pcwalton did you ever move forward from pre-rfc to rfc with that? I don't think I've seen this yet (it's a long thread...) but could |
We already have the staticlib crate type for bundling everything into a single |
Summary: This diff ports the `native_unbundle_deps` feature from buck1 to buck2. The effect of this change is to make all `cxx_library` -> `rust_library` edges in the dep graph result in linking `rlib` artifacts rather than `staticlib` artifacts. The benefit being that we can then avoid issues with duplicate deps across libraries. rust-lang/rust#73632 has much more detail about this approach. Reviewed By: zertosh Differential Revision: D50523279 fbshipit-source-id: 16ee22db9140dba1e882d86aa376a03e010ef352
Summary: This diff ports the `native_unbundle_deps` feature from buck1 to buck2. The effect of this change is to make all `cxx_library` -> `rust_library` edges in the dep graph result in linking `rlib` artifacts rather than `staticlib` artifacts. The benefit being that we can then avoid issues with duplicate deps across libraries. rust-lang/rust#73632 has much more detail about this approach. Reviewed By: zertosh Differential Revision: D50523279 fbshipit-source-id: 16ee22db9140dba1e882d86aa376a03e010ef352
Summary: This diff ports the `native_unbundle_deps` feature from buck1 to buck2. The effect of this change is to make all `cxx_library` -> `rust_library` edges in the dep graph result in linking `rlib` artifacts rather than `staticlib` artifacts. The benefit being that we can then avoid issues with duplicate deps across libraries. rust-lang/rust#73632 has much more detail about this approach. Reviewed By: zertosh Differential Revision: D50523279 fbshipit-source-id: 16ee22db9140dba1e882d86aa376a03e010ef352
This aligns with ChromeOS Rust. Also update rules_rust to latest release to pick up bazelbuild/rules_rust@aa4b3a8 Which includes bazelbuild/rules_rust@4bd44d0 for __rust_no_alloc_shim_is_unstable which is required since Rust 1.71.0 as described in rust-lang/rust#73632 (comment) BUG=None TEST=cop Change-Id: Ib579ee725f0adc233021fb07ba55046930f71f9c Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/5095413 Tested-by: [email protected] <[email protected]> Reviewed-by: Chih-Yang Hsia <[email protected]> Commit-Queue: Li-Yu Yu <[email protected]>
I'm working on a major existing C++ project which hopes to dip its toes into the Rusty waters. We:
We can't:
rustc
for final linking. C++ is the boss in our codebase; we're not ready to make the commitment to put Rust in charge of our final linking.staticlib
for each of our Rust components. This works if we're using Rust in only one place. For any binary containing several Rust components, there would be binary bloat and potentially violations of the one-definition-rule, by duplication of the Rust stdlib and any diamond dependencies.staticlib
containing all our Rust components, then link that into every binary. That monster static library would depend on many C++ symbols, which wouldn't be present in some circumstances.We can either:
staticlib
for each of our output binaries, usingrustc
and an auto-generated.rs
file containing lots ofextern crate
statements. Or,rlib
for each Rust component directly into the final C++ linking procedure.The first approach is officially supported, but is hard because:
staticlib
as part of our C++ tool invocations. This is awkward in our build system. Our C++ targets don't keep track of Rust compiler flags (--target
, etc.) and in general it just feels weird to be doing Rust stuff in C++ targets.rustc
to make astaticlib
for every single one of our C++ link targets. For most of our targets (especially unit test targets) there will be norlibs
in their dependency tree, so it will be a no-op. But the presence of this wrapper script will make Rust adoption appear intrusive, and of course will have some small actual performance cost.The second approach is not officially supported. An
rlib
is an internal implementation format within Rust, and its only client isrustc
. It is naughty to pass them directly into our own linker command line.But it does, currently, work. It makes our build process much simpler and makes use of Rust less disruptive.
Because external toolchains are not expected to consume
rlib
s, some magic is required:rlib
s, which would be easy apart from the fact they contain the symbol metadata hash in their names.__rust_alloc
to__rdl_alloc
etc.But obviously the bigger concern is that this is not a supported model, and Rust is free to break the
rlib
format at any moment.Is there any appetite for making this a supported model for those with mixed C/C++/Rust codebases?
I'm assuming the answer may be 'no' because it would tie Rust's hands for future
rlib
format changes. But just in case: how's about the following steps?rustc
as the final linker; orstaticlib
orcdylib
then pass that to your existing final linker(I think this would be worth explicitly explaining anyway, so unless anyone objects, I may raise a PR)
rustc --print stdrlibs
(or similar) which will output the names of all the standard library rlibs (not just their directory, which is already possible withtarget-libdir
)rustc
option which generates arust-dynamic-symbols.o
file (or similar) containing the codegen which is otherwise done byrustc
at final link-time (e.g. symbols to call__rdl_alloc
from__rust_alloc
, etc.)rust-dynamic-symbols.o
and everything output byrustc --print stdrlibs
)A few related issues:
rustc
#64191 wants to split the compile and link phases of rustc. This discussion has spawned from there.-Wl,--start-group
,-Wl,--end-group
on the linker line. (Per Add support for splitting linker invocation to a second execution ofrustc
#64191 (comment))staticlib
-per-C++-target model happen to be magnified by rlibs retain reference to proc-macro dependencies - possibly unnecessary? #73047@japaric @alexcrichton @retep998 @dtolnay I believe this may be the sort of thing you may wish to comment upon! I'm sure you'll come up with reasons why this is even harder than I already think. Thanks very much in advance.
The text was updated successfully, but these errors were encountered: