Skip to content

Proper way to statically link rust_binary target with a static non-PIC cc_library #118

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

Open
vitalyd opened this issue Aug 10, 2018 · 18 comments
Assignees

Comments

@vitalyd
Copy link
Contributor

vitalyd commented Aug 10, 2018

This may be more of a bazel question but since it's in the context of rust, I figured the audience here is as good as any 😄 .

Suppose we have the following targets:

  1. cc_library building some static lib
  2. rust_library FFI wrapper over the above
  3. rust_binary that uses the rust_library

Pretty standard and simple setup for rust using c/c++ code.

In a dbg or fastbuild setup, it seems the above just works. However, when I recently tried this with an opt build using a custom cc toolchain, the code failed to link. Quick note on the custom toolchain - it's very close to the configuration/setup of bazel's builtin crosstool but uses vendored gcc (and related tools).

The link failure was around invalid relocations. This is because rustc, by default, wants to build position-independent (PIE) executables. To do that, the c/c++ static libs need to be built with -fPIC - building the cc_library as PIC fixes the issue. However, this requires modifying a target whose other downstream users may not want to be PIC'd (and PIC can have non-zero perf cost, albeit low from what I gather).

The other fix that I came across is changing the rust_binary target to specify a codegen option to rustc: --relocation-model=static. This builds a non-PIE binary, and doesn't require -fPIC on the dependent cc libs.

This feels ... wrong, however. Either I'd need to modify all rust binary targets to select static relocation, or I'd need to modify the c/c++ static libs to be PIC. I also don't know why there's a difference between dbg/fastbuild and opt builds.

Does anyone know if there's a better way to control linkage and relocation model? Ideally (I think), a rust_binary would select the linkage it wants (static or dynamic) and the relocation model (static or dynamic), and this choice would flow down to dependent libs, where bazel would build them as either static or dynamic libs and also select -fPIC or not.

Curious to hear thoughts/suggestions.

cc @mfarrugi as he's interested in this as well, I believe.

@vitalyd
Copy link
Contributor Author

vitalyd commented Dec 17, 2018

Curious if anyone has thoughts on this topic? The current annoyance, from my standpoint, is the need to specify --relocation-model=static for rust_binary targets so they can link property against cc_library dependencies in -c opt mode.

I suppose one way around this is to configure the rust Bazel toolchain to automatically add this flag to a rust_binary target if we can determine that there are cc_library dependencies being built without PIC code. This seems unsatisfying, and hacky.

As mentioned in my original post, it seems one would want to convey link options "top-down", with a rust_binary target informing cc_library dependencies that it wants them to be compiled and linked with PIC code.

Or maybe something else entirely? How are others dealing with this issue? I presume this must be somewhat common for any Rust binaries that FFI to Bazel-built C libs.

@vitalyd
Copy link
Contributor Author

vitalyd commented Dec 18, 2018

Just a quick link drop - this appears to be the code in bazel cpp rules that determines PIC'ness based on (among other things) the compilation mode.

@vitalyd
Copy link
Contributor Author

vitalyd commented Dec 18, 2018

