Skip to content

Commit

Permalink
Add the runfiles directory to the runtime library search paths
Browse files Browse the repository at this point in the history
When executing a C++ binary through REv2 directly (e.g., directly as a
genrule, not as a dependency of another script/executable), the input
root will only contain a \_solib\_arch directory inside the executable's
runfiles directory. This means that library lookups also need to
consider rpath $ORIGIN/$executable.runfiles/$workspace/\_solib\_$arch/.
Only looking at ../../../\_solib\_$arch/ is not sufficient.

This issue isn't noticeable when doing local execution, for the reason
that rtld/dyld will first do a realpath() on the executable, meaning the
lookup of shared objects takes place outside of the sandbox, inside of
bazel-out/ directly.

This change extends LibrariesToLinkCollector to duplicate all runtime
library search paths. One for bare execution, and one for execution
inside a runfiles directory.

Closes bazelbuild#14600.

Change-Id: I4256cb46fee737139aba6f1cfb0531aea5deb315
PiperOrigin-RevId: 469733573
  • Loading branch information
EdSchouten authored and aiuto committed Oct 12, 2022
1 parent 60973a2 commit 3d123ed
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
import com.google.devtools.build.lib.cmdline.LabelConstants;
Expand All @@ -40,7 +41,6 @@ public class LibrariesToLinkCollector {
private final PathFragment toolchainLibrariesSolibDir;
private final CppConfiguration cppConfiguration;
private final CcToolchainProvider ccToolchainProvider;
private final Artifact outputArtifact;
private final boolean isLtoIndexing;
private final PathFragment solibDir;
private final Iterable<? extends LinkerInput> linkerInputs;
Expand All @@ -49,7 +49,8 @@ public class LibrariesToLinkCollector {
private final Artifact thinltoParamFile;
private final FeatureConfiguration featureConfiguration;
private final boolean needWholeArchive;
private final String rpathRoot;
private final ImmutableList<String> potentialExecRoots;
private final ImmutableList<String> rpathRoots;
private final boolean needToolchainLibrariesRpath;
private final Map<Artifact, Artifact> ltoMap;
private final RuleErrorConsumer ruleErrorConsumer;
Expand All @@ -76,7 +77,6 @@ public LibrariesToLinkCollector(
this.cppConfiguration = cppConfiguration;
this.ccToolchainProvider = toolchain;
this.toolchainLibrariesSolibDir = toolchainLibrariesSolibDir;
this.outputArtifact = output;
this.solibDir = solibDir;
this.isLtoIndexing = isLtoIndexing;
this.allLtoArtifacts = allLtoArtifacts;
Expand Down Expand Up @@ -106,12 +106,13 @@ public LibrariesToLinkCollector(
// and the second could use $ORIGIN/../_solib_[arch]. But since this is a shared
// artifact, both are symlinks to the same place, so
// there's no *one* RPATH setting that fits all targets involved in the sharing.
rpathRoot = ccToolchainProvider.getSolibDirectory() + "/";
potentialExecRoots = ImmutableList.of();
rpathRoots = ImmutableList.of(ccToolchainProvider.getSolibDirectory() + "/");
} else {
// When executed from within a runfiles directory, the binary lies under a path such as
// target.runfiles/some_repo/pkg/file, whereas the solib directory is located under
// target.runfiles/main_repo.
PathFragment runfilesPath = outputArtifact.getRunfilesPath();
PathFragment runfilesPath = output.getRunfilesPath();
String runfilesExecRoot;
if (runfilesPath.startsWith(LabelConstants.EXTERNAL_RUNFILES_PATH_PREFIX)) {
// runfilesPath is of the form ../some_repo/pkg/file, walk back some_repo/pkg and then
Expand All @@ -121,7 +122,22 @@ public LibrariesToLinkCollector(
// runfilesPath is of the form pkg/file, walk back pkg to reach the main workspace.
runfilesExecRoot = "../".repeat(runfilesPath.segmentCount() - 1);
}
rpathRoot = runfilesExecRoot + ccToolchainProvider.getSolibDirectory() + "/";

// When the binary gets run through remote execution as the main executable (i.e., not as a
// dependency of another target), it will not be able to find its libraries at
// $ORIGIN/../../../_solib_[arch], as execution is not taking place inside the runfiles
// directory. Solve this by adding ${executable}.runfiles/${workspace}/_solib_[arch] as an
// alternative rpath root.
PathFragment rootRelativePath = output.getRootRelativePath();
potentialExecRoots =
ImmutableList.of(
rootRelativePath.getBaseName() + ".runfiles/" + workspaceName + "/",
runfilesExecRoot);
rpathRoots =
ImmutableList.copyOf(
Lists.transform(
potentialExecRoots,
(execRoot) -> execRoot + ccToolchainProvider.getSolibDirectory() + "/"));
}

ltoMap = generateLtoMap();
Expand Down Expand Up @@ -196,10 +212,10 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
// directory. In other words, given blaze-bin/my/package/binary, rpathRoot would be
// "../../_solib_[arch]".
if (needToolchainLibrariesRpath) {
runtimeLibrarySearchDirectories.add(
"../".repeat(outputArtifact.getRootRelativePath().segmentCount() - 1)
+ toolchainLibrariesSolibName
+ "/");
for (String potentialExecRoot : potentialExecRoots) {
runtimeLibrarySearchDirectories.add(
potentialExecRoot + toolchainLibrariesSolibName + "/");
}
}
if (isNativeDeps) {
// We also retain the $ORIGIN/ path to solibs that are in _solib_<arch>, as opposed to
Expand Down Expand Up @@ -231,7 +247,9 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
NestedSetBuilder<String> allRuntimeLibrarySearchDirectories = NestedSetBuilder.linkOrder();
// rpath ordering matters for performance; first add the one where most libraries are found.
if (includeSolibDir) {
allRuntimeLibrarySearchDirectories.add(rpathRoot);
for (String rpathRoot : rpathRoots) {
allRuntimeLibrarySearchDirectories.add(rpathRoot);
}
}
allRuntimeLibrarySearchDirectories.addAll(rpathRootsForExplicitSoDeps.build());
if (includeToolchainLibrariesSolibDir) {
Expand Down Expand Up @@ -346,17 +364,21 @@ private void addDynamicInputLinkOptions(
// When all dynamic deps are built in transitioned configurations, the default solib dir is
// not created. While resolving paths, the dynamic linker stops at the first directory that
// does not exist, even when followed by "../". We thus have to normalize the relative path.
String relativePathToRoot =
rpathRoot + dotdots + libDir.relativeTo(commonParent).getPathString();
String normalizedPathToRoot = PathFragment.create(relativePathToRoot).getPathString();
rpathRootsForExplicitSoDeps.add(normalizedPathToRoot);
for (String rpathRoot : rpathRoots) {
String relativePathToRoot =
rpathRoot + dotdots + libDir.relativeTo(commonParent).getPathString();
String normalizedPathToRoot = PathFragment.create(relativePathToRoot).getPathString();
rpathRootsForExplicitSoDeps.add(normalizedPathToRoot);
}

// Unless running locally, libraries will be available under the root relative path, so we
// should add that to the rpath as well.
if (inputArtifact.getRootRelativePathString().startsWith("_solib_")) {
PathFragment artifactPathUnderSolib = inputArtifact.getRootRelativePath().subFragment(1);
rpathRootsForExplicitSoDeps.add(
rpathRoot + artifactPathUnderSolib.getParentDirectory().getPathString());
for (String rpathRoot : rpathRoots) {
rpathRootsForExplicitSoDeps.add(
rpathRoot + artifactPathUnderSolib.getParentDirectory().getPathString());
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2155,6 +2155,10 @@ public void testRpathAndLinkPathsWithoutTransitions() throws Exception {
Artifact mainBin = getBinArtifact("main", main);
CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
List<String> linkArgv = action.getLinkCommandLine().arguments();
assertThat(linkArgv)
.contains(
String.format(
"-Wl,-rpath,$ORIGIN/main.runfiles/%s/_solib_k8/", TestConstants.WORKSPACE_NAME));
assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
assertThat(linkArgv)
.contains("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild/bin/_solib_k8");
Expand Down Expand Up @@ -2215,6 +2219,10 @@ public void testRpathRootIsAddedEvenWithTransitionedDepsOnly() throws Exception
Artifact mainBin = getBinArtifact("main", main);
CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
List<String> linkArgv = action.getLinkCommandLine().arguments();
assertThat(linkArgv)
.contains(
String.format(
"-Wl,-rpath,$ORIGIN/main.runfiles/%s/_solib_k8/", TestConstants.WORKSPACE_NAME));
assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
assertThat(Joiner.on(" ").join(linkArgv))
.contains("-Wl,-rpath,$ORIGIN/../../../k8-fastbuild-ST-");
Expand Down

0 comments on commit 3d123ed

Please sign in to comment.