Add the runfiles directory to the runtime library search paths

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.

Change-Id: I4256cb46fee737139aba6f1cfb0531aea5deb315

src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParameterFile;
+import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
 import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
 import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
@@ -801,7 +802,8 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
             allowLtoIndexing,
             nonExpandedLinkerInputs,
             needWholeArchive,
-            ruleErrorConsumer);
+            ruleErrorConsumer,
+            ((RuleContext) actionConstructionContext).getWorkspaceName());
     CollectedLibrariesToLink collectedLibrariesToLink =
         librariesToLinkCollector.collectLibrariesToLink();
 

src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java

@@ -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.collect.nestedset.NestedSet;
@@ -39,7 +40,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;
@@ -48,7 +48,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;
@@ -69,12 +70,12 @@ public LibrariesToLinkCollector(
       boolean allowLtoIndexing,
       Iterable<LinkerInput> linkerInputs,
       boolean needWholeArchive,
-      RuleErrorConsumer ruleErrorConsumer) {
+      RuleErrorConsumer ruleErrorConsumer,
+      String workspaceName) {
     this.isNativeDeps = isNativeDeps;
     this.cppConfiguration = cppConfiguration;
     this.ccToolchainProvider = toolchain;
     this.toolchainLibrariesSolibDir = toolchainLibrariesSolibDir;
-    this.outputArtifact = output;
     this.solibDir = solibDir;
     this.isLtoIndexing = isLtoIndexing;
     this.allLtoArtifacts = allLtoArtifacts;
@@ -104,12 +105,24 @@ 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 {
-      rpathRoot =
-          "../".repeat(outputArtifact.getRootRelativePath().segmentCount() - 1)
-              + 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
+               + "/",
+           "../".repeat(rootRelativePath.segmentCount() - 1));
+      rpathRoots = ImmutableList.copyOf(
+          Lists.transform(potentialExecRoots,
+                          (execRoot) -> execRoot + ccToolchainProvider.getSolibDirectory() + "/"));
     }
 
     ltoMap = generateLtoMap();
@@ -184,10 +197,12 @@ 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
@@ -219,7 +234,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) {
@@ -334,17 +351,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());
+        }
       }
     }
 

src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java

@@ -2161,6 +2161,7 @@ public void testRpathAndLinkPathsWithoutTransitions() throws Exception {
     Artifact mainBin = getBinArtifact("main", main);
     CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
     List<String> linkArgv = action.getLinkCommandLine().arguments();
+    assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/main.runfiles/__main__/_solib_k8/");
     assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
     assertThat(linkArgv)
         .contains("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild/bin/_solib_k8");
@@ -2221,6 +2222,7 @@ public void testRpathRootIsAddedEvenWithTransitionedDepsOnly() throws Exception
     Artifact mainBin = getBinArtifact("main", main);
     CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
     List<String> linkArgv = action.getLinkCommandLine().arguments();
+    assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/main.runfiles/__main__/_solib_k8/");
     assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
     assertThat(Joiner.on(" ").join(linkArgv))
         .contains("-Wl,-rpath,$ORIGIN/../../../k8-fastbuild-ST-");