rules_rust icon indicating copy to clipboard operation
rules_rust copied to clipboard

FR: support for linking via cc_common.link

Open krasimirgg opened this issue 1 year ago • 2 comments

We would like to add support to rules_rust for linking rust binaries via the cc_common.link Bazel mechanism. This feature would allow for better cross-language interoperability and make it easier to provide cross-language integration of features like LTO and sanitizer support by adapting the rust compilation pipeline to produce actions and artifacts that are more uniform with other languages' rules implementation from Bazel point of view.

This feature should co-exist with the existing rules_rust mechanism that uses rustc as the linker driver, since there are use cases where rustc as the driver makes sense. For projects that are mostly written in rust using rustc-as-a-driver is simpler to set up and more flexible with respect to features such as custom rust allocator support. The use of cc_common.link would require a fully configured C++ toolchain and custom allocator library for example.

Initially we'd like to experiment with this and focus on rust_binary and rust_test, possibly extending the support to other targets that require linking in the future.

As a rough design, we'd like to be able to control the default linking mechanism on a rust_toolchain level, and additionally be able to selectively override the behavior on a per-target basis until the feature is complete. We'd add

  • a new boolean build setting --//rules_rust/rust/settings:experimental_use_cc_common_link, defaulting to false,
  • a new rust_toolchain label attribute experimental_use_cc_common_link, defaulting to the build setting above.
  • a new temporary int rust_{binary|test|shared_library}.experimental_use_cc_common_link attribute with possible values [-1, 0, 1] to rust_binary and other crate types that require linking, defaulting to -1. This is to support gradually moving targets t/from using the new feature; after the feature is stable, we'll remove this attribute.

Given rust rust_{binary|...}(name = bin), and a rust_toolchain, common_cc.link is enabled if:

  • bin.experimental_use_cc_common_link is 1, or
  • bin.experimental_use_cc_common_link is -1 and rust_toolchain.experimental_use_cc_common_link points to a label whose value is true.

Otherwise common_cc.link is disabled.

When enabled, instead of emitting one rustc_compile_action that does both compilation and linking using rustc, we will emit 2 actions:

  • A rustc one with –emit=obj to produce an .o file
  • A cc_common.link one to produce the final binary (or appropriate binary artefacts for other crates that require linking).

We'll focus on rust_binary first, then rust_test, then possibly rust_shared_library. After linking with cc_common.link has been stabilized, we'll drop the experimental_ suffix from the build setting and the rust_toolchain attribute and drop the rust_{binary|...} attribute.

krasimirgg avatar Jul 28 '22 12:07 krasimirgg

Will / should this have any impact on if you're linking rust_library targets into cc_binary targets instead of top level rust_* targets as you mention here?

keith avatar Aug 01 '22 16:08 keith

Will / should this have any impact on if you're linking rust_library targets into cc_binary targets instead of top level rust_* targets as you mention here?

I think the impact will be minimal: there could be some bug fixes that apply to both, but on the other hand the cc_common.link feature relies largely on the same abstractions as the ones used to support cc_binary, so a lot of functionality will be implicitly shared between them.

krasimirgg avatar Aug 02 '22 09:08 krasimirgg

I've been using the experimental support, and for most cases it works great! I have encountered one limitation though: it looks like cargo:rustc-link-lib (etc) directives emitted from a cargo_build_script get dropped. Not sure if that belongs here or as a separate bug.

bsilver8192 avatar Sep 28 '22 02:09 bsilver8192

looks like cargo:rustc-link-lib (etc) directives emitted from a cargo_build_script get dropped

Ah, nice catch! Something to fix definitely. @bsilver8192 I feel your comment on this issue is sufficient to capture and track this for now.

krasimirgg avatar Oct 05 '22 08:10 krasimirgg

If we knew what runtimes the C++ linker linked, could we invert this pattern and pass those as explicit linker args to rustc? We do this in the Fuchsia GN build, for instance:

  # -static-libstdc++ and -static-libgcc options are handled by the C++
  # compiler driver (that is clang), whereas these link-args get passed
  # straight to lld by rustc and need to be expanded manually.
  rustflags = [ "-Clink-args=--push-state -Bstatic -lc++ -Bdynamic -lm --pop-state -l:libunwind.a" ]

This would allow us to recover the flexibility of using custom allocators. But I don't know what providers/fields should carry the information about depending on the C++ runtime. (The simple existence of CcInfo would not be very useful, since we provide that on Rust targets too!) Perhaps this requires a custom aspect.

For knowing what those link args should look like in the first place, perhaps some new provider could be added for the user to attach to their C++ toolchain definition (somehow).

tmandry avatar Dec 06 '22 22:12 tmandry