Implement interface shared objects to avoid re-linking

[pconrad-sx]

Description:

As discussed on this thread, any changes to c++ sources or headers will trigger relinking of dynamically linked binaries. Bazel supports the concept of "interface shared objects", (*.ifso files), to separate the implementation from the interface, thus limiting the need to re-link for source-only changes. However, Google's current implementation of a tool that can create ifso files is not public.

This ticket is a request for a public implementation of an ifso tool. This is a non-trivial performance boost for users of dynamic linking. One compelling use case is for dynamically-linked unit test suites, where many linking tasks might be avoided. ​

If possible, provide a minimal example to reproduce the problem:

To demonstrate this behavior, we can use the cpp-tutorial/stage2 hello-world application in the bazel tutorials: https://github.com/bazelbuild/examples/
Make the binary dynamic by adding "linkstatic=0" to the cc_binary rule. Change the string in hello-greet.cc, and the binary will re-link upon building //main:hello-world.

Subcommands show:

SUBCOMMAND: # //main:hello-greet [action 'Compiling main/hello-greet.cc'] 
SUBCOMMAND: # //main:hello-greet [action 'Linking main/libhello-greet.so'] 
SUBCOMMAND: # //main:hello-world [action 'Linking main/hello-world'] 

Explanations of rebuilds show:

Executing action 'BazelWorkspaceStatusAction stable-status.txt': unconditional execution is requested. 
Executing action 'Compiling main/hello-greet.cc': One of the files has changed. 
Executing action 'Linking main/libhello-greet.so': One of the files has changed. 
Executing action 'SolibSymlink _solib_k8/libmain_Slibhello-greet.so': One of the files has changed. 
Executing action 'Linking main/hello-world': One of the files has changed. 

Since hello-greet.h wasn't changed, I think chain should have been able to stop after re-linking the shared library.

• Bazel version (output of bazel info release):
  0.7.0

Have you found anything relevant by searching the web?

There do not appear to be other issues on this topic.

[meteorcloudy]

/cc @mhlopko

[nicolov]

Clang 9 now includes a clang-ifso tool to create such stubs. Would it be easy to use it in Bazel?

https://www.phoronix.com/scan.php?page=news_item&px=Clang-Interface-Stubs

[hlopko]

Oh I thought clang-ifso didn't make it into clang, and that they're still waiting for tapi. I'll look into it when I find a free moment. But once the tool is there, hooking it into Bazel C++ toolchain will be fairly straightforward.

[nicolov]

I've set it up in Bazel (was fairly straightforward) but the problem is that the clang tool produces an .ifso file for each .o file, and not for each .so. What do you think is the best way to reconcile the two approaches?

[hlopko]

Oh cool! How did you wire it into the Bazel C++ toolchain? Maybe you're running the tool in the compilation action, and we only need it for the shared library linking action? Could you share your playground repository?

[nicolov]

