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
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
index 59fb92b..29c577d 100644
--- 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 @@
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
index 54c0aca..a029a9d 100644
--- 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 @@
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
index 1fc345b..a9c598f 100644
--- 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 @@
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");
+ }
}
