diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java')
| -rw-r--r-- | src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java | 215 |
1 files changed, 128 insertions, 87 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 80120f6129..322ed53c72 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 @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.analysis.actions.ExecutionInfoSpecifier; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.collect.CollectionUtils; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -188,8 +189,8 @@ public class CppCompileAction extends AbstractAction @VisibleForTesting final CppConfiguration cppConfiguration; private final FeatureConfiguration featureConfiguration; protected final Class<? extends CppCompileActionContext> actionContext; - protected final SpecialInputsHandler specialInputsHandler; - protected final CppSemantics cppSemantics; + private final SpecialInputsHandler specialInputsHandler; + private final CppSemantics semantics; /** * Identifier for the actual execution time behavior of the action. @@ -226,16 +227,14 @@ public class CppCompileAction extends AbstractAction * performed looking for inclusions. * @param usePic TODO(bazel-team): Add parameter description. * @param sourceLabel the label of the rule the source file is generated by - * @param mandatoryInputs any additional files that need to be present for the - * compilation to succeed, can be empty but not null, for example, extra sources for FDO. - * @param outputFile the object file that is written as result of the - * compilation, or the fake object for {@link FakeCppCompileAction}s - * @param dotdFile the .d file that is generated as a side-effect of - * compilation - * @param gcnoFile the coverage notes that are written in coverage mode, can - * be null - * @param dwoFile the .dwo output file where debug information is stored for Fission - * builds (null if Fission mode is disabled) + * @param mandatoryInputs any additional files that need to be present for the compilation to + * succeed, can be empty but not null, for example, extra sources for FDO. + * @param outputFile the object file that is written as result of the compilation, or the fake + * object for {@link FakeCppCompileAction}s + * @param dotdFile the .d file that is generated as a side-effect of compilation + * @param gcnoFile the coverage notes that are written in coverage mode, can be null + * @param dwoFile the .dwo output file where debug information is stored for Fission builds (null + * if Fission mode is disabled) * @param optionalSourceFile an additional optional source file (null if unneeded) * @param configuration the build configurations * @param cppConfiguration TODO(bazel-team): Add parameter description. @@ -248,10 +247,10 @@ public class CppCompileAction extends AbstractAction * @param additionalIncludeScannables list of additional artifacts to include-scan * @param actionClassId TODO(bazel-team): Add parameter description * @param executionRequirements out-of-band hints to be passed to the execution backend to signal - * platform requirements + * platform requirements * @param environment TODO(bazel-team): Add parameter description * @param actionName a string giving the name of this action for the purpose of toolchain - * evaluation + * evaluation * @param ruleContext The rule-context that produced this action */ protected CppCompileAction( @@ -286,7 +285,7 @@ public class CppCompileAction extends AbstractAction ImmutableMap<String, String> environment, String actionName, RuleContext ruleContext, - CppSemantics cppSemantics) { + CppSemantics semantics) { super( owner, createInputs( @@ -326,8 +325,8 @@ public class CppCompileAction extends AbstractAction // artifact and will definitely exist prior to this action execution. this.mandatoryInputs = mandatoryInputs; this.builtinIncludeFiles = CppHelper.getToolchain(ruleContext).getBuiltinIncludeFiles(); - this.cppSemantics = cppSemantics; - if (cppSemantics.needsIncludeValidation()) { + this.semantics = semantics; + if (semantics.needsIncludeValidation()) { verifyIncludePaths(ruleContext); } this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables); @@ -355,12 +354,6 @@ public class CppCompileAction extends AbstractAction continue; } - // One starting ../ is okay for getting to a sibling repository. - PathFragment originalInclude = include; - if (include.startsWith(new PathFragment(Label.EXTERNAL_PATH_PREFIX))) { - include = include.relativeTo(Label.EXTERNAL_PATH_PREFIX); - } - if (include.isAbsolute() || !PathFragment.EMPTY_FRAGMENT.getRelative(include).normalize().isNormalized()) { ruleContext.ruleError( @@ -469,6 +462,9 @@ public class CppCompileAction extends AbstractAction throws ActionExecutionException, InterruptedException { Executor executor = actionExecutionContext.getExecutor(); Collection<Artifact> initialResult; + if (!shouldScanIncludes) { + return null; + } try { initialResult = executor.getContext(actionContext) .findAdditionalInputs(this, actionExecutionContext); @@ -950,6 +946,105 @@ public class CppCompileAction extends AbstractAction } } + private DependencySet processDepset(Path execRoot, CppCompileActionContext.Reply reply) + throws IOException { + DotdFile dotdFile = getDotdFile(); + Preconditions.checkNotNull(dotdFile); + DependencySet depSet = new DependencySet(execRoot); + // artifact() is null if we are using in-memory .d files. We also want to prepare for the + // case where we expected an in-memory .d file, but we did not get an appropriate response. + // Perhaps we produced the file locally. + if (dotdFile.artifact() != null || reply == null) { + return depSet.read(dotdFile.getPath()); + } else { + // This is an in-memory .d file. + return depSet.process(reply.getContents()); + } + } + + /** + * Returns a collection with additional input artifacts relevant to the action by reading the + * dynamically-discovered dependency information from the .d file after the action has run. + * + * <p>Artifacts are considered inputs but not "mandatory" inputs. + * + * @param reply the reply from the compilation. + * @throws ActionExecutionException iff the .d is missing (when required), malformed, or has + * unresolvable included artifacts. + */ + @VisibleForTesting + @ThreadCompatible + public NestedSet<Artifact> discoverInputsFromDotdFiles( + Path execRoot, ArtifactResolver artifactResolver, Reply reply) + throws ActionExecutionException { + NestedSetBuilder<Artifact> inputs = NestedSetBuilder.stableOrder(); + if (getDotdFile() == null) { + return inputs.build(); + } + try { + // Read .d file. + DependencySet depSet = processDepset(execRoot, reply); + + // Determine prefixes of allowed absolute inclusions. + CppConfiguration toolchain = cppConfiguration; + List<Path> systemIncludePrefixes = new ArrayList<>(); + for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) { + if (includePath.isAbsolute()) { + systemIncludePrefixes.add(execRoot.getFileSystem().getPath(includePath)); + } + } + + // Check inclusions. + IncludeProblems problems = new IncludeProblems(); + Map<PathFragment, Artifact> allowedDerivedInputsMap = getAllowedDerivedInputsMap(); + for (Path execPath : depSet.getDependencies()) { + // Module .pcm files are generated and thus aren't declared inputs. + if (execPath.getBaseName().endsWith(".pcm")) { + continue; + } + PathFragment execPathFragment = execPath.asFragment(); + if (execPathFragment.isAbsolute()) { + // Absolute includes from system paths are ignored. + if (FileSystemUtils.startsWithAny(execPath, systemIncludePrefixes)) { + continue; + } + // Since gcc is given only relative paths on the command line, + // non-system include paths here should never be absolute. If they + // are, it's probably due to a non-hermetic #include, & we should stop + // the build with an error. + if (execPath.startsWith(execRoot)) { + execPathFragment = execPath.relativeTo(execRoot); // funky but tolerable path + } else { + problems.add(execPathFragment.getPathString()); + continue; + } + } + Artifact artifact = allowedDerivedInputsMap.get(execPathFragment); + if (artifact == null) { + artifact = artifactResolver.resolveSourceArtifact(execPathFragment, RepositoryName.MAIN); + } + if (artifact != null) { + inputs.add(artifact); + // In some cases, execution backends need extra files for each included file. Add them + // to the set of actual inputs. + inputs.addAll(specialInputsHandler.getInputsForIncludedFile(artifact, artifactResolver)); + } else { + // Abort if we see files that we can't resolve, likely caused by + // undeclared includes or illegal include constructs. + problems.add(execPathFragment.getPathString()); + } + } + //TODO(b/22551695): Remove in favor of seperate implementations. + if (semantics == null || semantics.needsIncludeValidation()) { + problems.assertProblemFree(this, getSourceFile()); + } + } catch (IOException e) { + // Some kind of IO or parse exception--wrap & rethrow it to stop the build. + throw new ActionExecutionException("error while parsing .d file", e, this, false); + } + return inputs.build(); + } + @Override public Iterable<Artifact> resolveInputsFromCache( ArtifactResolver artifactResolver, @@ -1001,7 +1096,7 @@ public class CppCompileAction extends AbstractAction } } - protected Map<PathFragment, Artifact> getAllowedDerivedInputsMap() { + private Map<PathFragment, Artifact> getAllowedDerivedInputsMap() { Map<PathFragment, Artifact> allowedDerivedInputMap = new HashMap<>(); addToMap(allowedDerivedInputMap, mandatoryInputs); addToMap(allowedDerivedInputMap, getDeclaredIncludeSrcs()); @@ -1124,10 +1219,9 @@ public class CppCompileAction extends AbstractAction // This is the .d file scanning part. IncludeScanningContext scanningContext = executor.getContext(IncludeScanningContext.class); - Path execRoot = executor.getExecRoot(); - NestedSet<Artifact> discoveredInputs = - discoverInputsFromDotdFiles(execRoot, scanningContext.getArtifactResolver(), reply); + discoverInputsFromDotdFiles( + executor.getExecRoot(), scanningContext.getArtifactResolver(), reply); reply = null; // Clear in-memory .d files early. // Post-execute "include scanning", which modifies the action inputs to match what the compile @@ -1145,67 +1239,14 @@ public class CppCompileAction extends AbstractAction updateActionInputs(discoveredInputs); } - // hdrs_check: This cannot be switched off, because doing so would allow for incorrect builds. - validateInclusions( - discoveredInputs, - actionExecutionContext.getArtifactExpander(), - executor.getEventHandler()); - } - - @VisibleForTesting - public NestedSet<Artifact> discoverInputsFromDotdFiles( - Path execRoot, ArtifactResolver artifactResolver, Reply reply) - throws ActionExecutionException { - if (getDotdFile() == null) { - return NestedSetBuilder.<Artifact>stableOrder().build(); - } - HeaderDiscovery.Builder discoveryBuilder = new HeaderDiscovery.Builder() - .setAction(this) - .setDotdFile(getDotdFile()) - .setSourceFile(getSourceFile()) - .setSpecialInputsHandler(specialInputsHandler) - .setDependencySet(processDepset(execRoot, reply)) - .setPermittedSystemIncludePrefixes(getPermittedSystemIncludePrefixes(execRoot)) - .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap()); - - if (cppSemantics.needsIncludeValidation()) { - discoveryBuilder.shouldValidateInclusions(); - } - - return discoveryBuilder - .build() - .discoverInputsFromDotdFiles(execRoot, artifactResolver); - } - - public DependencySet processDepset(Path execRoot, Reply reply) throws ActionExecutionException { - try { - DotdFile dotdFile = getDotdFile(); - Preconditions.checkNotNull(dotdFile); - DependencySet depSet = new DependencySet(execRoot); - // artifact() is null if we are using in-memory .d files. We also want to prepare for the - // case where we expected an in-memory .d file, but we did not get an appropriate response. - // Perhaps we produced the file locally. - if (dotdFile.artifact() != null || reply == null) { - return depSet.read(dotdFile.getPath()); - } else { - // This is an in-memory .d file. - return depSet.process(reply.getContents()); - } - } catch (IOException e) { - // Some kind of IO or parse exception--wrap & rethrow it to stop the build. - throw new ActionExecutionException("error while parsing .d file", e, this, false); - } - } - - public List<Path> getPermittedSystemIncludePrefixes(Path execRoot) { - CppConfiguration toolchain = cppConfiguration; - List<Path> systemIncludePrefixes = new ArrayList<>(); - for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) { - if (includePath.isAbsolute()) { - systemIncludePrefixes.add(execRoot.getFileSystem().getPath(includePath)); - } + // hdrs_check: Turning this off opens the door to incorrect builds. However, we allow it + // to accommodate the current behavior in the objc rules. + if (semantics == null || semantics.needsIncludeValidation()) { + validateInclusions( + discoveredInputs, + actionExecutionContext.getArtifactExpander(), + executor.getEventHandler()); } - return systemIncludePrefixes; } /** |