Add the default solib dir to rpath even when all shared libs have tra…

…nsitions.

When an executable has dynamically-linked dependencies that have transitions,
shared libraries are looked up under rpaths like this:
    $ORIGIN/(../)*_solib_k8/../../../k8-fastbuild-ST-[0-9a-f]+/bin/_solib_k8

Unless running under the execroot, the transitioned solib directory may not be
available at all; the right files are actually present under the default solib
directory, so it should be an rpath.

Adding another directory to the rpath may cause the wrong version of a shared
library to be loaded. To prevent this, we also add the transition mnemonic to
the name of the library (adding the default solib last to the rpath won't really
help, because it could legitimately be added first). This is a potential issue
only when running locally under the execroot, but it could be annoying to debug.

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

@@ -693,7 +693,9 @@ private boolean createDynamicLinkAction(
             ImmutableList.of(
                 "-Wl,-soname="
                     + SolibSymlinkAction.getDynamicLibrarySoname(
-                        linkerOutput.getRootRelativePath(), /* preserveName= */ false));
+                        linkerOutput.getRootRelativePath(),
+                        /* preserveName= */ false,
+                        actionConstructionContext.getConfiguration().getMnemonic()));
       }
     }
 

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

@@ -226,6 +226,12 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
       allRuntimeLibrarySearchDirectories.addTransitive(runtimeLibrarySearchDirectories.build());
     }
 
