Make rules referencing paths outside of the execution root an error.

RELNOTES: Referencing a path outside the execution root is now an error.

--
MOS_MIGRATED_REVID=89129910

4 files changed, 45 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 1d49d676f3..793c31db66 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.actions.PackageRootResolver;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.extra.CppCompileInfo;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
+import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
 import com.google.devtools.build.lib.collect.CollectionUtils;
@@ -218,7 +219,8 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       IncludeResolver includeResolver,
       Iterable<IncludeScannable> lipoScannables,
       UUID actionClassId,
-      boolean usePic) {
+      boolean usePic,
+      RuleContext ruleContext) {
     // getInputs() method is overridden in this class so we pass a dummy empty
     // list to the AbstractAction constructor in place of a real input collection.
     super(owner,
@@ -250,6 +252,28 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     this.mandatoryInputs = mandatoryInputs;
     setInputs(createInputs(mandatoryInputs, context.getCompilationPrerequisites(),
         optionalSourceFile));
+    verifyIncludePaths(ruleContext);
+  }
+
+  /**
+   * Verifies that the include paths of this action are within the limits of the execution root.
+   */
+  private void verifyIncludePaths(RuleContext ruleContext) {
+    if (ruleContext == null) {
+      return;
+    }
+    // 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(getIncludeDirs(), getSystemIncludeDirs())) {
+      if (include.isAbsolute()
+          || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) {
+        ruleContext.ruleError("The include path '" + include
+            + "' references a path outside of the execution root.");
+      }
+    }
   }
 
   private static NestedSet<Artifact> createInputs(
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 ad88747061..f64e8a28c8 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
@@ -77,6 +77,7 @@ public class CppCompileActionBuilder {
   private Class<? extends CppCompileActionContext> actionContext;
   private CppConfiguration cppConfiguration;
   private ImmutableMap<Artifact, IncludeScannable> lipoScannableMap;
+  private RuleContext ruleContext = null;
   // New fields need to be added to the copy constructor.
 
   /**
@@ -94,6 +95,7 @@ public class CppCompileActionBuilder {
     this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder();
     this.pluginInputsBuilder = NestedSetBuilder.stableOrder();
     this.lipoScannableMap = getLipoScannableMap(ruleContext);
+    this.ruleContext = ruleContext;
 
     features.addAll(ruleContext.getFeatures());
   }
@@ -111,7 +113,11 @@ public class CppCompileActionBuilder {
 
   /**
    * Creates a builder for an owner that is not required to be rule.
+   * 
+   * <p>If errors are found when creating the {@code CppCompileAction}, builders constructed
+   * this way will throw a runtime exception.
    */
+  @VisibleForTesting
   public CppCompileActionBuilder(
       ActionOwner owner, AnalysisEnvironment analysisEnvironment, Artifact sourceFile,
       Label sourceLabel, BuildConfiguration configuration) {
@@ -159,6 +165,7 @@ public class CppCompileActionBuilder {
     this.fdoBuildStamp = other.fdoBuildStamp;
     this.usePic = other.usePic;
     this.lipoScannableMap = other.lipoScannableMap;
+    this.ruleContext = other.ruleContext;
   }
 
   public PathFragment getTempOutputFile() {
@@ -257,7 +264,7 @@ public class CppCompileActionBuilder {
           sourceFile, sourceLabel, realMandatoryInputsBuilder.build(), outputFile, tempOutputFile,
           dotdFile, configuration, cppConfiguration, context, ImmutableList.copyOf(copts),
           ImmutableList.copyOf(pluginOpts), getNocoptPredicate(nocopts),
-          extraSystemIncludePrefixes, fdoBuildStamp);
+          extraSystemIncludePrefixes, fdoBuildStamp, ruleContext);
     } else {
       NestedSet<Artifact> realMandatoryInputs = realMandatoryInputsBuilder.build();
 
@@ -270,7 +277,7 @@ public class CppCompileActionBuilder {
           getNocoptPredicate(nocopts),
           extraSystemIncludePrefixes, fdoBuildStamp,
           includeResolver, getLipoScannables(realMandatoryInputs), actionClassId,
-          usePic);
+          usePic, ruleContext);
     }
   }
   
@@ -366,8 +373,7 @@ public class CppCompileActionBuilder {
     return this;
   }
 