@acmcarther, curious - how do you guys handle this at Google (assuming you're using rules_rust there)?

@acmcarther
Copy link
Collaborator

My team at Google doesn't use Rust[1], so I don't have an answer on this item. I'm also not well informed on nuances of linking (within Rust or elsewhere), so I'm afraid you're on the cutting edge here.

[1] Some teams do, Fuchsia happens to be public facing, and their rust repos may or may not be enlightening. They don't use Bazel though.

@hlopko hlopko self-assigned this Jan 9, 2019
@mfarrugi
Copy link
Collaborator

mfarrugi commented Apr 2, 2019

It looks like this was recently exposed in the cc starlark api, and could be as simple as reordering these two lines in our rules:

library_to_link.static_library or
library_to_link.pic_static_library or

Did you have any particular thoughts @hlopko?

@kalcutter
Copy link
Contributor

@mfarrugi This issue is also causing us some pain. I tried the solution you mentioned, but I get this error:

io_bazel_rules_rust/rust/private/rustc.bzl:417:5:` assignment length mismatch: left-hand side has length 2, but right-hand side evaluates to value of length 3

@kalcutter
Copy link
Contributor

We also tried to work around this issue with bazel option --force_pic but then we get:

/usr/lib/gcc/x86_64-redhat-linux/9/../../../../lib64/Scrt1.o:function _start: error: undefined reference to 'main'
          collect2: error: ld returned 1 exit status```

@mfarrugi
Copy link
Collaborator

@kalcutter I have no insight on either error. I also am not very confident in the snippet I posted earlier, and don't have access to the complicated codebase I used to work with.

filmil added a commit to filmil/kythe that referenced this issue Jul 29, 2020
Rust binaries are compiled with -fPIC by default, and this fights with
bazel's default of -fno-pic when using `-c opt` mode.  This has caused
the compilation to fail.

After some running around with hair on fire, I found
bazelbuild/rules_rust#118, which seems to
work around the issue.

I think in the long run, rules_rust should handle this automagically for
the users.
@filmil
Copy link

filmil commented Jul 29, 2020

Hi folks. I just randomly hit this issue while working on Kythe.

First off, I am grateful for the workaround (rustc_flags="-Crelocation-model-static).

It seems to me that rust_rules should be able to know automatically what to do based on the current compilation settings. Not sure if this is some sort of a default, but Kythe's build produces both PIC and non-PIC libraries during compilation. And it seems that rust_rules should have enough info at that point to select the correct deps to link. This is a non-expertly opinion, stated with a big user hat on.

shahms pushed a commit to kythe/kythe that referenced this issue Jul 29, 2020
* feat(rust): Adds a C API for the kzip writer

This will allow me to reuse the C++ kzip writing code for
the rust indexer instead of redoing that work in rust.

Issue #4606

* Renames the 'status' library

Somewhat surprisingly -- if there are two libraries in the global namespace
that have the same name, bazel does not disambiguate them, and an attempt
to link them will fail.  Not sure why, but that's what happens.

* fixup: handled review comments

Simplified and cleared up issues raised during
code review.

* Works around the -fPIC issue for rust

Rust binaries are compiled with -fPIC by default, and this fights with
bazel's default of -fno-pic when using `-c opt` mode.  This has caused
the compilation to fail.

After some running around with hair on fire, I found
bazelbuild/rules_rust#118, which seems to
work around the issue.

I think in the long run, rules_rust should handle this automagically for
the users.

* Document no NUL byte at the end of results
@hlopko
Copy link
Member

hlopko commented Aug 10, 2021

I'm looking into this and I cannot reproduce. Are you compiling your C++ as no-PIC and no-PIE (my testing toolchain uses PIE for C++)?

@kalcutter
Copy link
Contributor

We build our C++ libraries with linkstatic = True and have this issue. Our workaround is adding build --copt -fPIC to .bazelrc.

@hlopko Are you testing with -c opt? I think Bazel builds everything with -fPIC otherwise.

@hlopko
Copy link
Member

hlopko commented Sep 8, 2021

Yup I was using -c opt but my toolchain uses -fPIE in opt. It seems yours does not, and that is a problem for rustc when it drives linking (for executables on linux it defaults to PIE, but your C++ objects would be NOPIC).

You may want to change your fix to -fPIE if you're building executables. It should be a hair faster, and still working. But the fix is reasonable, and I don't have a better solution just yet.

@hlopko
Copy link
Member

hlopko commented Sep 8, 2021

One related thing I may pursue soonish is rust-lang/rust#87934 - that will not help with NOPIC and NOPIE objects, but it will make executables with PIE a bit faster.

@bsilver8192
Copy link
Contributor

I'm running into this too. I have experience with C++ and Bazel toolchains, and I'm interested in helping with this. Before I dive in, any updates @hlopko?

To hopefully help others find this from the error messages, here's the symptoms I saw:

  = note: ld.lld: error: can't create dynamic relocation R_X86_64_32 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
          >>> defined in bazel-out/k8-opt/bin/external/raze__cxx__1_0_64/libcxx_cc.a(cxx.o)
          >>> referenced by iostream:74 (external/amd64_debian_sysroot//usr/include/c++/10/iostream:74)
          >>>               cxx.o:(_GLOBAL__sub_I_cxx.cc) in archive bazel-out/k8-opt/bin/external/raze__cxx__1_0_64/libcxx_cc.a
          
          ld.lld: error: can't create dynamic relocation R_X86_64_32 against local symbol in readonly segment; recompile object files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in the output
          >>> defined in bazel-out/k8-opt/bin/external/raze__cxx__1_0_64/libcxx_cc.a(cxx.o)
          >>> referenced by cxx.cc:0 (external/raze__cxx__1_0_64/src/cxx.cc:0)
          >>>               cxx.o:(_GLOBAL__sub_I_cxx.cc) in archive bazel-out/k8-opt/bin/external/raze__cxx__1_0_64/libcxx_cc.a
          
          ld.lld: error: cannot preempt symbol: __dso_handle
          >>> defined in external/amd64_debian_sysroot/lib/gcc/x86_64-linux-gnu/10/crtbeginS.o
          >>> referenced by cxx.cc:0 (external/raze__cxx__1_0_64/src/cxx.cc:0)
          >>>               cxx.o:(_GLOBAL__sub_I_cxx.cc) in archive bazel-out/k8-opt/bin/external/raze__cxx__1_0_64/libcxx_cc.a
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

@hlopko
Copy link
Member

hlopko commented Feb 16, 2022

I just merged a failing test case for shared libraries (#1141), and #1134 is where we're attempting to fix it.

If the issue you're seeing is with rust_binary we'll need to investigate more. For example, are you building the C++ part with -fPIE?

@bsilver8192
Copy link
Contributor

No, the C++ part is not built with -fPIE.

My whole toolchain is at https://github.com/frc971/971-Robot-Code/tree/master/third_party/bazel-toolchain (fork of https://github.com/grailbio/bazel-toolchain). It's using unix_cc_toolchain_config.bzl (forked version that just adds a few features), which uses -fPIC for object files when enabled. I can push a branch with the failing test case if you want.

@rdelfin
Copy link

rdelfin commented Jun 14, 2022

@bsilver8192 did you get a chance to push those changes? Looks like this is still very much broken :(
Interestingly, I'm also using the same toolchain, so I wonder if it's something to do with that

@bsilver8192
Copy link
Contributor

It's actually merged now. If you comment out https://github.com/frc971/971-Robot-Code/blob/8e815fbb728ddc4df9714f72beb925cb8c93159d/third_party/cargo_raze/cargo_raze.patch#L194 and then bazel build -c opt @cargo_raze//:raze it will fail.

So far I don't have any other rust_binary targets outside of third_party which are complex enough to run into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants