Add supportsDynamicLinker to CppConfiguration

Rather than a new protobuf field, this enabling flag is presence of a section with DYNAMIC linking mode in the CROSSTOOL file, whether or not it needs to specify any linker flags.

RELNOTES[NEW]: Better support for toolchains that don't have a dynamic linker.

--
MOS_MIGRATED_REVID=114110200

5 files changed, 143 insertions, 5 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 700de93de5..fe6428b1b7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -138,8 +138,10 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
     }
 
     Artifact soImplArtifact = null;
+    boolean supportsDynamicLinker =
+        ruleContext.getFragment(CppConfiguration.class).supportsDynamicLinker();
     boolean createDynamicLibrary =
-        !linkStatic && appearsToHaveObjectFiles(ruleContext.attributes());
+        !linkStatic && appearsToHaveObjectFiles(ruleContext.attributes()) && supportsDynamicLinker;
     if (ruleContext.getRule().isAttrDefined("outs", Type.STRING_LIST)) {
       List<String> outs = ruleContext.attributes().get("outs", Type.STRING_LIST);
       if (outs.size() > 1) {
@@ -178,13 +180,21 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory {
     helper.setCreateDynamicLibrary(createDynamicLibrary);
     helper.setDynamicLibrary(soImplArtifact);
 
+    // If the reason we're not creating a dynamic library is that the toolchain
+    // doesn't support it, then register an action which complains when triggered,
+    // which only happens when some rule explicitly depends on the dynamic library.
+    if (!createDynamicLibrary && !supportsDynamicLinker) {
+      Artifact solibArtifact = CppHelper.getLinkedArtifact(
+          ruleContext, LinkTargetType.DYNAMIC_LIBRARY);
+      ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(),
+          ImmutableList.of(solibArtifact), "Toolchain does not support dynamic linking"));
+    } else if (!createDynamicLibrary
+        && ruleContext.attributes().isConfigurable("srcs", BuildType.LABEL_LIST)) {
     // If "srcs" is configurable, the .so output is always declared because the logic that
     // determines implicit outs doesn't know which value of "srcs" will ultimately get chosen. Here,
     // where we *do* have the correct value, it may not contain any source files to generate an
     // .so with. If that's the case, register a fake generating action to prevent a "no generating
     // action for this artifact" error.
-    if (!createDynamicLibrary
-        && ruleContext.attributes().isConfigurable("srcs", BuildType.LABEL_LIST)) {
       Artifact solibArtifact = CppHelper.getLinkedArtifact(
           ruleContext, LinkTargetType.DYNAMIC_LIBRARY);
       ruleContext.registerAction(new FailAction(ruleContext.getActionOwner(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 2b9dca28ef..d67f0058d1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -284,6 +284,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   private final boolean supportsGoldLinker;
   private final boolean supportsThinArchives;
   private final boolean supportsStartEndLib;
+  private final boolean supportsDynamicLinker;
   private final boolean supportsInterfaceSharedObjects;
   private final boolean supportsEmbeddedRuntimes;
   private final boolean supportsFission;
@@ -507,16 +508,35 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     }
 
     linkOptionsFromLinkingMode = ArrayListMultimap.create();
+
+    // If a toolchain supports dynamic libraries at all, there must be at least one
+    // of the following:
+    // - a "DYNAMIC" section in linking_mode_flags (even if no flags are needed)
+    // - a non-empty list in one of the dynamicLibraryLinkerFlag fields
+    // If none of the above contain data, then the toolchain can't do dynamic linking.
+    boolean haveDynamicMode = false;
+
     for (LinkingModeFlags flags : toolchain.getLinkingModeFlagsList()) {
       LinkingMode realmode = importLinkingMode(flags.getMode());
+      if (realmode == LinkingMode.DYNAMIC) {
+        haveDynamicMode = true;
+      }
       linkOptionsFromLinkingMode.putAll(realmode, flags.getLinkerFlagList());
     }
 
     this.commonLinkOptions = ImmutableList.copyOf(toolchain.getLinkerFlagList());
+    List<String> linkerFlagList = toolchain.getDynamicLibraryLinkerFlagList();
+    List<CToolchain.OptionalFlag> optionalLinkerFlagList =
+        toolchain.getOptionalDynamicLibraryLinkerFlagList();
+    if (!linkerFlagList.isEmpty() || !optionalLinkerFlagList.isEmpty()) {
+      haveDynamicMode = true;
+    }
+    this.supportsDynamicLinker = haveDynamicMode;
     dynamicLibraryLinkFlags = new FlagList(
-        ImmutableList.copyOf(toolchain.getDynamicLibraryLinkerFlagList()),
-        convertOptionalOptions(toolchain.getOptionalDynamicLibraryLinkerFlagList()),
+        ImmutableList.copyOf(linkerFlagList),
+        convertOptionalOptions(optionalLinkerFlagList),
         ImmutableList.<String>of());
+
     this.objcopyOptions = ImmutableList.copyOf(toolchain.getObjcopyEmbedFlagList());
     this.ldOptions = ImmutableList.copyOf(toolchain.getLdEmbedFlagList());
     this.arOptions = copyOrDefaultIfEmpty(toolchain.getArFlagList(), "rcsD");
@@ -1093,6 +1113,13 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   }
 
   /**
+   * Returns whether the toolchain supports dynamic linking.
+   */
+  public boolean supportsDynamicLinker() {
+    return supportsDynamicLinker;
+  }
+
+  /**
    * Returns whether build_interface_so can build interface shared objects for this toolchain.
    * Should be true if this toolchain generates ELF objects.
    */
diff --git a/src/test/java/com/google/devtools/build/lib/MOCK_CROSSTOOL b/src/test/java/com/google/devtools/build/lib/MOCK_CROSSTOOL
index 60bbf2b2d0..323c3598ef 100644
--- a/src/test/java/com/google/devtools/build/lib/MOCK_CROSSTOOL
+++ b/src/test/java/com/google/devtools/build/lib/MOCK_CROSSTOOL
@@ -29,6 +29,46 @@ toolchain {
   target_cpu: "k8"
   target_libc: "local"
   compiler: "compiler"
+  linking_mode_flags { mode: DYNAMIC }
+
+  abi_version: "local"
+  abi_libc_version: "local"
+
+  tool_path { name: "ar" path: "/usr/bin/ar" }
+  tool_path { name: "compat-ld" path: "/usr/bin/ld" }
+  tool_path { name: "cpp" path: "/usr/bin/cpp" }
+  tool_path { name: "dwp" path: "/usr/bin/dwp" }
+  tool_path { name: "gcc" path: "/usr/bin/gcc" }
+  tool_path { name: "gcov" path: "/usr/bin/gcov" }
+  tool_path { name: "ld" path: "/usr/bin/ld" }
+  tool_path { name: "nm" path: "/usr/bin/nm" }
+  tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
+  tool_path { name: "objdump" path: "/usr/bin/objdump" }
+  tool_path { name: "strip" path: "/usr/bin/strip" }
+
+  needsPic: true
+  supports_fission: true
+
+  builtin_sysroot: ""
+  cxx_flag: "-std=c++0x"
+  linker_flag: "-lstdc++"
+  cxx_builtin_include_directory: "/usr/lib/gcc/"
+  cxx_builtin_include_directory: "/usr/local/include"
+  cxx_builtin_include_directory: "/usr/include"
+  objcopy_embed_flag: "-I"
+  objcopy_embed_flag: "binary"
+  supports_interface_shared_objects: true
+}
+
+toolchain {
+  toolchain_identifier: "k8-no-dyn-linker"
+
+  host_system_name: "local"
+  target_system_name: "local"
+  target_cpu: "k8"
+  target_libc: "local"
+  compiler: "compiler_no_dyn_linker"
+  # No linking_mode_flags here
 
   abi_version: "local"
   abi_libc_version: "local"
@@ -67,6 +107,7 @@ toolchain {
   target_cpu: "piii"
   target_libc: "local"
   compiler: "compiler"
+  linking_mode_flags { mode: DYNAMIC }
 
   abi_version: "local"
   abi_libc_version: "local"
@@ -104,6 +145,50 @@ toolchain {
   target_cpu: "darwin"
   target_libc: "macosx"
   compiler: "compiler"
+  linking_mode_flags { mode: DYNAMIC }
+
+  abi_version: "local"
+  abi_libc_version: "local"
+
+  tool_path { name: "ar" path: "/usr/bin/libtool" }
+  tool_path { name: "compat-ld" path: "/usr/bin/ld" }
+  tool_path { name: "cpp" path: "/usr/bin/cpp" }
+  tool_path { name: "dwp" path: "/usr/bin/dwp" }
+  tool_path { name: "gcc" path: "/usr/bin/gcc" }
+  tool_path { name: "gcov" path: "/usr/bin/gcov" }
+  tool_path { name: "ld" path: "/usr/bin/ld" }
+  tool_path { name: "nm" path: "/usr/bin/nm" }
+  tool_path { name: "objcopy" path: "/usr/bin/objcopy" }
+  tool_path { name: "objdump" path: "/usr/bin/objdump" }
+  tool_path { name: "strip" path: "/usr/bin/strip" }
+
+  needsPic: false
+
+  builtin_sysroot: ""
+  cxx_flag: "-std=c++0x"
+  ar_flag: "-static"
+  ar_flag: "-s"
+  ar_flag: "-o"
+  linker_flag: "-lstdc++"
+  cxx_builtin_include_directory: "/usr/include"
+  cxx_builtin_include_directory: "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain"
+  cxx_builtin_include_directory: "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs"
+  cxx_builtin_include_directory: "/opt/local/include"
+  cxx_builtin_include_directory: "/Library/Developer/CommandLineTools"
+  objcopy_embed_flag: "-I"
+  objcopy_embed_flag: "binary"
+  supports_interface_shared_objects: true
+}
+
+toolchain {
+  toolchain_identifier: "local_darwin-no-dyn-linker"
+
+  host_system_name: "local"
+  target_system_name: "local"
+  target_cpu: "darwin"
+  target_libc: "macosx"
+  compiler: "compiler_no_dyn_linker"
+  # No linking_mode_flags here
 
   abi_version: "local"
   abi_libc_version: "local"
@@ -146,6 +231,7 @@ toolchain {
   target_cpu: "k8"
   target_libc: "local"
   compiler: "windows_mingw"
+  linking_mode_flags { mode: DYNAMIC }
 
   abi_version: "local"
   abi_libc_version: "local"
@@ -179,6 +265,7 @@ toolchain {
   target_cpu: "k8"
   target_libc: "local"
   compiler: "windows_msys64_mingw64"
+  linking_mode_flags { mode: DYNAMIC }
 
   abi_version: "local"
   abi_libc_version: "local"
@@ -211,6 +298,7 @@ toolchain {
   target_cpu: "k8"
   target_libc: "local"
   compiler: "windows_clang"
+  linking_mode_flags { mode: DYNAMIC }
 
   abi_version: "local"
   abi_libc_version: "local"
@@ -250,6 +338,7 @@ toolchain {
   abi_libc_version: "armeabi-v7a"
   builtin_sysroot: ""
   compiler: "compiler"
+  linking_mode_flags { mode: DYNAMIC }
   host_system_name: "armeabi-v7a"
   needsPic: true
   supports_gold_linker: false
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
index a91016c90e..5b6f44362c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -114,6 +114,15 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
   }
 
   @Test
+  public void testFilesToBuildWithoutDSO() throws Exception {
+    // This is like the preceding test, but with a toolchain that can't build '.so' files
+    useConfiguration("--compiler=compiler_no_dyn_linker");
+    ConfiguredTarget hello = getConfiguredTarget("//hello:hello");
+    Artifact archive = getBinArtifact("libhello.a", hello);
+    assertThat(getFilesToBuild(hello)).containsExactly(archive);
+  }
+
+  @Test
   public void testFilesToBuildWithInterfaceSharedObjects() throws Exception {
     useConfiguration("--interface_shared_objects");
     ConfiguredTarget hello = getConfiguredTarget("//hello:hello");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java
index a3214fe0cd..113fa2255f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java
@@ -88,6 +88,9 @@ public class CrosstoolConfigurationHelper {
         .setCompiler("gcc-4.3.1")
         .setAbiVersion("gcc-3.4")
         .setAbiLibcVersion("2.3.2")
+        // add a submessage that implies support for '.so' files
+        .addLinkingModeFlags(CrosstoolConfig.LinkingModeFlags.newBuilder()
+            .setMode(CrosstoolConfig.LinkingMode.DYNAMIC))
         .addCxxBuiltinIncludeDirectory("/include/directory");
     builder.addToolchain(toolchainBuilder);
     return builder.build();