+    // If we had any rpaths, we should also add the rpath root as a fallback: that's where all the
+    // libraries will be found when not running from the execroot.
+    if (!allRuntimeLibrarySearchDirectories.isEmpty()) {
+      allRuntimeLibrarySearchDirectories.add(rpathRoot);
+    }
+
     return new CollectedLibrariesToLink(
         librariesToLink,
         expandedLinkerInputsBuilder.build(),

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

@@ -140,6 +140,7 @@ public static Artifact getDynamicLibrarySymlink(
         getMangledName(
             actionRegistry.getOwner().getLabel(),
             solibDir,
+            actionConstructionContext.getConfiguration().getMnemonic(),
             library.getRootRelativePath(),
             preserveName,
             prefixConsumer);
@@ -234,11 +235,12 @@ private static Artifact getDynamicLibrarySymlinkInternal(
   private static PathFragment getMangledName(
       Label label,
       String solibDir,
+      String mnemonic,
       PathFragment libraryPath,
       boolean preserveName,
       boolean prefixConsumer) {
     String escapedRulePath = Actions.escapedPath("_" + label);
-    String soname = getDynamicLibrarySoname(libraryPath, preserveName);
+    String soname = getDynamicLibrarySoname(libraryPath, preserveName, mnemonic);
     PathFragment solibDirPath = PathFragment.create(solibDir);
     if (preserveName) {
       String escapedLibraryPath =
@@ -258,15 +260,21 @@ private static PathFragment getMangledName(
    *
    * @param libraryPath name of the shared library that needs to be mangled
    * @param preserveName true if filename should be preserved, false - mangled
+   * @param mnemonic the output directory mnemonic, to be mangled in for nondefault configurations
    * @return soname to embed in the dynamic library
    */
   public static String getDynamicLibrarySoname(PathFragment libraryPath,
-                                               boolean preserveName) {
+                                               boolean preserveName,
+                                               String mnemonic) {
     String mangledName;
     if (preserveName) {
       mangledName = libraryPath.getBaseName();
     } else {
-      mangledName = "lib" + Actions.escapedPath(libraryPath.getPathString());
+      String mnemonicMangling = "";
+      if (mnemonic.indexOf("ST-") > -1) {
+        mnemonicMangling = "ST_" + mnemonic.substring(mnemonic.indexOf("ST-") + 3) + "_";
+      }
+      mangledName = "lib" + mnemonicMangling + Actions.escapedPath(libraryPath.getPathString());
     }
     return mangledName;
   }

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

@@ -18,6 +18,7 @@
 import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
@@ -2035,4 +2036,116 @@ public void testImplementationDepsFailsWithoutFlag() throws Exception {
     getConfiguredTarget("//foo:lib");
     assertContainsEvent("requires --experimental_cc_implementation_deps");
   }
+
+  @Test
+  public void testRpathIsNotAddedWhenThereAreNoSoDeps() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));
+
+    prepareCustomTransition();
+
+    scratch.file(
+        "BUILD",
+        "cc_binary(",
+        "    name = 'main',",
+        "    srcs = ['main.cc'],",
+        "    linkstatic = 0,",
+        ")");
+
+    ConfiguredTarget main = getConfiguredTarget("//:main");
+    Artifact mainBin = getBinArtifact("main", main);
+    CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
+    assertThat(Joiner.on(" ").join(action.getLinkCommandLine().arguments())).doesNotContain(
+        "-Wl,-rpath");
+  }
+
+  @Test
+  public void testRpathAndLinkPathsWithoutTransitions() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));
+
+    prepareCustomTransition();
+    useConfiguration("--cpu=k8", "--compilation_mode=fastbuild");
+
+    scratch.file(
+        "no-transition/BUILD",
+        "cc_binary(",
+        "    name = 'main',",
+        "    srcs = ['main.cc'],",
+        "    linkstatic = 0,",
+        "    deps = ['dep1'],",
+        ")",
+        "",
+        "cc_library(",
+        "    name = 'dep1',",
+        "    srcs = ['test.cc'],",
+        "    hdrs = ['test.h'],",
+        ")");
+
+    ConfiguredTarget main = getConfiguredTarget("//no-transition:main");
+    Artifact mainBin = getBinArtifact("main", main);
+    CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
+    List<String> linkArgv = action.getLinkCommandLine().arguments();
+    assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
+    assertThat(linkArgv).contains("-Lbazel-out/k8-fastbuild/bin/_solib_k8");
+    assertThat(linkArgv).contains("-lno-transition_Slibdep1");
+    assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain(
+        "-Wl,-rpath,$ORIGIN/../_solib_k8/../../../k8-fastbuild-ST-");
+    assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain("-Lbazel-out/k8-fastbuild-ST-");
+    assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain("-lST_");
+  }
+
+  @Test
+  public void testRpathRootIsAddedEvenWithTransitionedDepsOnly() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));
+
+    prepareCustomTransition();
+    useConfiguration("--cpu=k8", "--compilation_mode=fastbuild");
+
+    scratch.file(
+        "transition/BUILD",
+        "load(':custom_transition.bzl', 'apply_custom_transition')",
+        "cc_binary(",
+        "    name = 'main',",
+        "    srcs = ['main.cc'],",
+        "    linkstatic = 0,",
+        "    deps = ['dep1'],",
+        ")",
+        "",
+        "apply_custom_transition(",
+        "    name = 'dep1',",
+        "    deps = [",
+        "        ':dep2',",
+        "    ],",
+        ")",
+        "",
+        "cc_library(",
+        "    name = 'dep2',",
+        "    srcs = ['test.cc'],",
+        "    hdrs = ['test.h'],",
+        ")");
+
+    ConfiguredTarget main = getConfiguredTarget("//transition:main");
+    Artifact mainBin = getBinArtifact("main", main);
+    CppLinkAction action = (CppLinkAction) getGeneratingAction(mainBin);
+    List<String> linkArgv = action.getLinkCommandLine().arguments();
+    assertThat(linkArgv).contains("-Wl,-rpath,$ORIGIN/../_solib_k8/");
+    assertThat(Joiner.on(" ").join(linkArgv)).contains(
+        "-Wl,-rpath,$ORIGIN/../_solib_k8/../../../k8-fastbuild-ST-");
+    assertThat(Joiner.on(" ").join(linkArgv)).contains("-Lbazel-out/k8-fastbuild-ST-");
+    assertThat(Joiner.on(" ").join(linkArgv)).containsMatch("-lST_[0-9a-f]+_transition_Slibdep2");
+    assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain(
+        "-Lbazel-out/k8-fastbuild/bin/_solib_k8");
+    assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain("-ltransition_Slibdep2");
+  }
 }