Sadly, the feature is not really useful right now. First of all, the interface stubs implementation (in Clang 9, haven't checked trunk yet) crashes even with the simplest C++ example:

clang++ -fvisibility=hidden -emit-interface-stubs -interface-stub-version=experimental-tapi-elf-v1 main.cc

where main.cc is just:

#include <iostream>

int main() {
  std::cout << "Hello, world!\n";
}

Which basically prevented any further progress on this.

Assuming we can fix that, how do you suggest we support this in Bazel? It's my understanding that the Google-internal tool generates the stub from the .so, so we have one stub for each cc_library. On the other hand, AFAICT, the clang approach runs as a frontend action, and therefore would generate a stub for each translation unit/object file. We would need a separate step to merge those stubs.

On the Bazel side, I simply added a feature with the right flags. I noticed I also had to declare support for has_configured_linker_path so that Bazel wouldn't use the linker wrapper script when ifsos are enabled.

[adrianimboden]

is there any news on this? Maybe in the last three years, clang did get proper support.

I would also invest some time to test that, but my knowledge about bazel toolchains is pretty limited. If you could maybe share what you already have so I can try it out some times?

[kjteske]

We're extremely interested in this too. We use gcc and link with LTO , which is extremely slow: 60+ seconds for our biggest binary. It really hurts us to re-link because of an implementation change in a dependent library that doesn't change the library's public interface.

To avoid this, some developers turn off LTO for dev builds, but this is becoming less and less feasible. We turn on LTO not just for speed improvements, but because we are very sensitive to binary size, and LTO shrinks binaries significantly. On some of our platforms, LTO is no longer "nice to have" - the binaries are too large without LTO and can't be deployed.

@hlopko - we'd love if Google could open source their tool to generate .ifso libraries, or even provide a rough sketch of what goes into the tool - the savings are significant enough to us that we'd be open to writing a tool from scratch ourselves (and hopefully in an open-source way).

[kjteske]

@oquenchil - I found Marcel on Slack and he pointed me to you, do you know if this could be open-sourced or share a general outline of how this tool would work?

[oquenchil]

Sorry we don't own the tool. https://github.com/ianlancetaylor may be able to help here.

[kjteske]

I emailed Ian (couldn't seem to @ him here): he doesn't work on this, but his understanding (could be wrong) is that the tool is https://llvm.org/docs/CommandGuide/llvm-ifs.html, and suggested talking to LLVM developers, especially people who work on lld, about this, but wasn't sure who those people are.

If llvm-ifs is the right tool, that would be great - we'd still like to get that confirmed by someone at Google who knows for sure about this, and get guidance on hooking this up to Bazel.

[ulfjack]

@hlopko or @oquenchil, would you mind looking in source control who wrote the tool that is currently being used by Bazel inside of Google, and possibly let them know that there's outside interest? IIRC Paul P. wrote the original tool, but I might be wrong about that.

[ulfjack]

Ping?

[EmployedRussian]

I just read the description of llvm-ifs, and it appears to be doing the exact same thing that Google's internal implementation does (and more).

In particular, it can take ELF .so as input and produce ELF .stub.so as output. This output .stub.so changes only when there is an ABI change (and thus avoids unnecessary re-linking).

Is there a reason llvm-ifs isn't the answer here?

[kjteske]

I had just emailed the author of llvm-ifs - he confirmed llvm-ifs is not what Bazel uses (Bazel's tool was created before llvm-ifs), but he does believe llvm-ifs could be used for this. He isn't very familiar with Bazel and doesn't know all the details of how to wire it up.

The documentation is a bit lacking, but with some time and looking at the Bazel source (e.g. hooks like https://github.com/bazelbuild/bazel/blob/1841676f2950a29c14f7b6cb0657e1490378e9de/tools/cpp/link_dynamic_library.sh) we might have all we need to get this working. We aren't actively working on this (still finishing our Bazel migration), but this may come up sooner rather than later on our roadmap to improve performance.

[MaskRay]

Using llvm-ifs for this purpose looks good to me. We can use: llvm-ifs --input-format=ELF a.so --output-elf=a.ifso.

llvm-ifs does not handle symbol versioning. We probably need to implement the feature and test this with a large code base to sort out all possible issues.

[fmeum]

I submitted #21194, which adds llvm-ifs based support to the default Linux C++ toolchain. If you get a chance to try it out, let me know how it went.

@MaskRay Do you know whether there are any efforts to add versioning support to llvm-ifs? If not, could we recognize shared libraries relying on it without extra tooling, e.g. by checking for a section name with grep?

[kjteske]

@fmeum - thank you!

We started down the llvm-ifs path internally, but hit some roadblocks and haven't finished it off yet. We have some llvm-ifs patches to handle symbol versioning and preserve ELF e_flags (which contains important EABI info for ARM binaries). We're talking about getting these upstreamed now, or at least posting what we have as a starting point. Hoping we can get that up soon with this motivating us.

It might be a bit longer before we have time to test @fmeum 's PR for our use case, but we're super excited to see work on this!

[fmeum]

@kjteske Sounds great, please keep us updated about these efforts here.

As the ARM case probably isn't relevant for the auto-configured toolchain shipped with Bazel, symbol versioning seems like the only issue users may experience with the linked change. Since it can always be disabled with --nointerface_shared_objects or even on a per-target level via features, this doesn't look like a blocker either. I can look into simple detection mechanisms if it becomes a problem.

[EmployedRussian]

The idea of using .ifso is that you get an identical binary whether you use .ifso or not.

If symbol versions are present in .so and not in the .ifso, you may not get an identical binary, and the ifso-linked binary can be broken in very subtle ways, so I'd be very worried about introducing such bugs into default setup.

This of course becomes moot once llvm-ifs properly supports symbol versions.

P.S. The symbol version definition info is contained in the .gnu.version_d sections (use readelf -WS foo.so). If this section is not present, using .ifso for the corresponding library is safe.

[fmeum]

Can version information for undefined symbols affect the result of linking? If so, then skipping .ifso generation via llvm-ifs for shared objects with versioned symbols will disable this feature for anything linking against glibc. This would probably make it moot as part of the default toolchain and we would really have to add support to llvm-ifs instead.

I added a check for any versioned symbols, but that results in a trivial .ifso being generated for the most trivial test case due to the presence of e.g. __cxa_finalize@GLIBC_2.2.5:
https://github.com/bazelbuild/bazel/pull/21194/files#diff-9f649e4c747cfe1b0299609b063fc4f45da10b284fa5bd070cc21cbf5afaf72fR10-R13

[EmployedRussian]

Can version information for undefined symbols affect the result of linking?

I can't think of a reason it would, but I am only 95% certain of this answer.

[fmeum]

I can't think of a reason it would, but I am only 95% certain of this answer.

Unfortunately it looks like this does indeed matter. The following Bazel integration test, in which a library with versioned symbols is used through an intermediate library, results in a cc_test printing an incorrect result after updating only the version script of the library even when we apply llvm-ifs only to libraries with versioned undefined symbols:

function test_ifso_undefined_versioned_symbol() {
  type -P llvm-ifs-14 || return 0

  mkdir pkg
  cat > pkg/BUILD <<'EOF'
cc_library(
  name = "lib",
  srcs = ["lib.c"],
  hdrs = ["lib.h"],
  additional_linker_inputs = ["version_script.lds"],
  linkopts = ["-Wl,--version-script,$(execpath version_script.lds)"],
)
cc_library(
  name = "intermediate",
  srcs = ["intermediate.c"],
  hdrs = ["intermediate.h"],
  deps = [":lib"],
)
cc_test(
  name = "test",
  srcs = ["test.c"],
  deps = [":intermediate"],
)
EOF
  cat > pkg/version_script.lds <<'EOF'
v1 {};
v2 {};
EOF
  cat > pkg/lib.c <<'EOF'
#include "lib.h"
__asm__(".symver old_func, func@@v1, remove");
int old_func() { return 42; }
__asm__(".symver new_func, func@v2, remove");
int new_func() { return 43; }
EOF
  cat > pkg/lib.h <<'EOF'
int func();
EOF
  cat > pkg/intermediate.c <<'EOF'
#include "intermediate.h"
#include "lib.h"
int intermediate() { return func(); }
EOF
  cat > pkg/intermediate.h <<'EOF'
int intermediate();
EOF
  cat > pkg/test.c <<'EOF'
#include "intermediate.h"
#include "stdio.h"
int main() {
  printf("intermediate(): %d\n", intermediate());
}
EOF

  bazel test //pkg:test -s --test_output=all \
    --repo_env=BAZEL_LLVM_IFS=llvm-ifs-14 \
    &> "$TEST_log" || fail "Build failed"
  expect_log "action 'Compiling pkg/lib.c'"
  expect_log "action 'Linking pkg/liblib.so'"
  expect_log "action 'Linking pkg/libintermediate.so'"
  expect_log "action 'Linking pkg/test'"
  expect_log "intermediate(): 42"
  [[ -f ${PRODUCT_NAME}-bin/pkg/liblib.ifso ]] || fail "liblib.ifso not found"

  # Only modify the default symbol version of func(). This results in a
  # different implementation being at runtime after a relink. Without the
  # relink, the runtime behavior will be incorrect.
  cat > pkg/lib.c <<'EOF'
#include "lib.h"
__asm__(".symver old_func, func@v1, remove");
int old_func() { return 42; }
__asm__(".symver new_func, func@@v2, remove");
int new_func() { return 43; }
EOF
  bazel test //pkg:test -s --test_output=all \
    --repo_env=BAZEL_LLVM_IFS=llvm-ifs-14 \
    &> "$TEST_log" || fail "Build failed"
  expect_log "action 'Compiling pkg/lib.c'"
  expect_log "action 'Linking pkg/liblib.so'"
  expect_not_log "action 'Linking pkg/libintermediate.so'"
  expect_log "action 'Linking pkg/test'"
  expect_log "intermediate(): 43"
}

[fmeum]

@kjteske Happy to collaborate on LLVM patches that add versioned symbol support, it looks like this is indeed a blocker for adding Bazel support.

[ismell]

Does anyone have a link to the llvm-ifs patches? I'm looking to enable llvm-ifs for the ChromeOS portage builder: https://chromium-review.googlesource.com/c/chromiumos/bazel/+/5582954/5/docs/advanced.md

[ismell]

Here is the upstream bug: llvm/llvm-project#54516

If someone could post the patches there, it would be appreciated.