C++: Fix crash reported in bazelbuild#7721

objc_libraries used to always take the static libraries of C++ dependencies even if dynamic libraries were available. This is because support for dynamic linking of C++ dependencies hasn't been added to objective-C rules yet.

With bazelbuild@3e727b1#diff-033df3f3a834149658486b8a5754baefL438, we started propagating both static libraries and dynamic libraries in the new implementation of LibraryToLink. Since cc_test by default will always try to link dynamically, the behavior changed and if we had the dependency chain cc_test -> objc_library -> cc_library; the cc_test would start using the .so file from the cc_library, whereas before it used the .a file. Linking the .so file gives an error since as mentioned, support for this hasn't been added to Objective-C.

The fix has been to nullify the dynamic library fields of LibraryToLink as long as a static library was present. This makes the behavior the same as before.

RELNOTES:none
PiperOrigin-RevId: 239226003

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

@@ -566,6 +566,8 @@ public static List<Artifact> getDynamicLibrariesForLinking(Iterable<LibraryToLin
     return dynamicLibrariesForLinkingBuilder.build();
   }
 
+  public abstract Builder toBuilder();
+
   /** Builder for LibraryToLink. */
   @AutoValue.Builder
   public abstract static class Builder {
@@ -592,11 +594,13 @@ abstract Builder setPicSharedNonLtoBackends(
 
     public abstract Builder setDynamicLibrary(Artifact dynamicLibrary);
 
-    abstract Builder setResolvedSymlinkDynamicLibrary(Artifact resolvedSymlinkDynamicLibrary);
+    public abstract Builder setResolvedSymlinkDynamicLibrary(
+        Artifact resolvedSymlinkDynamicLibrary);
 
-    abstract Builder setInterfaceLibrary(Artifact interfaceLibrary);
+    public abstract Builder setInterfaceLibrary(Artifact interfaceLibrary);
 
-    abstract Builder setResolvedSymlinkInterfaceLibrary(Artifact resolvedSymlinkInterfaceLibrary);
+    public abstract Builder setResolvedSymlinkInterfaceLibrary(
+        Artifact resolvedSymlinkInterfaceLibrary);
 
     public abstract Builder setAlwayslink(boolean alwayslink);
 

src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java

@@ -136,7 +136,8 @@ private CcLinkingContext buildCcLinkingContext(
                   FileSystemUtils.removeExtension(library.getRootRelativePathString()))
               .build());
     }
-    libraries.addAll(objcProvider.get(ObjcProvider.CC_LIBRARY));
+
+    libraries.addAll(convertLibrariesToStaticLibraries(objcProvider.get(ObjcProvider.CC_LIBRARY)));
 
     CcLinkingContext.Builder ccLinkingContext =
         CcLinkingContext.builder()
@@ -153,6 +154,27 @@ private CcLinkingContext buildCcLinkingContext(
     return ccLinkingContext.build();
   }
 
+  /**
+   * This method removes dynamic libraries from LibraryToLink objects coming from C++ dependencies.
+   * The reason for this is that objective-C rules do not support linking the dynamic version of the
+   * libraries.
+   */
+  private ImmutableList<LibraryToLink> convertLibrariesToStaticLibraries(
+      Iterable<LibraryToLink> librariesToLink) {
+    ImmutableList.Builder<LibraryToLink> libraries = ImmutableList.builder();
+    for (LibraryToLink libraryToLink : librariesToLink) {
+      LibraryToLink.Builder staticLibraryToLink = libraryToLink.toBuilder();
+      if (libraryToLink.getPicStaticLibrary() != null || libraryToLink.getStaticLibrary() != null) {
+        staticLibraryToLink.setDynamicLibrary(null);
+        staticLibraryToLink.setResolvedSymlinkDynamicLibrary(null);
+        staticLibraryToLink.setInterfaceLibrary(null);
+        staticLibraryToLink.setResolvedSymlinkInterfaceLibrary(null);
+      }
+      libraries.add(staticLibraryToLink.build());
+    }
+    return libraries.build();
+  }
+
   /** Throws errors or warnings for bad attribute state. */
   private static void validateAttributes(RuleContext ruleContext) throws RuleErrorException {
     if (ObjcRuleClasses.objcConfiguration(ruleContext).disableObjcLibraryResources()) {

src/test/java/com/google/devtools/build/lib/rules/objc/ObjcLibraryTest.java

@@ -48,6 +48,7 @@
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
 import com.google.devtools.build.lib.analysis.util.ScratchAttributeWriter;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
+import com.google.devtools.build.lib.packages.util.MockCcSupport;
 import com.google.devtools.build.lib.packages.util.MockObjcSupport;
 import com.google.devtools.build.lib.rules.apple.ApplePlatform;
 import com.google.devtools.build.lib.rules.apple.AppleToolchain;
@@ -2134,4 +2135,20 @@ public void testIncompatibleResourceAttributeFlag_resourcesEmpty() throws Except
         "objc_library resource attributes are not allowed. Please use the 'data' attribute instead",
         "objc_library(name = 'x', resources = [])");
   }
+
+  /** Regression test for https://github.com/bazelbuild/bazel/issues/7721. */
+  @Test
+  public void testToolchainRuntimeLibrariesSolibDir() throws Exception {
+    MockObjcSupport.setup(
+        mockToolsConfig,
+        MockCcSupport.SUPPORTS_INTERFACE_SHARED_LIBRARIES_FEATURE,
+        MockCcSupport.SUPPORTS_DYNAMIC_LINKER_FEATURE);
+    scratch.file(
+        "foo/BUILD",
+        "cc_test(name = 'd', deps = [':b'])",
+        "objc_library(name = 'b', deps = [':a'])",
+        "cc_library(name = 'a', srcs = ['a.c'])");
+    ConfiguredTarget configuredTarget = getConfiguredTarget("//foo:d");
+    assertThat(configuredTarget).isNotNull();
+  }
 }

src/test/shell/bazel/apple/bazel_objc_test.sh

@@ -159,4 +159,41 @@ EOF
     || fail "should fail to find symbol addOne"
 }
 
+function test_cc_test_depending_on_objc() {
+  setup_objc_test_support
+
+  rm -rf foo
+  mkdir -p foo
+
+  cat >foo/a.cc <<EOF
+#include <iostream>
+int main(int argc, char** argv) {
+  std::cout << "Hello! I'm a test!\n";
+  return 0;
+}
+EOF
+
+  cat >foo/BUILD <<EOF
+cc_library(
+    name = "a",
+    srcs = ["a.cc"],
+)
+
+objc_library(
+    name = "b",
+    deps = [
+        ":a",
+    ],
+)
+
+cc_test(
+    name = "d",
+    deps = [":b"],
+)
+EOF
+
+  bazel test --verbose_failures \
+      //foo:d>$TEST_log 2>&1 || fail "should pass"
+}
+
 run_suite "objc/ios test suite"