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

Add opt-out from merging cc_lib object files in bindgen #2959

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

hlopko
Copy link
Member

@hlopko hlopko commented Oct 24, 2024

This is a ~rollback of
#2590, as it turns out the current behavior is incomatible with
https://bazel.build/docs/user-manual#dynamic-mode (which is very hard to use in the OSS because of
bazelbuild/bazel#4135).

In short, the dynamic mode works as follows:

  • Each cc_library produces object files that are used to produce
    • a static library (often not materialized but objects get passed as --start-lib/--end-lib to the linker),
    • a shared library (only containing the objects, not transitive dependencies),
    • and an interface library (produced from .so by "stripping function bodies").
  • When linking binaries, by default we use the static linking.
  • When linking tests, by default we use interface libraries for linking, and shared libraries for test execution.

The motivation for this is to avoid lengthy linking actions when all that has changed between previous build is the method body (therefore Bazel produces the same interface library, so we don't need to reexecute linking action).

We do not support dynamic mode in rules_rust yet.

The current bindgen behavior works by taking object from cc_lib, and merging them into the rlib output. When dynamic_mode is on, we now have a linking action that has the rlib early on the command line, and then interface libraries of dependencies of the cc_lib, and then the interface library of the cc_lib. For reasons, lld prefers statically defined symbols, it sees that the rlib defines the same symbols as cc_lib but statically, and errors out with backward reference detected, even though cc_lib is positioned correctly.

The fix for us is to not merge objects.

This is a ~rollback of
bazelbuild#2590, as it turns out the
current behavior is incomatible with
https://bazel.build/docs/user-manual#dynamic-mode (which is very hard to
use in the OSS because of
bazelbuild/bazel#4135).

In short, the dynamic mode works as follows:

* Each cc_library produces object files that are used to produce
    * a static library (often not materialized but objects get passed as
      `--start-lib`/`--end-lib` to the linker),
    * a shared library (only
      containing the objects, not transitive dependencies),
    * and an interface library (produced from .so by "stripping function
      bodies").
* When linking binaries, by default we use the static linking.
* When linking tests, by default we use interface libraries for linking,
  and shared libraries for test execution.

The motivation for this is to avoid lengthy linking actions when all
that has changed between previous build is the method body (therefore
Bazel produces the same interface library, so we don't need to reexecute
linking action).

We do not support dynamic mode in rules_rust yet.

The current bindgen behavior works by taking object from `cc_lib`, and
merging them into the `rlib` output. When dynamic_mode is on, we now
have a linking action that has the rlib early on the command line, and then
interface libraries of dependencies of the `cc_lib`, and then the interface library of the `cc_lib`. For reasons,
lld prefers statically defined symbols, it sees that the `rlib` defines the same symbols as `cc_lib` but statically, and errors
out with `backward reference detected`, even though `cc_lib` is
positioned correctly.

The fix for us is to not merge objects.
@krasimirgg krasimirgg added this pull request to the merge queue Nov 8, 2024
Merged via the queue into bazelbuild:main with commit cf6e76e Nov 8, 2024
4 checks passed
@hlopko hlopko deleted the bindgen_merging branch November 8, 2024 11:20
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

Successfully merging this pull request may close these issues.

2 participants