C++: Fixes crash happening with invalid input

Fixes crash happening when a library with the wrong extension is passed to cc_common.create_library_to_link(). Fixes #7136 RELNOTES:none PiperOrigin-RevId: 232268610
bazelbuild · Feb 4, 2019 · 40c2c7b · 40c2c7b
1 parent 71bc38f
commit 40c2c7b
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.rules.cpp;
 
+import static com.google.common.base.StandardSystemProperty.LINE_SEPARATOR;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
@@ -358,17 +360,64 @@ public LibraryToLinkWrapper createLibraryLinkerInput(
     Artifact interfaceLibrary = nullIfNone(interfaceLibraryObject, Artifact.class);
 
     Artifact notNullArtifactForIdentifier = null;
+    StringBuilder extensionErrorsBuilder = new StringBuilder();
+    String extensionErrorMessage = "does not have any of the allowed extensions";
     if (staticLibrary != null) {
+      String filename = staticLibrary.getFilename();
+      if (!Link.ARCHIVE_FILETYPES.matches(filename)
+          && (!alwayslink || !Link.LINK_LIBRARY_FILETYPES.matches(filename))) {
+        String extensions = Link.ARCHIVE_FILETYPES.toString();
+        if (alwayslink) {
+          extensions += ", " + Link.LINK_LIBRARY_FILETYPES;
+        }
+        extensionErrorsBuilder.append(
+            String.format("'%s' %s %s", filename, extensionErrorMessage, extensions));
+        extensionErrorsBuilder.append(LINE_SEPARATOR.value());
+      }
       notNullArtifactForIdentifier = staticLibrary;
-    } else if (picStaticLibrary != null) {
+    }
+    if (picStaticLibrary != null) {
+      String filename = picStaticLibrary.getFilename();
+      if (!Link.ARCHIVE_FILETYPES.matches(filename)
+          && (!alwayslink || !Link.LINK_LIBRARY_FILETYPES.matches(filename))) {
+        String extensions = Link.ARCHIVE_FILETYPES.toString();
+        if (alwayslink) {
+          extensions += ", " + Link.LINK_LIBRARY_FILETYPES;
+        }
+        extensionErrorsBuilder.append(
+            String.format("'%s' %s %s", filename, extensionErrorMessage, extensions));
+        extensionErrorsBuilder.append(LINE_SEPARATOR.value());
+      }
       notNullArtifactForIdentifier = picStaticLibrary;
-    } else if (dynamicLibrary != null) {
+    }
+    if (dynamicLibrary != null) {
+      String filename = dynamicLibrary.getFilename();
+      if (!Link.ONLY_SHARED_LIBRARY_FILETYPES.matches(filename)) {
+        extensionErrorsBuilder.append(
+            String.format(
+                "'%s' %s %s", filename, extensionErrorMessage, Link.ONLY_SHARED_LIBRARY_FILETYPES));
+        extensionErrorsBuilder.append(LINE_SEPARATOR.value());
+      }
       notNullArtifactForIdentifier = dynamicLibrary;
-    } else if (interfaceLibrary != null) {
+    }
+    if (interfaceLibrary != null) {
+      String filename = interfaceLibrary.getFilename();
+      if (!Link.ONLY_INTERFACE_LIBRARY_FILETYPES.matches(filename)) {
+        extensionErrorsBuilder.append(
+            String.format(
+                "'%s' %s %s",
+                filename, extensionErrorMessage, Link.ONLY_INTERFACE_LIBRARY_FILETYPES));
+        extensionErrorsBuilder.append(LINE_SEPARATOR.value());
+      }
       notNullArtifactForIdentifier = interfaceLibrary;
-    } else {
+    }
+    if (notNullArtifactForIdentifier == null) {
       throw new EvalException(location, "Must pass at least one artifact");
     }
+    String extensionErrors = extensionErrorsBuilder.toString();
+    if (!extensionErrors.isEmpty()) {
+      throw new EvalException(location, extensionErrors);
+    }
 
     Artifact resolvedSymlinkDynamicLibrary = null;
     Artifact resolvedSymlinkInterfaceLibrary = null;

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
@@ -40,6 +40,12 @@ private Link() {} // uninstantiable
       CppFileTypes.VERSIONED_SHARED_LIBRARY,
       CppFileTypes.INTERFACE_SHARED_LIBRARY);
 
+  public static final FileTypeSet ONLY_SHARED_LIBRARY_FILETYPES =
+      FileTypeSet.of(CppFileTypes.SHARED_LIBRARY, CppFileTypes.VERSIONED_SHARED_LIBRARY);
+
+  public static final FileTypeSet ONLY_INTERFACE_LIBRARY_FILETYPES =
+      FileTypeSet.of(CppFileTypes.INTERFACE_SHARED_LIBRARY);
+
   public static final FileTypeSet ARCHIVE_LIBRARY_FILETYPES = FileTypeSet.of(
       CppFileTypes.ARCHIVE,
       CppFileTypes.PIC_ARCHIVE,

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
@@ -4676,4 +4676,34 @@ public void testIsToolchainResolutionEnabled_enabled() throws Exception {
 
     assertThat(toolchainResolutionEnabled()).isTrue();
   }
+
+  @Test
+  public void testWrongExtensionThrowsError() throws Exception {
+    setUpCcLinkingContextTest();
+    scratch.file(
+        "foo/BUILD",
+        "load('//tools/build_defs/cc:rule.bzl', 'crule')",
+        "cc_binary(name='bin',",
+        "   deps = [':a'],",
+        ")",
+        "crule(name='a',",
+        "   static_library = 'a.o',",
+        "   pic_static_library = 'a.pic.o',",
+        "   dynamic_library = 'a.ifso',",
+        "   interface_library = 'a.so',",
+        ")");
+    AssertionError e = assertThrows(AssertionError.class, () -> getConfiguredTarget("//foo:bin"));
+    assertThat(e)
+        .hasMessageThat()
+        .contains("'a.o' does not have any of the allowed extensions .a, .lib or .pic.a");
+    assertThat(e)
+        .hasMessageThat()
+        .contains("'a.pic.o' does not have any of the allowed extensions .a, .lib or .pic.a");
+    assertThat(e)
+        .hasMessageThat()
+        .contains("'a.ifso' does not have any of the allowed extensions .so, .dylib or .dll");
+    assertThat(e)
+        .hasMessageThat()
+        .contains("'a.so' does not have any of the allowed extensions .ifso, .tbd or .lib");
+  }
 }