Add the default solib dir to the rpath for shared libs with transitions.

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 will be present under the default solib directory, so it should be on the rpath.

Adding the default solib directory to the rpath may cause the wrong version of a shared library to be loaded, e.g. if it has been compiled in the default configuration. To prevent this, we also add the transition mnemonic to the mangled name of the library (adding the default solib last to the rpath won't really help, because it can legitimately be added first).

#13819

Closes #14011.

PiperOrigin-RevId: 401521899

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

@@ -267,7 +267,10 @@ private Pair<Boolean, Boolean> addLinkerInputs(
         // When COPY_DYNAMIC_LIBRARIES_TO_BINARY is enabled, dynamic libraries are not symlinked
         // under solibDir, so don't check it and don't include solibDir.
         if (!featureConfiguration.isEnabled(CppRuleClasses.COPY_DYNAMIC_LIBRARIES_TO_BINARY)) {
-          if (libDir.equals(solibDir)) {
+          // The first fragment is bazel-out, and the second may contain a configuration mnemonic.
+          // We should always add the default solib dir because that's where libraries will be found
+          // e.g. in remote execution, so we ignore the first two fragments.
+          if (libDir.subFragment(2).equals(solibDir.subFragment(2))) {
             includeSolibDir = true;
           }
           if (libDir.equals(toolchainLibrariesSolibDir)) {

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 =
@@ -253,20 +255,25 @@ private static PathFragment getMangledName(
   }
 
   /**
-   * Compute the SONAME to use for a dynamic library. This name is basically the
-   * name of the shared library in its final symlinked location.
+   * Compute the SONAME to use for a dynamic library. This name is basically the name of the shared
+   * library in its final symlinked location.
    *
    * @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) {
+  public static String getDynamicLibrarySoname(
+      PathFragment libraryPath, boolean preserveName, String mnemonic) {
     String mangledName;
     if (preserveName) {
       mangledName = libraryPath.getBaseName();
     } else {
-      mangledName = "lib" + Actions.escapedPath(libraryPath.getPathString());
+      String mnemonicMangling = "";
+      if (mnemonic.contains("ST-")) {
+        mnemonicMangling = mnemonic.substring(mnemonic.indexOf("ST-")) + "_";
+      }
+      mangledName = "lib" + mnemonicMangling + Actions.escapedPath(libraryPath.getPathString());
     }
     return mangledName;
   }

src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD

@@ -232,6 +232,7 @@ java_test(
         "//src/test/java/com/google/devtools/build/lib/analysis/util",
         "//src/test/java/com/google/devtools/build/lib/packages:testutil",
         "//src/test/java/com/google/devtools/build/lib/testutil",
+        "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants",
         "//third_party:guava",
         "//third_party:junit4",
         "//third_party:truth",

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;
@@ -39,6 +40,7 @@
 import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig;
 import com.google.devtools.build.lib.packages.util.MockCcSupport;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
+import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.StringUtil;
@@ -2021,4 +2023,136 @@ public void testCcLibraryProducesEmptyArchive() throws Exception {
                 .toList())
         .isNotEmpty();
   }
+
+  @Test
+  public void testRpathIsNotAddedWhenThereAreNoSoDeps() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder().withFeatures(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER));
+
+    prepareCustomTransition();
+
+    scratch.file(
+        "BUILD",
+        "cc_library(",
+        "    name = 'malloc',",
+        "    srcs = ['malloc.cc'],",
+        "    linkstatic = 1,",
+        ")",
+        "cc_binary(",
+        "    name = 'main',",
+        "    srcs = ['main.cc'],",
+        "    malloc = ':malloc',",
+        "    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("-L" + TestConstants.PRODUCT_NAME + "-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("-L" + TestConstants.PRODUCT_NAME + "-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_library(",
+        "    name = 'malloc',",
+        "    srcs = ['malloc.cc'],",
+        "    linkstatic = 1,",
+        ")",
+        "cc_binary(",
+        "    name = 'main',",
+        "    srcs = ['main.cc'],",
+        "    linkstatic = 0,",
+        "    malloc = ':malloc',",
+        "    deps = ['dep1'],",
+        ")",
+        "",
+        "apply_custom_transition(",
+        "    name = 'dep1',",
+        "    deps = [",
+        "        ':dep2',':dep3',",
+        "    ],",
+        ")",
+        "",
+        "cc_library(",
+        "    name = 'dep2',",
+        "    srcs = ['test.cc'],",
+        "    hdrs = ['test.h'],",
+        ")",
+        "cc_library(",
+        "    name = 'dep3',",
+        "    srcs = ['test3.cc'],",
+        "    hdrs = ['test3.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("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild-ST-");
+    assertThat(Joiner.on(" ").join(linkArgv)).containsMatch("-lST-[0-9a-f]+_transition_Slibdep2");
+    assertThat(Joiner.on(" ").join(linkArgv))
+        .doesNotContain("-L" + TestConstants.PRODUCT_NAME + "-out/k8-fastbuild/bin/_solib_k8");
+    assertThat(Joiner.on(" ").join(linkArgv)).doesNotContain("-ltransition_Slibdep2");
+  }
 }