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

Wrong cc toolchain solib path in rpath for external repos with sibling layout #16956

Closed
yuzhy8701 opened this issue Dec 8, 2022 · 2 comments
Closed
Labels
help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug

Comments

@yuzhy8701
Copy link
Contributor

Description of the bug:

We are setting up bazel for Android Emulator. The cc toolchain we've setup:

  1. Uses a hermetic clang compiler.
  2. Enables the static_link_cpp_runtimes feature and feeds libc++ to the (dynamic | static)_runtime_lib attributes of the cc_toolchain rule.

And due to the layout of the source tree (a top-level "external" directory), we have to pass --experimental_sibling_repository_layout to solve the conflict with external repositories.

With this setup, we find that we are unable to correctly build dynamically linked binaries in external repositories. The rpath for dynamic_runtime_lib (libc++) is set incorrectly. Since cc_test is always linked dynamically, this is prohibiting us to run any cc_test in external repos.


e.g., with the following setup:

  • The custom toolchain is located at @//toolchains/cc:linux_clang_x64
  • The binary target is @zlib//:zlib_test
  • CPU is k8

The following rpaths are set by bazel:

❯ objdump -p zlib_test
...

Dynamic Section:
  RUNPATH              $ORIGIN/_solib_k8/
                       :$ORIGIN/zlib_test.runfiles/zlib/_solib_k8/
                       :$ORIGIN/_solib__toolchains_Scc_Clinux_Uclang_Ux64/
                       :$ORIGIN/zlib_test.runfiles/zlib/_solib__toolchains_Scc_Clinux_Uclang_Ux64/
  NEEDED               liblibzlib.so
  NEEDED               libc++.so.1
...

However, what bazel generates in the execroot file tree is:

❯ tree ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/zlib/k8-fastbuild/bin
bin
├── libzlib.a
├── libzlib.so
├── libzlib.so-2.params
├── _solib_k8
│   └── liblibzlib.so -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/zlib/k8-fastbuild/bin/libzlib.so
├── zlib_test
├── zlib_test-2.params
├── zlib_test.runfiles
│   ├── __main__
│   │   ├── external
│   │   │   └── zlib
│   │   │       ├── _solib_k8
│   │   │       │   └── liblibzlib.so -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/zlib/k8-fastbuild/bin/_solib_k8/liblibzlib.so
│   │   │       └── zlib_test -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/zlib/k8-fastbuild/bin/zlib_test
│   │   └── _solib__toolchains_Scc_Clinux_Uclang_Ux64
│   │       ├── libc++.so -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/k8-fastbuild/bin/_solib__toolchains_Scc_Clinux_Uclang_Ux64/libc++.so
│   │       └── libc++.so.1 -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/k8-fastbuild/bin/_solib__toolchains_Scc_Clinux_Uclang_Ux64/libc++.so.1
│   └── zlib
│       ├── _solib_k8
│       │   └── liblibzlib.so -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/zlib/k8-fastbuild/bin/_solib_k8/liblibzlib.so
│       └── zlib_test -> ~/.cache/bazel/_bazel_<username>/<md5>/execroot/__main__/bazel-out/zlib/k8-fastbuild/bin/zlib_test
...

Form the execroot tree, it seems that the correct rpath to find libc++ should be either:

  1. $ORIGIN/../../../k8-fastbuild/bin/_solib__toolchains_Scc_Clinux_Uclang_Ux64, or
  2. $ORIGIN/zlib_test.runfiles/__main__/_solib__toolchains_Scc_Clinux_Uclang_Ux64

However bazel generates:

  1. $ORIGIN/_solib__toolchains_Scc_Clinux_Uclang_Ux64
  2. $ORIGIN/zlib_test.runfiles/zlib/_solib__toolchains_Scc_Clinux_Uclang_Ux64

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

  1. Create a toolchain config and a toolchain that feeds libs into the dynamic_runtime_lib attribute, and enable feature static_link_cpp_runtimes.
  2. Define an external repo and create a cc binary target in it.
  3. Turn on --experimental_sibling_repository_layout flag and build the target
  4. Inspect the created binary or the linking command to check rpath.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 7.0.0-pre.20221111.3

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No.

Any other information, logs, or outputs that you want to share?

No response

@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) help wanted Someone outside the Bazel team could own this and removed untriaged labels Jan 23, 2023
copybara-service bot pushed a commit that referenced this issue Feb 15, 2023
This should fix issue #16956

Includes the following changes:

- Refactor logics to find runtime library search directories
- Correctly compute RPATHs for toolchain solib.

Closes #17276.

PiperOrigin-RevId: 509815187
Change-Id: I7f2a2412aea6430fa712dd2836e18f9536757753
@yuzhy8701
Copy link
Contributor Author

Fixed by commit 773d232

ShreeM01 added a commit that referenced this issue Feb 17, 2023
This should fix issue #16956

Includes the following changes:

- Refactor logics to find runtime library search directories
- Correctly compute RPATHs for toolchain solib.

Closes #17276.

PiperOrigin-RevId: 509815187
Change-Id: I7f2a2412aea6430fa712dd2836e18f9536757753
(cherry picked from commit 773d232)

Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
jrguzman-ms pushed a commit to msft-mirror-aosp/platform.build.bazel that referenced this issue Mar 15, 2023
This is the first release with
bazelbuild/bazel#16956 fixed.

Test: n/a
Bug: n/a
Change-Id: Iadb7c46555b21d94e6297d366043f6b1b82340d7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests

4 participants
@fmeum @yuzhy8701 @oquenchil and others