Do not NPE crash when C++ actions are not configured in crosstool

Show meaningful message instead.

RELNOTES: None.
PiperOrigin-RevId: 161196096

4 files changed, 136 insertions, 34 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java
index 482e933f09..681855b48a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java
@@ -54,7 +54,8 @@ public class ActionTester {
      *
      * @param attributesToFlip
      */
-    Action generate(ImmutableSet<E> attributesToFlip) throws InterruptedException;
+    Action generate(ImmutableSet<E> attributesToFlip)
+        throws InterruptedException;
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
index 18e0b254f7..fae4347df9 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java
@@ -357,6 +357,21 @@ public abstract class MockCcSupport {
           + "   pattern: 'foo%{bad_variable}bar'"
           + "}";
 
+  /** An action_config for 'c++-compile action using DUMMY_TOOL that doesn't imply any features. */
+  public static final String INCOMPLETE_COMPILE_ACTION_CONFIG =
+      ""
+          + "action_config {"
+          + "   config_name: '"
+          + CppCompileAction.CPP_COMPILE
+          + "'"
+          + "   action_name: '"
+          + CppCompileAction.CPP_COMPILE
+          + "'"
+          + "   tool {"
+          + "      tool_path: 'DUMMY_TOOL'"
+          + "   }"
+          + "}";
+
   /**
    * An action_config for 'c++-module-codegen action using DUMMY_TOOL that doesn't imply any
    * features.
@@ -375,6 +390,24 @@ public abstract class MockCcSupport {
           + "   }"
           + "}";
 
+  /**
+   * An action_config for 'c++-module-compile action using DUMMY_TOOL that doesn't imply any
+   * features.
+   */
+  public static final String INCOMPLETE_MODULE_COMPILE_ACTION_CONFIG =
+      ""
+          + "action_config {"
+          + "   config_name: '"
+          + CppCompileAction.CPP_MODULE_COMPILE
+          + "'"
+          + "   action_name: '"
+          + CppCompileAction.CPP_MODULE_COMPILE
+          + "'"
+          + "   tool {"
+          + "      tool_path: 'DUMMY_TOOL'"
+          + "   }"
+          + "}";
+
   public static final String INCOMPLETE_EXECUTABLE_ACTION_CONFIG =
       ""
           + "action_config {"
@@ -468,6 +501,21 @@ public abstract class MockCcSupport {
           + "   }"
           + "}";
 
+  /** An action_config for clif-match action using DUMMY_TOOL that doesn't imply any features. */
+  public static final String INCOMPLETE_CLIF_MATCH_ACTION_CONFIG =
+      ""
+          + "action_config {"
+          + "   config_name: '"
+          + CppCompileAction.CLIF_MATCH
+          + "'"
+          + "   action_name: '"
+          + CppCompileAction.CLIF_MATCH
+          + "'"
+          + "   tool {"
+          + "      tool_path: 'DUMMY_TOOL'"
+          + "   }"
+          + "}";
+
   /** Filter to remove implicit dependencies of C/C++ rules. */
   private final Predicate<Label> ccLabelFilter =
       new Predicate<Label>() {
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 ee33a76561..e258d4eaaa 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
@@ -90,6 +90,39 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
     assertThat(target.getProvider(CppCompilationContext.class).getCppModuleMap()).isNull();
   }
 
+  @Test
+  public void testMisconfiguredCrosstoolRaisesErrorWhenLinking() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCrosstool(
+            mockToolsConfig,
+            MockCcSupport.NO_LEGACY_FEATURES_FEATURE,
+            MockCcSupport.INCOMPLETE_COMPILE_ACTION_CONFIG);
+    useConfiguration();
+
+    checkError(
+        "test",
+        "test",
+        "Expected action_config for 'c++-link-static-library' to be configured",
+        "cc_library(name = 'test', srcs = ['test.cc'])");
+  }
+
+  @Test
+  public void testMisconfiguredCrosstoolRaisesErrorWhenCompiling() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCrosstool(
+            mockToolsConfig,
+            MockCcSupport.NO_LEGACY_FEATURES_FEATURE,
+            MockCcSupport.INCOMPLETE_STATIC_LIBRARY_ACTION_CONFIG);
+    useConfiguration();
+
+    checkError(
+        "test",
+        "test",
+        "Expected action_config for 'c++-compile' to be configured",
+        "cc_library(name = 'test', srcs = ['test.cc'])");
+  }
 
   @Test
   public void testFilesToBuild() throws Exception {
@@ -709,8 +742,11 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
   public void testPicNotAvailableError() throws Exception {
     AnalysisMock.get()
         .ccSupport()
-        .setupCrosstool(mockToolsConfig,
-            "feature { name: 'no_legacy_features' }");
+        .setupCrosstool(
+            mockToolsConfig,
+            MockCcSupport.INCOMPLETE_STATIC_LIBRARY_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_COMPILE_ACTION_CONFIG,
+            MockCcSupport.NO_LEGACY_FEATURES_FEATURE);
     useConfiguration("--cpu=k8");
     writeSimpleCcLibrary();
     reporter.removeHandler(failFastHandler);
@@ -722,9 +758,14 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
   public void testToolchainWithoutPicForNoPicCompilation() throws Exception {
     AnalysisMock.get()
         .ccSupport()
-        .setupCrosstool(mockToolsConfig,
+        .setupCrosstool(
+            mockToolsConfig,
             "needsPic: false",
-            "feature { name: 'no_legacy_features' }");
+            MockCcSupport.INCOMPLETE_COMPILE_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_EXECUTABLE_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_DYNAMIC_LIBRARY_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_STATIC_LIBRARY_ACTION_CONFIG,
+            MockCcSupport.NO_LEGACY_FEATURES_FEATURE);
     useConfiguration();
     scratchConfiguredTarget("a", "a",
         "cc_binary(name='a', srcs=['a.cc'], deps=[':b'])",
@@ -735,9 +776,14 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
   public void testNoCppModuleMap() throws Exception {
     AnalysisMock.get()
         .ccSupport()
-        .setupCrosstool(mockToolsConfig,
-            "feature { name: 'no_legacy_features' }",
-            "feature { name: 'pic' }");
+        .setupCrosstool(
+            mockToolsConfig,
+            MockCcSupport.INCOMPLETE_COMPILE_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_EXECUTABLE_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_STATIC_LIBRARY_ACTION_CONFIG,
+            MockCcSupport.INCOMPLETE_DYNAMIC_LIBRARY_ACTION_CONFIG,
+            MockCcSupport.NO_LEGACY_FEATURES_FEATURE,
+            MockCcSupport.PIC_FEATURE);
     useConfiguration();
     writeSimpleCcLibrary();
     assertNoCppModuleMapAction("//module:map");
@@ -1002,28 +1048,36 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase {
 
   @Test
   public void testIncludePathsOutsideExecutionRoot() throws Exception {
-    checkError("root", "a",
+    checkError(
+        "root",
+        "a",
         "The include path 'd/../../somewhere' references a path outside of the execution root.",
         "cc_library(name='a', srcs=['a.cc'], copts=['-Id/../../somewhere'])");
   }
 
   @Test
   public void testAbsoluteIncludePathsOutsideExecutionRoot() throws Exception {
-    checkError("root", "a",
+    checkError(
+        "root",
+        "a",
         "The include path '/somewhere' references a path outside of the execution root.",
         "cc_library(name='a', srcs=['a.cc'], copts=['-I/somewhere'])");
   }
 
   @Test
   public void testSystemIncludePathsOutsideExecutionRoot() throws Exception {
-    checkError("root", "a",
+    checkError(
+        "root",
+        "a",
         "The include path '../system' references a path outside of the execution root.",
         "cc_library(name='a', srcs=['a.cc'], copts=['-isystem../system'])");
   }
 
   @Test
   public void testAbsoluteSystemIncludePathsOutsideExecutionRoot() throws Exception {
-    checkError("root", "a",
+    checkError(
+        "root",
+        "a",
         "The include path '/system' references a path outside of the execution root.",
         "cc_library(name='a', srcs=['a.cc'], copts=['-isystem/system'])");
   }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
index 55fd396b87..48debc1ea7 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.packages.util.MockCcSupport;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValue;
 import com.google.devtools.build.lib.rules.cpp.CppActionConfigs.CppPlatform;
@@ -105,19 +106,13 @@ public class CppLinkActionTest extends BuildViewTestCase {
   public void testToolchainFeatureFlags() throws Exception {
     FeatureConfiguration featureConfiguration =
         CcToolchainFeaturesTest.buildFeatures(
+                MockCcSupport.INCOMPLETE_EXECUTABLE_ACTION_CONFIG,
                 "feature {",
                 "   name: 'a'",
                 "   flag_set {",
                 "      action: '" + Link.LinkTargetType.EXECUTABLE.getActionName() + "'",
                 "      flag_group { flag: 'some_flag' }",
                 "   }",
-                "}",
-                "action_config {",
-                "   config_name: '" + Link.LinkTargetType.EXECUTABLE.getActionName() + "'",
-                "   action_name: '" + Link.LinkTargetType.EXECUTABLE.getActionName() + "'",
-                "   tool {",
-                "      tool_path: 'toolchain/mock_tool'",
-                "   }",
                 "}")
             .getFeatureConfiguration(
                 FeatureSpecification.create(
@@ -233,6 +228,7 @@ public class CppLinkActionTest extends BuildViewTestCase {
   public void testToolchainFeatureEnv() throws Exception {
     FeatureConfiguration featureConfiguration =
         CcToolchainFeaturesTest.buildFeatures(
+                MockCcSupport.INCOMPLETE_EXECUTABLE_ACTION_CONFIG,
                 "feature {",
                 "   name: 'a'",
                 "   env_set {",
@@ -241,7 +237,9 @@ public class CppLinkActionTest extends BuildViewTestCase {
                 "   }",
                 "}")
             .getFeatureConfiguration(
-                FeatureSpecification.create(ImmutableSet.of("a"), ImmutableSet.<String>of()));
+                FeatureSpecification.create(
+                    ImmutableSet.of(Link.LinkTargetType.EXECUTABLE.getActionName(), "a"),
+                    ImmutableSet.<String>of()));
 
     CppLinkAction linkAction =
         createLinkBuilder(
@@ -292,7 +290,8 @@ public class CppLinkActionTest extends BuildViewTestCase {
                         ? dynamicOutputFile
                         : staticOutputFile,
                     CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
-                    CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext)) {};
+                    CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext),
+                    featureConfiguration) {};
             builder.addCompilationInputs(
                 attributesToFlip.contains(NonStaticAttributes.COMPILATION_INPUTS)
                     ? ImmutableList.of(oFile)
@@ -312,7 +311,6 @@ public class CppLinkActionTest extends BuildViewTestCase {
                 attributesToFlip.contains(NonStaticAttributes.RUNTIME_SOLIB_DIR)
                     ? null
                     : PathFragment.create("so1"));
-            builder.setFeatureConfiguration(featureConfiguration);
 
             return builder.build();
           }
@@ -353,7 +351,8 @@ public class CppLinkActionTest extends BuildViewTestCase {
                         ? staticOutputFile
                         : dynamicOutputFile,
                     CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
-                    CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext)) {};
+                    CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext),
+                    featureConfiguration) {};
             builder.addCompilationInputs(
                 attributes.contains(StaticKeyAttributes.COMPILATION_INPUTS)
                     ? ImmutableList.of(oFile)
@@ -363,7 +362,6 @@ public class CppLinkActionTest extends BuildViewTestCase {
                     ? LinkTargetType.STATIC_LIBRARY
                     : LinkTargetType.DYNAMIC_LIBRARY);
             builder.setLibraryIdentifier("foo");
-            builder.setFeatureConfiguration(featureConfiguration);
             return builder.build();
           }
         });
@@ -385,7 +383,8 @@ public class CppLinkActionTest extends BuildViewTestCase {
             ruleContext,
             output,
             CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
-            CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext));
+            CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext),
+            new FeatureConfiguration());
     builder.setLinkType(LinkTargetType.STATIC_LIBRARY);
     assertThat(builder.canSplitCommandLine()).isTrue();
 
@@ -427,11 +426,11 @@ public class CppLinkActionTest extends BuildViewTestCase {
 
     CppLinkAction linkAction =
         createLinkBuilder(
-            Link.LinkTargetType.EXECUTABLE,
-            "dummyRuleContext/binary2",
-            objects.build(),
-            ImmutableList.<LibraryToLink>of(),
-            new FeatureConfiguration())
+                Link.LinkTargetType.EXECUTABLE,
+                "dummyRuleContext/binary2",
+                objects.build(),
+                ImmutableList.<LibraryToLink>of(),
+                getMockFeatureConfiguration())
             .setFake(true)
             .build();
 
@@ -477,7 +476,8 @@ public class CppLinkActionTest extends BuildViewTestCase {
                         .getBinDirectory(ruleContext.getRule().getRepository())),
                 ruleContext.getConfiguration(),
                 CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext),
-                CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext))
+                CppHelper.getFdoSupportUsingDefaultCcToolchainAttribute(ruleContext),
+                featureConfiguration)
             .addObjectFiles(nonLibraryInputs)
             .addLibraries(NestedSetBuilder.wrap(Order.LINK_ORDER, libraryInputs))
             .setLinkType(type)
@@ -485,8 +485,7 @@ public class CppLinkActionTest extends BuildViewTestCase {
             .setLinkStaticness(
                 type.staticness() == Staticness.STATIC
                     ? LinkStaticness.FULLY_STATIC
-                    : LinkStaticness.MOSTLY_STATIC)
-            .setFeatureConfiguration(featureConfiguration);
+                    : LinkStaticness.MOSTLY_STATIC);
     return builder;
   }
 
@@ -497,7 +496,7 @@ public class CppLinkActionTest extends BuildViewTestCase {
         output.getPathString(),
         ImmutableList.<Artifact>of(),
         ImmutableList.<LibraryToLink>of(),
-        new FeatureConfiguration());
+        getMockFeatureConfiguration());
   }
 
   public Artifact getOutputArtifact(String relpath) {