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

Show meaningful message instead.

RELNOTES: None.
PiperOrigin-RevId: 161196096

6 files changed, 237 insertions, 155 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 41a2eb3c2d..49c4a7d18a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -39,6 +39,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.apple.Platform;
+import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper.Info;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
@@ -236,6 +237,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
         determineLinkerArguments(
             ruleContext,
             ccToolchain,
+            featureConfiguration,
             fdoSupport,
             common,
             precompiledFiles,
@@ -268,7 +270,6 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
     linkActionBuilder.setLinkType(linkType);
     linkActionBuilder.setLinkStaticness(linkStaticness);
     linkActionBuilder.setFake(fake);
-    linkActionBuilder.setFeatureConfiguration(featureConfiguration);
 
     if (CppLinkAction.enableSymbolsCounts(cppConfiguration, fake, linkType)) {
       linkActionBuilder.setSymbolCountsOutput(ruleContext.getBinArtifact(
@@ -450,10 +451,11 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
   private static CppLinkActionBuilder determineLinkerArguments(
       RuleContext context,
       CcToolchainProvider toolchain,
+      FeatureConfiguration featureConfiguration,
       FdoSupportProvider fdoSupport,
       CcCommon common,
       PrecompiledFiles precompiledFiles,
-      CcLibraryHelper.Info info,
+      Info info,
       ImmutableSet<Artifact> compilationPrerequisites,
       boolean fake,
       Artifact binary,
@@ -462,7 +464,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
       boolean linkCompileOutputSeparately)
       throws InterruptedException {
     CppLinkActionBuilder builder =
-        new CppLinkActionBuilder(context, binary, toolchain, fdoSupport)
+        new CppLinkActionBuilder(context, binary, toolchain, fdoSupport, featureConfiguration)
             .setCrosstoolInputs(toolchain.getLink())
             .addNonCodeInputs(compilationPrerequisites);
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index f5d21f486d..617460b208 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables;
 import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile;
@@ -43,6 +44,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.function.Consumer;
 import java.util.regex.Pattern;
 
 /**
@@ -90,11 +92,11 @@ public class CppCompileActionBuilder {
   // New fields need to be added to the copy constructor.
 
   /**
-   * Creates a builder from a rule. This also uses the configuration and
-   * artifact factory from the rule.
+   * Creates a builder from a rule. This also uses the configuration and artifact factory from the
+   * rule.
    */
-  public CppCompileActionBuilder(RuleContext ruleContext, Label sourceLabel,
-      CcToolchainProvider ccToolchain) {
+  public CppCompileActionBuilder(
+      RuleContext ruleContext, Label sourceLabel, CcToolchainProvider ccToolchain) {
     this(
         ruleContext.getActionOwner(),
         sourceLabel,
@@ -104,11 +106,12 @@ public class CppCompileActionBuilder {
         ccToolchain);
   }
 
-  /**
-   * Creates a builder from a rule and configuration.
-   */
-  public CppCompileActionBuilder(RuleContext ruleContext, Label sourceLabel,
-      CcToolchainProvider ccToolchain, BuildConfiguration configuration) {
+  /** Creates a builder from a rule and configuration. */
+  public CppCompileActionBuilder(
+      RuleContext ruleContext,
+      Label sourceLabel,
+      CcToolchainProvider ccToolchain,
+      BuildConfiguration configuration) {
     this(
         ruleContext.getActionOwner(),
         sourceLabel,
@@ -118,9 +121,7 @@ public class CppCompileActionBuilder {
         ccToolchain);
   }
 
-  /**
-   * Creates a builder from a rule and configuration.
-   */
+  /** Creates a builder from a rule and configuration. */
   private CppCompileActionBuilder(
       ActionOwner actionOwner,
       Label sourceLabel,
@@ -292,29 +293,51 @@ public class CppCompileActionBuilder {
   }
 
   /**
-   * Builds the action and performs some validations on the action.
+   * Builds the Action as configured and performs some validations on the action. Uses {@link
+   * RuleContext#throwWithRuleError(String)} to report errors. Prefer this method over {@link
+   * CppCompileActionBuilder#buildOrThrowIllegalStateException()} whenever possible (meaning
+   * whenever you have access to {@link RuleContext}).
    *
-   * <p>This method may be called multiple times to create multiple compile
-   * actions (usually after calling some setters to modify the generated
-   * action).
+   * <p>This method may be called multiple times to create multiple compile actions (usually after
+   * calling some setters to modify the generated action).
    */
-  public CppCompileAction buildAndValidate(RuleContext ruleContext) {
-    CppCompileAction action = build();
-    if (cppSemantics.needsIncludeValidation()) {
-      verifyActionIncludePaths(action, ruleContext);
+  public CppCompileAction buildOrThrowRuleError(RuleContext ruleContext) throws RuleErrorException {
+    List<String> errorMessages = new ArrayList<>();
+    CppCompileAction result =
+        buildAndVerify((String errorMessage) -> errorMessages.add(errorMessage));
+
+    if (!errorMessages.isEmpty()) {
+      for (String errorMessage : errorMessages) {
+        ruleContext.ruleError(errorMessage);
+      }
+
+      throw new RuleErrorException();
     }
-    return action;
+
+    return result;
   }
 
   /**
-   * Builds the Action as configured without validations. Users may want to call
-   * {@link #buildAndValidate} instead.
+   * Builds the Action as configured and performs some validations on the action. Throws {@link
+   * IllegalStateException} to report errors. Prefer {@link
+   * CppCompileActionBuilder#buildOrThrowRuleError(RuleContext)} over this method whenever possible
+   * (meaning whenever you have access to {@link RuleContext}).
    *
-   * <p>This method may be called multiple times to create multiple compile
-   * actions (usually after calling some setters to modify the generated
-   * action).
+   * <p>This method may be called multiple times to create multiple compile actions (usually after
+   * calling some setters to modify the generated action).
    */
-  public CppCompileAction build() {
+  public CppCompileAction buildOrThrowIllegalStateException() {
+    return buildAndVerify(
+        (String errorMessage) -> {
+          throw new IllegalStateException(errorMessage);
+        });
+  }
+
+  /**
+   * Builds the Action as configured and performs some validations on the action. Uses given {@link
+   * Consumer} to collect validation errors.
+   */
+  public CppCompileAction buildAndVerify(Consumer<String> errorCollector) {
     // This must be set either to false or true by CppSemantics, otherwise someone forgot to call
     // finalizeCompileActionBuilder on this builder.
     Preconditions.checkNotNull(shouldScanIncludes);
@@ -322,10 +345,10 @@ public class CppCompileActionBuilder {
         allowUsingHeaderModules
             && featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES);
 
-    // If the crosstool uses action_configs to configure cc compilation, collect execution info
-    // from there, otherwise, use no execution info.
-    // TODO(b/27903698): Assert that the crosstool has an action_config for this action.
-    if (featureConfiguration.actionIsConfigured(getActionName())) {
+    if (!featureConfiguration.actionIsConfigured(getActionName())) {
+      errorCollector.accept(
+          String.format("Expected action_config for '%s' to be configured", getActionName()));
+    } else {
       for (String executionRequirement :
           featureConfiguration.getToolForAction(getActionName()).getExecutionRequirements()) {
         executionInfo.put(executionRequirement, "");
@@ -356,72 +379,80 @@ public class CppCompileActionBuilder {
     NestedSet<Artifact> prunableInputs = prunableInputBuilder.build();
 
     // Copying the collections is needed to make the builder reusable.
+    CppCompileAction action;
     boolean fake = tempOutputFile != null;
     if (fake) {
-      return new FakeCppCompileAction(
-          owner,
-          allInputs,
-          ImmutableList.copyOf(features),
-          featureConfiguration,
-          variables,
-          sourceFile,
-          shouldScanIncludes,
-          shouldPruneModules(),
-          usePic,
-          useHeaderModules,
-          sourceLabel,
-          realMandatoryInputs,
-          prunableInputs,
-          outputFile,
-          tempOutputFile,
-          dotdFile,
-          localShellEnvironment,
-          cppConfiguration,
-          context,
-          actionContext,
-          ImmutableList.copyOf(copts),
-          getNocoptPredicate(nocopts),
-          getLipoScannables(realMandatoryInputs),
-          cppSemantics,
-          ccToolchain,
-          ImmutableMap.copyOf(executionInfo));
+      action =
+          new FakeCppCompileAction(
+              owner,
+              allInputs,
+              ImmutableList.copyOf(features),
+              featureConfiguration,
+              variables,
+              sourceFile,
+              shouldScanIncludes,
+              shouldPruneModules(),
+              usePic,
+              useHeaderModules,
+              sourceLabel,
+              realMandatoryInputs,
+              prunableInputs,
+              outputFile,
+              tempOutputFile,
+              dotdFile,
+              localShellEnvironment,
+              cppConfiguration,
+              context,
+              actionContext,
+              ImmutableList.copyOf(copts),
+              getNocoptPredicate(nocopts),
+              getLipoScannables(realMandatoryInputs),
+              cppSemantics,
+              ccToolchain,
+              ImmutableMap.copyOf(executionInfo));
     } else {
-      return new CppCompileAction(
-          owner,
-          allInputs,
-          ImmutableList.copyOf(features),
-          featureConfiguration,
-          variables,
-          sourceFile,
-          shouldScanIncludes,
-          shouldPruneModules(),
-          usePic,
-          useHeaderModules,
-          sourceLabel,
-          realMandatoryInputs,
-          prunableInputs,
-          outputFile,
-          dotdFile,
-          gcnoFile,
-          dwoFile,
-          ltoIndexingFile,
-          optionalSourceFile,
-          localShellEnvironment,
-          cppConfiguration,
-          context,
-          actionContext,
-          ImmutableList.copyOf(copts),
-          getNocoptPredicate(nocopts),
-          specialInputsHandler,
-          getLipoScannables(realMandatoryInputs),
-          additionalIncludeFiles.build(),
-          actionClassId,
-          ImmutableMap.copyOf(executionInfo),
-          ImmutableMap.copyOf(environment),
-          getActionName(),
-          cppSemantics,
-          ccToolchain);
+      action =
+          new CppCompileAction(
+              owner,
+              allInputs,
+              ImmutableList.copyOf(features),
+              featureConfiguration,
+              variables,
+              sourceFile,
+              shouldScanIncludes,
+              shouldPruneModules(),
+              usePic,
+              useHeaderModules,
+              sourceLabel,
+              realMandatoryInputs,
+              prunableInputs,
+              outputFile,
+              dotdFile,
+              gcnoFile,
+              dwoFile,
+              ltoIndexingFile,
+              optionalSourceFile,
+              localShellEnvironment,
+              cppConfiguration,
+              context,
+              actionContext,
+              ImmutableList.copyOf(copts),
+              getNocoptPredicate(nocopts),
+              specialInputsHandler,
+              getLipoScannables(realMandatoryInputs),
+              additionalIncludeFiles.build(),
+              actionClassId,
+              ImmutableMap.copyOf(executionInfo),
+              ImmutableMap.copyOf(environment),
+              getActionName(),
+              cppSemantics,
+              ccToolchain);
     }
+
+    if (cppSemantics.needsIncludeValidation()) {
+      verifyActionIncludePaths(action, errorCollector);
+    }
+    return action;
   }
 
   /**
@@ -461,27 +492,30 @@ public class CppCompileActionBuilder {
     return cppConfiguration.getPruneCppModules() && shouldScanIncludes && useHeaderModules();
   }
 
-  private static void verifyActionIncludePaths(CppCompileAction action, RuleContext ruleContext) {
+  private void verifyActionIncludePaths(CppCompileAction action, Consumer<String> errorReporter) {
     Iterable<PathFragment> ignoredDirs = action.getValidationIgnoredDirs();
     // We currently do not check the output of:
     // - getQuoteIncludeDirs(): those only come from includes attributes, and are checked in
     //   CcCommon.getIncludeDirsFromIncludesAttribute().
     // - getBuiltinIncludeDirs(): while in practice this doesn't happen, bazel can be configured
     //   to use an absolute system root, in which case the builtin include dirs might be absolute.
-    for (PathFragment include :
-        Iterables.concat(action.getIncludeDirs(), action.getSystemIncludeDirs())) {
-      // Ignore headers from built-in include directories.
-      if (FileSystemUtils.startsWithAny(include, ignoredDirs)) {
+
+    Iterable<PathFragment> includePathsToVerify =
+        Iterables.concat(action.getIncludeDirs(), action.getSystemIncludeDirs());
+    for (PathFragment includePath : includePathsToVerify) {
+      if (FileSystemUtils.startsWithAny(includePath, ignoredDirs)) {
         continue;
       }
       // One starting ../ is okay for getting to a sibling repository.
-      if (include.startsWith(Label.EXTERNAL_PATH_PREFIX)) {
-        include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX);
+      if (includePath.startsWith(Label.EXTERNAL_PATH_PREFIX)) {
+        includePath = includePath.relativeTo(Label.EXTERNAL_PATH_PREFIX);
       }
-      if (include.isAbsolute()
-          || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) {
-        ruleContext.ruleError(
-            "The include path '" + include + "' references a path outside of the execution root.");
+      if (includePath.isAbsolute()
+          || !PathFragment.EMPTY_FRAGMENT.getRelative(includePath).normalize().isNormalized()) {
+        errorReporter.accept(
+            String.format(
+                "The include path '%s' references a path outside of the execution root.",
+                includePath));
       }
     }
   }
@@ -696,4 +730,5 @@ public class CppCompileActionBuilder {
   public CcToolchainProvider getToolchain() {
     return ccToolchain;
   }
+
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
index ea0697cd67..8fad25b074 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.cpp;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -28,6 +29,8 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * An {@link ActionTemplate} that expands into {@link CppCompileAction}s at execution time.
@@ -73,7 +76,8 @@ public final class CppCompileActionTemplate implements ActionTemplate<CppCompile
 
   @Override
   public Iterable<CppCompileAction> generateActionForInputArtifacts(
-      Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner) {
+      Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner)
+      throws ActionTemplateExpansionException {
     ImmutableList.Builder<CppCompileAction> expandedActions = new ImmutableList.Builder<>();
     for (TreeFileArtifact inputTreeFileArtifact : inputTreeFileArtifacts) {
       String outputName = outputTreeFileArtifactName(inputTreeFileArtifact);
@@ -89,7 +93,8 @@ public final class CppCompileActionTemplate implements ActionTemplate<CppCompile
   }
 
   private CppCompileAction createAction(
-      Artifact sourceTreeFileArtifact, Artifact outputTreeFileArtifact) {
+      Artifact sourceTreeFileArtifact, Artifact outputTreeFileArtifact)
+      throws ActionTemplateExpansionException {
     CppCompileActionBuilder builder = new CppCompileActionBuilder(cppCompileActionBuilder);
     builder.setSourceFile(sourceTreeFileArtifact);
     builder.setOutputs(outputTreeFileArtifact, null);
@@ -105,7 +110,14 @@ public final class CppCompileActionTemplate implements ActionTemplate<CppCompile
 
     builder.setVariables(buildVariables.build());
 
-    return builder.build();
+    List<String> errors = new ArrayList<>();
+    CppCompileAction result =
+        builder.buildAndVerify((String errorMessage) -> errors.add(errorMessage));
+    if (!errors.isEmpty()) {
+      throw new ActionTemplateExpansionException(Joiner.on(".\n").join(errors));
+    }
+
+    return result;
   }
 
   private String outputTreeFileArtifactName(TreeFileArtifact inputTreeFileArtifact) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 8dc16d0329..2d8ccabcd3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -218,14 +218,16 @@ public class CppLinkActionBuilder {
       RuleContext ruleContext,
       Artifact output,
       CcToolchainProvider toolchain,
-      FdoSupportProvider fdoSupport) {
+      FdoSupportProvider fdoSupport,
+      FeatureConfiguration featureConfiguration) {
     this(
         ruleContext,
         output,
         ruleContext.getConfiguration(),
         ruleContext.getAnalysisEnvironment(),
         toolchain,
-        fdoSupport);
+        fdoSupport,
+        featureConfiguration);
   }
 
   /**
@@ -242,9 +244,16 @@ public class CppLinkActionBuilder {
       Artifact output,
       BuildConfiguration configuration,
       CcToolchainProvider toolchain,
-      FdoSupportProvider fdoSupport) {
-    this(ruleContext, output, configuration, ruleContext.getAnalysisEnvironment(), toolchain,
-        fdoSupport);
+      FdoSupportProvider fdoSupport,
+      FeatureConfiguration featureConfiguration) {
+    this(
+        ruleContext,
+        output,
+        configuration,
+        ruleContext.getAnalysisEnvironment(),
+        toolchain,
+        fdoSupport,
+        featureConfiguration);
   }
 
   /**
@@ -263,7 +272,8 @@ public class CppLinkActionBuilder {
       BuildConfiguration configuration,
       AnalysisEnvironment analysisEnvironment,
       CcToolchainProvider toolchain,
-      FdoSupportProvider fdoSupport) {
+      FdoSupportProvider fdoSupport,
+      FeatureConfiguration featureConfiguration) {
     this.ruleContext = ruleContext;
     this.analysisEnvironment = Preconditions.checkNotNull(analysisEnvironment);
     this.output = Preconditions.checkNotNull(output);
@@ -274,6 +284,7 @@ public class CppLinkActionBuilder {
     if (cppConfiguration.supportsEmbeddedRuntimes() && toolchain != null) {
       runtimeSolibDir = toolchain.getDynamicRuntimeSolibDir();
     }
+    this.featureConfiguration = featureConfiguration;
   }
 
   /**
@@ -293,7 +304,8 @@ public class CppLinkActionBuilder {
       Context linkContext,
       BuildConfiguration configuration,
       CcToolchainProvider toolchain,
-      FdoSupportProvider fdoSupport) {
+      FdoSupportProvider fdoSupport,
+      FeatureConfiguration featureConfiguration) {
     // These Builder-only fields get set in the constructor:
     //   ruleContext, analysisEnvironment, outputPath, configuration, runtimeSolibDir
     this(
@@ -302,7 +314,8 @@ public class CppLinkActionBuilder {
         configuration,
         ruleContext.getAnalysisEnvironment(),
         toolchain,
-        fdoSupport);
+        fdoSupport,
+        featureConfiguration);
     Preconditions.checkNotNull(linkContext);
 
     // All linkContext fields should be transferred to this Builder.
@@ -561,6 +574,12 @@ public class CppLinkActionBuilder {
           "Interface output can only be used " + "with non-fake DYNAMIC_LIBRARY targets");
     }
 
+    if (!featureConfiguration.actionIsConfigured(linkType.getActionName())) {
+      ruleContext.ruleError(
+          String.format(
+              "Expected action_config for '%s' to be configured", linkType.getActionName()));
+    }
+
     final ImmutableList<Artifact> buildInfoHeaderArtifacts =
         !linkstamps.isEmpty()
             ? analysisEnvironment.getBuildInfo(ruleContext, CppBuildInfo.KEY, configuration)
@@ -985,12 +1004,6 @@ public class CppLinkActionBuilder {
     return this;
   }
 
-  /** Sets the feature configuration for the action. */
-  public CppLinkActionBuilder setFeatureConfiguration(FeatureConfiguration featureConfiguration) {
-    this.featureConfiguration = featureConfiguration;
-    return this;
-  }
-
   /**
    * This is the LTO indexing step, rather than the real link.
    *
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index e87ab7b666..e8b0e2690e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -582,7 +582,7 @@ public final class CppModel {
    * file. It takes into account LIPO, fake-ness, coverage, and PIC, in addition to using the
    * settings specified on the current object. This method should only be called once.
    */
-  public CcCompilationOutputs createCcCompileActions() {
+  public CcCompilationOutputs createCcCompileActions() throws RuleErrorException {
     CcCompilationOutputs.Builder result = new CcCompilationOutputs.Builder();
     Preconditions.checkNotNull(context);
     AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
@@ -670,8 +670,13 @@ public final class CppModel {
     return compilationOutputs;
   }
 
-  private void createHeaderAction(String outputName, Builder result, AnalysisEnvironment env,
-      CppCompileActionBuilder builder, boolean generateDotd) {
+  private void createHeaderAction(
+      String outputName,
+      Builder result,
+      AnalysisEnvironment env,
+      CppCompileActionBuilder builder,
+      boolean generateDotd)
+      throws RuleErrorException {
     String outputNameBase = CppHelper.getArtifactNameForCategory(
         ruleContext, ccToolchain, ArtifactCategory.GENERATED_HEADER, outputName);
 
@@ -691,13 +696,14 @@ public final class CppModel {
         ImmutableMap.<String, String>of());
     semantics.finalizeCompileActionBuilder(
         ruleContext, builder, featureConfiguration.getFeatureSpecification());
-    CppCompileAction compileAction = builder.buildAndValidate(ruleContext);
+    CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
     env.registerAction(compileAction);
     Artifact tokenFile = compileAction.getOutputFile();
     result.addHeaderTokenFile(tokenFile);
   }
 
-  private void createModuleCodegenAction(CcCompilationOutputs.Builder result, Artifact module) {
+  private void createModuleCodegenAction(CcCompilationOutputs.Builder result, Artifact module)
+      throws RuleErrorException {
     if (fake) {
       // We can't currently foresee a situation where we'd want nocompile tests for module codegen.
       // If we find one, support needs to be added here.
@@ -752,7 +758,7 @@ public final class CppModel {
 
     semantics.finalizeCompileActionBuilder(
         ruleContext, builder, featureConfiguration.getFeatureSpecification());
-    CppCompileAction compileAction = builder.build();
+    CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
     AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
     env.registerAction(compileAction);
     Artifact objectFile = compileAction.getOutputFile();
@@ -764,7 +770,7 @@ public final class CppModel {
   }
 
   private Collection<Artifact> createModuleAction(
-      CcCompilationOutputs.Builder result, CppModuleMap cppModuleMap) {
+      CcCompilationOutputs.Builder result, CppModuleMap cppModuleMap) throws RuleErrorException {
     AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
     Label moduleMapLabel = Label.parseAbsoluteUnchecked(context.getCppModuleMap().getName());
     Artifact moduleMapArtifact = cppModuleMap.getArtifact();
@@ -792,7 +798,8 @@ public final class CppModel {
   }
 
   private void createClifMatchAction(
-      String outputName, Builder result, AnalysisEnvironment env, CppCompileActionBuilder builder) {
+      String outputName, Builder result, AnalysisEnvironment env, CppCompileActionBuilder builder)
+      throws RuleErrorException {
     builder
         .setOutputs(ruleContext, ArtifactCategory.CLIF_OUTPUT_PROTO, outputName, false)
         .setPicMode(false)
@@ -812,7 +819,7 @@ public final class CppModel {
         /*sourceSpecificBuildVariables=*/ ImmutableMap.<String, String>of());
     semantics.finalizeCompileActionBuilder(
         ruleContext, builder, featureConfiguration.getFeatureSpecification());
-    CppCompileAction compileAction = builder.buildAndValidate(ruleContext);
+    CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
     env.registerAction(compileAction);
     Artifact tokenFile = compileAction.getOutputFile();
     result.addHeaderTokenFile(tokenFile);
@@ -830,7 +837,8 @@ public final class CppModel {
       boolean enableCoverage,
       boolean generateDwo,
       boolean generateDotd,
-      Map<String, String> sourceSpecificBuildVariables) {
+      Map<String, String> sourceSpecificBuildVariables)
+      throws RuleErrorException {
     ImmutableList.Builder<Artifact> directOutputs = new ImmutableList.Builder<>();
     PathFragment ccRelativeName = semantics.getEffectiveSourcePath(sourceArtifact);
     if (cppConfiguration.isLipoOptimization()) {
@@ -892,7 +900,7 @@ public final class CppModel {
 
         semantics.finalizeCompileActionBuilder(
             ruleContext, picBuilder, featureConfiguration.getFeatureSpecification());
-        CppCompileAction picAction = picBuilder.buildAndValidate(ruleContext);
+        CppCompileAction picAction = picBuilder.buildOrThrowRuleError(ruleContext);
         env.registerAction(picAction);
         directOutputs.add(picAction.getOutputFile());
         if (addObject) {
@@ -959,7 +967,7 @@ public final class CppModel {
 
         semantics.finalizeCompileActionBuilder(
             ruleContext, builder, featureConfiguration.getFeatureSpecification());
-        CppCompileAction compileAction = builder.buildAndValidate(ruleContext);
+        CppCompileAction compileAction = builder.buildOrThrowRuleError(ruleContext);
         env.registerAction(compileAction);
         Artifact objectFile = compileAction.getOutputFile();
         directOutputs.add(objectFile);
@@ -1000,6 +1008,7 @@ public final class CppModel {
         source.getBuildVariables());
     semantics.finalizeCompileActionBuilder(
         ruleContext, builder, featureConfiguration.getFeatureSpecification());
+    // Make sure this builder doesn't reference ruleContext outside of analysis phase.
     CppCompileActionTemplate actionTemplate = new CppCompileActionTemplate(
         sourceArtifact,
         outputFiles,
@@ -1019,10 +1028,18 @@ public final class CppModel {
         : base;
   }
 
-  private void createFakeSourceAction(String outputName, CcCompilationOutputs.Builder result,
-      AnalysisEnvironment env, CppCompileActionBuilder builder,
-      ArtifactCategory outputCategory, boolean addObject, PathFragment ccRelativeName,
-      PathFragment execPath, boolean usePic, boolean generateDotd) {
+  private void createFakeSourceAction(
+      String outputName,
+      CcCompilationOutputs.Builder result,
+      AnalysisEnvironment env,
+      CppCompileActionBuilder builder,
+      ArtifactCategory outputCategory,
+      boolean addObject,
+      PathFragment ccRelativeName,
+      PathFragment execPath,
+      boolean usePic,
+      boolean generateDotd)
+      throws RuleErrorException {
     String outputNameBase = getOutputNameBaseWith(outputName, usePic);
     String tempOutputName = ruleContext.getConfiguration().getBinFragment()
         .getRelative(CppHelper.getObjDirectory(ruleContext.getLabel()))
@@ -1047,7 +1064,7 @@ public final class CppModel {
         ImmutableMap.<String, String>of());
     semantics.finalizeCompileActionBuilder(
         ruleContext, builder, featureConfiguration.getFeatureSpecification());
-    CppCompileAction action = builder.buildAndValidate(ruleContext);
+    CppCompileAction action = builder.buildOrThrowRuleError(ruleContext);
     env.registerAction(action);
     if (addObject) {
       if (usePic) {
@@ -1171,7 +1188,6 @@ public final class CppModel {
             .addActionInputs(linkActionInputs)
             .setLibraryIdentifier(libraryIdentifier)
             .addVariablesExtensions(variablesExtensions)
-            .setFeatureConfiguration(featureConfiguration)
             .build();
     env.registerAction(maybePicAction);
     if (linkType != LinkTargetType.EXECUTABLE) {
@@ -1198,7 +1214,6 @@ public final class CppModel {
               .addActionInputs(linkActionInputs)
               .setLibraryIdentifier(libraryIdentifier)
               .addVariablesExtensions(variablesExtensions)
-              .setFeatureConfiguration(featureConfiguration)
               .build();
       env.registerAction(picAction);
       if (linkType != LinkTargetType.EXECUTABLE) {
@@ -1260,7 +1275,6 @@ public final class CppModel {
                 ArtifactCategory.DYNAMIC_LIBRARY,
                 ccToolchain.getDynamicRuntimeLinkMiddleman(),
                 ccToolchain.getDynamicRuntimeLinkInputs())
-            .setFeatureConfiguration(featureConfiguration)
             .addVariablesExtensions(variablesExtensions);
 
     if (!ccOutputs.getLtoBitcodeFiles().isEmpty()
@@ -1314,7 +1328,8 @@ public final class CppModel {
   }
 
   private CppLinkActionBuilder newLinkActionBuilder(Artifact outputArtifact) {
-    return new CppLinkActionBuilder(ruleContext, outputArtifact, ccToolchain, fdoSupport)
+    return new CppLinkActionBuilder(
+            ruleContext, outputArtifact, ccToolchain, fdoSupport, featureConfiguration)
         .setCrosstoolInputs(ccToolchain.getLink())
         .addNonCodeInputs(context.getTransitiveCompilationPrerequisites());
   }
@@ -1347,12 +1362,15 @@ public final class CppModel {
     return picBuilder;
   }
 
-  /**
-   * Create the actions for "--save_temps".
-   */
-  private ImmutableList<Artifact> createTempsActions(Artifact source, String outputName,
-      CppCompileActionBuilder builder, boolean usePic, boolean generateDotd,
-      PathFragment ccRelativeName) {
+  /** Create the actions for "--save_temps". */
+  private ImmutableList<Artifact> createTempsActions(
+      Artifact source,
+      String outputName,
+      CppCompileActionBuilder builder,
+      boolean usePic,
+      boolean generateDotd,
+      PathFragment ccRelativeName)
+      throws RuleErrorException {
     if (!cppConfiguration.getSaveTemps()) {
       return ImmutableList.of();
     }
@@ -1384,7 +1402,7 @@ public final class CppModel {
         ImmutableMap.<String, String>of());
     semantics.finalizeCompileActionBuilder(
         ruleContext, dBuilder, featureConfiguration.getFeatureSpecification());
-    CppCompileAction dAction = dBuilder.buildAndValidate(ruleContext);
+    CppCompileAction dAction = dBuilder.buildOrThrowRuleError(ruleContext);
     ruleContext.registerAction(dAction);
 
     CppCompileActionBuilder sdBuilder = new CppCompileActionBuilder(builder);
@@ -1402,7 +1420,7 @@ public final class CppModel {
         ImmutableMap.<String, String>of());
     semantics.finalizeCompileActionBuilder(
         ruleContext, sdBuilder, featureConfiguration.getFeatureSpecification());
-    CppCompileAction sdAction = sdBuilder.buildAndValidate(ruleContext);
+    CppCompileAction sdAction = sdBuilder.buildOrThrowRuleError(ruleContext);
     ruleContext.registerAction(sdAction);
 
     return ImmutableList.of(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index a904db923b..2b0e3e3c7e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -385,6 +385,9 @@ public final class LinkCommandLine extends CommandLine {
     if (forcedToolPath != null) {
       argv.add(forcedToolPath);
     } else {
+      Preconditions.checkArgument(
+          featureConfiguration.actionIsConfigured(actionName),
+          String.format("Expected action_config for '%s' to be configured", actionName));
       argv.add(
           featureConfiguration
               .getToolForAction(linkTargetType.getActionName())
@@ -657,8 +660,7 @@ public final class LinkCommandLine extends CommandLine {
       if (linkstampCompileOptions.isEmpty()) {
         actualLinkstampCompileOptions = DEFAULT_LINKSTAMP_OPTIONS;
       } else {
-        actualLinkstampCompileOptions =
-            ImmutableList.copyOf(
+        actualLinkstampCompileOptions = ImmutableList.copyOf(
                 Iterables.concat(DEFAULT_LINKSTAMP_OPTIONS, linkstampCompileOptions));
       }