-  public CppCompileActionBuilder setDotdFile(PathFragment outputName, String extension,
-      RuleContext ruleContext) {
+  public CppCompileActionBuilder setDotdFile(PathFragment outputName, String extension) {
     if (configuration.getFragment(CppConfiguration.class).getInmemoryDotdFiles()) {
       // Just set the path, no artifact is constructed
       PathFragment file = FileSystemUtils.replaceExtension(outputName, extension);
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 ef0574ad63..004662da62 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
@@ -335,7 +335,7 @@ public final class CppModel {
       CppCompileActionBuilder builder) {
     builder
         .setOutputFile(ruleContext.getRelatedArtifact(outputName, ".h.processed"))
-        .setDotdFile(outputName, ".h.d", ruleContext)
+        .setDotdFile(outputName, ".h.d")
         // If we generate pic actions, we prefer the header actions to use the pic artifacts.
         .setPicMode(this.getGeneratePicActions());
     semantics.finalizeCompileActionBuilder(ruleContext, builder);
@@ -373,7 +373,7 @@ public final class CppModel {
           FileSystemUtils.replaceExtension(outputFile.getExecPath(), ".temp" + outputExtension);
       builder
           .setOutputFile(outputFile)
-          .setDotdFile(outputName, dependencyFileExtension, ruleContext)
+          .setDotdFile(outputName, dependencyFileExtension)
           .setTempOutputFile(tempOutputName);
       semantics.finalizeCompileActionBuilder(ruleContext, builder);
       CppCompileAction action = builder.build();
@@ -423,7 +423,7 @@ public final class CppModel {
       if (generateNoPicAction) {
         builder
             .setOutputFile(ruleContext.getRelatedArtifact(outputName, outputExtension))
-            .setDotdFile(outputName, dependencyFileExtension, ruleContext);
+            .setDotdFile(outputName, dependencyFileExtension);
         // Create non-PIC compile actions
         cppConfiguration.getFdoSupport().configureCompilation(builder, ruleContext, env,
             ruleContext.getLabel(), ccRelativeName, nocopts, /*usePic=*/false,
@@ -622,7 +622,7 @@ public final class CppModel {
     picBuilder
         .setPicMode(true)
         .setOutputFile(ruleContext.getRelatedArtifact(outputName, ".pic" + outputExtension))
-        .setDotdFile(outputName, ".pic" + dependencyFileExtension, ruleContext);
+        .setDotdFile(outputName, ".pic" + dependencyFileExtension);
     return picBuilder;
   }
 
@@ -650,14 +650,14 @@ public final class CppModel {
 
     dBuilder
         .setOutputFile(ruleContext.getRelatedArtifact(outputName, picExt + iExt))
-        .setDotdFile(outputName, picExt + iExt + ".d", ruleContext);
+        .setDotdFile(outputName, picExt + iExt + ".d");
     semantics.finalizeCompileActionBuilder(ruleContext, dBuilder);
     CppCompileAction dAction = dBuilder.build();
     ruleContext.registerAction(dAction);
 
     sdBuilder
         .setOutputFile(ruleContext.getRelatedArtifact(outputName, picExt + ".s"))
-        .setDotdFile(outputName, picExt + ".s.d", ruleContext);
+        .setDotdFile(outputName, picExt + ".s.d");
     semantics.finalizeCompileActionBuilder(ruleContext, sdBuilder);
     CppCompileAction sdAction = sdBuilder.build();
     ruleContext.registerAction(sdAction);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index 17fa62e609..ba87869994 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.ResourceSet;
+import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
@@ -74,7 +75,8 @@ public class FakeCppCompileAction extends CppCompileAction {
       ImmutableList<String> pluginOpts,
       Predicate<String> nocopts,
       ImmutableList<PathFragment> extraSystemIncludePrefixes,
-      @Nullable String fdoBuildStamp) {
+      @Nullable String fdoBuildStamp,
+      RuleContext ruleContext) {
     super(owner, features, featureConfiguration, sourceFile, sourceLabel, mandatoryInputs,
         outputFile, dotdFile, null, null, null,
         configuration, cppConfiguration,
@@ -88,7 +90,7 @@ public class FakeCppCompileAction extends CppCompileAction {
         CppCompilationContext.disallowUndeclaredHeaders(context), null, copts, pluginOpts, nocopts,
         extraSystemIncludePrefixes, fdoBuildStamp, VOID_INCLUDE_RESOLVER,
         ImmutableList.<IncludeScannable>of(),
-        GUID, /*usePic=*/false);
+        GUID, /*usePic=*/false, ruleContext);
     this.tempOutputFile = Preconditions.checkNotNull(tempOutputFile);
   }