Use pic mode for cc_fake_binary if pic actions are used for binaries.
To allow the commands in the cc_fake_binary to work with modules (which is
a precondition to writing nocompile tests for modules), we need to create
header modules in the same pic/nopic mode as the rest of the commandline
indicates.
There are two ways to resolve this problem:
a) Make cc_fake_binary use the same pic mode as other binaries.
b) Make sure fake compile actions get all their inputs in non-pic mode.
This patch proposes solution a), as that makes sure cc_fake_binary is as close
to the actual compilation going on as possible; for example, if we decide to
always use pic in the future, this will simply work; additionally, we will
currently get more test coverage through different compilation modes.
RELNOTES: 'cc_fake_binary' uses the same pic/nopic mode as other binaries.
--
MOS_MIGRATED_REVID=99902738
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 4be3a57..815af70 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
@@ -359,7 +359,7 @@
.addNonLibraryInputs(compilationPrerequisites);
// Determine the object files to link in.
- boolean usePic = CppHelper.usePic(context, !isLinkShared(context)) && !fake;
+ boolean usePic = CppHelper.usePic(context, !isLinkShared(context));
Iterable<Artifact> compiledObjectFiles = compilationOutputs.getObjectFiles(usePic);
if (fake) {
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 41ebc91..a943057 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
@@ -263,7 +263,8 @@
variables, sourceFile, sourceLabel, realMandatoryInputsBuilder.build(), outputFile,
tempOutputFile, dotdFile, configuration, cppConfiguration, context, actionContext,
ImmutableList.copyOf(copts), ImmutableList.copyOf(pluginOpts),
- getNocoptPredicate(nocopts), extraSystemIncludePrefixes, fdoBuildStamp, ruleContext);
+ getNocoptPredicate(nocopts), extraSystemIncludePrefixes, fdoBuildStamp, ruleContext,
+ usePic);
} else {
NestedSet<Artifact> realMandatoryInputs = realMandatoryInputsBuilder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
index ceeff8c..cc0caa1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java
@@ -266,7 +266,8 @@
// Concatenate all the (fake) .o files into the result.
for (LinkerInput linkerInput : getLinkCommandLine().getLinkerInputs()) {
Artifact objectFile = linkerInput.getArtifact();
- if (CppFileTypes.OBJECT_FILE.matches(objectFile.getFilename())
+ if ((CppFileTypes.OBJECT_FILE.matches(objectFile.getFilename())
+ || CppFileTypes.PIC_OBJECT_FILE.matches(objectFile.getFilename()))
&& linkerInput.isFake()) {
s.append(FileSystemUtils.readContentAsLatin1(objectFile.getPath())); // (IOException)
}
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 0556819..5282dc1 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
@@ -437,31 +437,15 @@
builder.setContext(CppCompilationContext.mergeForLipo(lipoProvider.getLipoContext(),
context));
}
+ boolean generatePicAction = getGeneratePicActions();
+ // If we always need pic for everything, then don't bother to create a no-pic action.
+ boolean generateNoPicAction = getGenerateNoPicActions();
+ Preconditions.checkState(generatePicAction || generateNoPicAction);
if (fake) {
- // For cc_fake_binary, we only create a single fake compile action. It's
- // not necessary to use -fPIC for negative compilation tests, and using
- // .pic.o files in cc_fake_binary would break existing uses of
- // cc_fake_binary.
- Artifact outputFile = ruleContext.getRelatedArtifact(outputName, outputExtension);
- PathFragment tempOutputName =
- FileSystemUtils.replaceExtension(outputFile.getExecPath(), ".temp" + outputExtension);
- builder
- .setOutputFile(outputFile)
- .setDotdFile(outputName, dependencyFileExtension)
- .setTempOutputFile(tempOutputName);
- setupBuildVariables(builder, getGeneratePicActions(), ccRelativeName);
- semantics.finalizeCompileActionBuilder(ruleContext, builder);
- CppCompileAction action = builder.build();
- env.registerAction(action);
- if (addObject) {
- result.addObjectFile(action.getOutputFile());
- }
+ boolean usePic = !generateNoPicAction;
+ createFakeSourceAction(outputName, result, env, builder, outputExtension,
+ dependencyFileExtension, addObject, ccRelativeName, usePic);
} else {
- boolean generatePicAction = getGeneratePicActions();
- // If we always need pic for everything, then don't bother to create a no-pic action.
- boolean generateNoPicAction = getGenerateNoPicActions();
- Preconditions.checkState(generatePicAction || generateNoPicAction);
-
// Create PIC compile actions (same as non-PIC, but use -fPIC and
// generate .pic.o, .pic.d, .pic.gcno instead of .o, .d, .gcno.)
if (generatePicAction) {
@@ -529,6 +513,36 @@
}
}
+ private void createFakeSourceAction(PathFragment outputName, CcCompilationOutputs.Builder result,
+ AnalysisEnvironment env, CppCompileActionBuilder builder, String outputExtension,
+ String dependencyFileExtension, boolean addObject, PathFragment ccRelativeName,
+ boolean usePic) {
+ if (usePic) {
+ outputExtension = ".pic" + outputExtension;
+ dependencyFileExtension = ".pic" + dependencyFileExtension;
+ }
+ Artifact outputFile = ruleContext.getRelatedArtifact(outputName, outputExtension);
+ PathFragment tempOutputName =
+ FileSystemUtils.replaceExtension(
+ outputFile.getExecPath(), ".temp" + outputExtension, outputExtension);
+ builder
+ .setPicMode(usePic)
+ .setOutputFile(outputFile)
+ .setDotdFile(outputName, dependencyFileExtension)
+ .setTempOutputFile(tempOutputName);
+ setupBuildVariables(builder, getGeneratePicActions(), ccRelativeName);
+ semantics.finalizeCompileActionBuilder(ruleContext, builder);
+ CppCompileAction action = builder.build();
+ env.registerAction(action);
+ if (addObject) {
+ if (usePic) {
+ result.addPicObjectFile(action.getOutputFile());
+ } else {
+ result.addObjectFile(action.getOutputFile());
+ }
+ }
+ }
+
/**
* Constructs the C++ linker actions. It generally generates two actions, one for a static library
* and one for a dynamic library. If PIC is required for shared libraries, but not for binaries,
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 e273031..f309c68 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
@@ -59,7 +59,8 @@
private final PathFragment tempOutputFile;
- FakeCppCompileAction(ActionOwner owner,
+ FakeCppCompileAction(
+ ActionOwner owner,
ImmutableList<String> features,
FeatureConfiguration featureConfiguration,
CcToolchainFeatures.Variables variables,
@@ -78,10 +79,22 @@
Predicate<String> nocopts,
ImmutableList<PathFragment> extraSystemIncludePrefixes,
@Nullable String fdoBuildStamp,
- RuleContext ruleContext) {
- super(owner, features, featureConfiguration, variables, sourceFile, sourceLabel,
- mandatoryInputs, outputFile, dotdFile, null, null, null,
- configuration, cppConfiguration,
+ RuleContext ruleContext,
+ boolean usePic) {
+ super(owner,
+ features,
+ featureConfiguration,
+ variables,
+ sourceFile,
+ sourceLabel,
+ mandatoryInputs,
+ outputFile,
+ dotdFile,
+ null,
+ null,
+ null,
+ configuration,
+ cppConfiguration,
// We only allow inclusion of header files explicitly declared in
// "srcs", so we only use declaredIncludeSrcs, not declaredIncludeDirs.
// (Disallowing use of undeclared headers for cc_fake_binary is needed
@@ -91,8 +104,7 @@
// time, so they can't depend on the contents of the ".d" file.)
CppCompilationContext.disallowUndeclaredHeaders(context), actionContext, copts, pluginOpts,
nocopts, extraSystemIncludePrefixes, fdoBuildStamp, VOID_INCLUDE_RESOLVER,
- ImmutableList.<IncludeScannable>of(),
- GUID, /*usePic=*/false, ruleContext);
+ ImmutableList.<IncludeScannable>of(), GUID, usePic, ruleContext);
this.tempOutputFile = Preconditions.checkNotNull(tempOutputFile);
}
