Automated rollback of commit 8ca66458a42266f11aba77732b05ab06b96e95fb.
PiperOrigin-RevId: 194506134
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 4739b7b..19bd3a7 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
@@ -838,7 +838,7 @@
}
Map<Artifact, Artifact> ltoMapping = new HashMap<>();
-
+ ;
if (isFinalLinkOfLtoBuild()) {
for (LtoBackendArtifacts a : allLtoArtifacts) {
ltoMapping.put(a.getBitcodeFile(), a.getObjectFile());
@@ -846,8 +846,15 @@
}
Iterable<Artifact> objectArtifacts =
getArtifactsPossiblyLtoMapped(objectFileInputs, ltoMapping);
- ImmutableSet<Artifact> linkstampObjectArtifacts =
+ Iterable<Artifact> linkstampObjectArtifacts =
getArtifactsPossiblyLtoMapped(linkstampObjectFileInputs, ltoMapping);
+ Iterable<Artifact> expandedInputs =
+ getArtifactsPossiblyLtoMapped(
+ Link.mergeInputsDependencies(
+ uniqueLibraries,
+ needWholeArchive,
+ CppHelper.getArchiveType(cppConfiguration, toolchain)),
+ ltoMapping);
ImmutableSet<Artifact> combinedObjectArtifacts =
ImmutableSet.<Artifact>builder()
@@ -921,12 +928,16 @@
symbolCounts);
}
- // Linker inputs without any start/end lib expansions.
- final Iterable<LinkerInput> nonExpandedLinkerInputs =
+ final Iterable<LinkerInput> linkerInputs =
IterablesChain.<LinkerInput>builder()
.add(objectFileInputs)
.add(linkstampObjectFileInputs)
- .add(ImmutableIterable.from(uniqueLibraries))
+ .add(
+ ImmutableIterable.from(
+ Link.mergeInputsCmdLine(
+ uniqueLibraries,
+ needWholeArchive,
+ CppHelper.getArchiveType(cppConfiguration, toolchain))))
.add(
// Adding toolchain libraries without whole archive no-matter-what. People don't
// want to include whole libstdc++ in their binary ever.
@@ -971,21 +982,10 @@
featureConfiguration,
thinltoParamFile,
allowLtoIndexing,
- nonExpandedLinkerInputs,
+ linkerInputs,
needWholeArchive);
CollectedLibrariesToLink collectedLibrariesToLink =
librariesToLinkCollector.collectLibrariesToLink();
-
- ImmutableSet<Artifact> expandedLinkerArtifacts =
- getArtifactsPossiblyLtoMapped(
- collectedLibrariesToLink.getExpandedLinkerInputs(), ltoMapping);
-
- // Remove the linkstamp objects from inputs so that createLinkstampCompileAction doesn't cause a
- // circular dependency.
- Set<Artifact> expandedLinkerArtifactsNoLinkstamps =
- new LinkedHashSet<Artifact>(expandedLinkerArtifacts);
- expandedLinkerArtifactsNoLinkstamps.removeAll(linkstampObjectArtifacts);
-
Variables variables =
LinkBuildVariables.setupVariables(
this,
@@ -1041,7 +1041,7 @@
LinkCommandLine.Builder linkCommandLineBuilder =
new LinkCommandLine.Builder(ruleContext)
- .setLinkerInputs(collectedLibrariesToLink.getExpandedLinkerInputs())
+ .setLinkerInputs(linkerInputs)
.setLinkTargetType(linkType)
.setLinkStaticness(linkStaticness)
.setToolchainLibrariesSolibDir(
@@ -1107,7 +1107,7 @@
.add(ImmutableList.copyOf(objectArtifacts))
.add(ImmutableList.copyOf(nonCodeInputs))
.add(dependencyInputsBuilder.build())
- .add(ImmutableSet.copyOf(expandedLinkerArtifactsNoLinkstamps));
+ .add(ImmutableIterable.from(expandedInputs));
if (thinltoParamFile != null && !isLtoIndexing) {
inputsBuilder.add(ImmutableList.of(thinltoParamFile));
@@ -1116,8 +1116,9 @@
inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile()));
// Pass along tree artifacts, so they can be properly expanded.
ImmutableSet<Artifact> paramFileActionInputs =
- expandedLinkerArtifacts
+ ImmutableSet.<LinkerInput>copyOf(linkerInputs)
.stream()
+ .map(LinkerInput::getArtifact)
.filter(a -> a.isTreeArtifact())
.collect(ImmutableSet.toImmutableSet());
@@ -1206,7 +1207,7 @@
return !isLtoIndexing && allLtoArtifacts != null;
}
- private ImmutableSet<Artifact> getArtifactsPossiblyLtoMapped(
+ private Iterable<Artifact> getArtifactsPossiblyLtoMapped(
Iterable<LinkerInput> inputs, Map<Artifact, Artifact> ltoMapping) {
Preconditions.checkNotNull(ltoMapping);
ImmutableSet.Builder<Artifact> result = ImmutableSet.builder();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java
index ec86c09..dc7fb3b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java
@@ -117,17 +117,14 @@
*/
public static class CollectedLibrariesToLink {
private final SequenceBuilder librariesToLink;
- private final ImmutableSet<LinkerInput> expandedLinkerInputs;
private final ImmutableSet<String> librarySearchDirectories;
private final ImmutableSet<String> runtimeLibrarySearchDirectories;
public CollectedLibrariesToLink(
SequenceBuilder librariesToLink,
- ImmutableSet<LinkerInput> expandedLinkerInputs,
ImmutableSet<String> librarySearchDirectories,
ImmutableSet<String> runtimeLibrarySearchDirectories) {
this.librariesToLink = librariesToLink;
- this.expandedLinkerInputs = expandedLinkerInputs;
this.librarySearchDirectories = librarySearchDirectories;
this.runtimeLibrarySearchDirectories = runtimeLibrarySearchDirectories;
}
@@ -136,11 +133,6 @@
return librariesToLink;
}
- // TODO(b/78347840): Figure out how to make these Artifacts.
- public ImmutableSet<LinkerInput> getExpandedLinkerInputs() {
- return expandedLinkerInputs;
- }
-
public ImmutableSet<String> getLibrarySearchDirectories() {
return librarySearchDirectories;
}
@@ -163,7 +155,6 @@
ImmutableSet.Builder<String> librarySearchDirectories = ImmutableSet.builder();
ImmutableSet.Builder<String> runtimeLibrarySearchDirectories = ImmutableSet.builder();
ImmutableSet.Builder<String> rpathRootsForExplicitSoDeps = ImmutableSet.builder();
- ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder = ImmutableSet.builder();
// List of command line parameters that need to be placed *outside* of
// --whole-archive ... --no-whole-archive.
SequenceBuilder librariesToLink = new SequenceBuilder();
@@ -202,11 +193,7 @@
}
Pair<Boolean, Boolean> includeSolibsPair =
- addLinkerInputs(
- librarySearchDirectories,
- rpathRootsForExplicitSoDeps,
- librariesToLink,
- expandedLinkerInputsBuilder);
+ addLinkerInputs(librarySearchDirectories, rpathRootsForExplicitSoDeps, librariesToLink);
boolean includeSolibDir = includeSolibsPair.first;
boolean includeToolchainLibrariesSolibDir = includeSolibsPair.second;
Preconditions.checkState(
@@ -224,7 +211,6 @@
return new CollectedLibrariesToLink(
librariesToLink,
- expandedLinkerInputsBuilder.build(),
librarySearchDirectories.build(),
allRuntimeLibrarySearchDirectories.build());
}
@@ -232,8 +218,7 @@
private Pair<Boolean, Boolean> addLinkerInputs(
Builder<String> librarySearchDirectories,
Builder<String> rpathEntries,
- SequenceBuilder librariesToLink,
- ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder) {
+ SequenceBuilder librariesToLink) {
boolean includeSolibDir = false;
boolean includeToolchainLibrariesSolibDir = false;
for (LinkerInput input : linkerInputs) {
@@ -257,14 +242,9 @@
includeToolchainLibrariesSolibDir = true;
}
}
- addDynamicInputLinkOptions(
- input,
- librariesToLink,
- expandedLinkerInputsBuilder,
- librarySearchDirectories,
- rpathEntries);
+ addDynamicInputLinkOptions(input, librariesToLink, librarySearchDirectories, rpathEntries);
} else {
- addStaticInputLinkOptions(input, librariesToLink, expandedLinkerInputsBuilder);
+ addStaticInputLinkOptions(input, librariesToLink);
}
}
return Pair.of(includeSolibDir, includeToolchainLibrariesSolibDir);
@@ -278,7 +258,6 @@
private void addDynamicInputLinkOptions(
LinkerInput input,
SequenceBuilder librariesToLink,
- ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder,
ImmutableSet.Builder<String> librarySearchDirectories,
ImmutableSet.Builder<String> rpathRootsForExplicitSoDeps) {
Preconditions.checkState(
@@ -288,7 +267,6 @@
!Link.useStartEndLib(
input, CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider)));
- expandedLinkerInputsBuilder.add(input);
Artifact inputArtifact = input.getArtifact();
PathFragment libDir = inputArtifact.getExecPath().getParentDirectory();
if (!libDir.equals(solibDir)
@@ -331,24 +309,9 @@
*
* @param librariesToLink - a collection that will be exposed as a build variable.
*/
- private void addStaticInputLinkOptions(
- LinkerInput input,
- SequenceBuilder librariesToLink,
- ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder) {
+ private void addStaticInputLinkOptions(LinkerInput input, SequenceBuilder librariesToLink) {
ArtifactCategory artifactCategory = input.getArtifactCategory();
- Preconditions.checkArgument(
- artifactCategory.equals(ArtifactCategory.OBJECT_FILE)
- || artifactCategory.equals(ArtifactCategory.STATIC_LIBRARY)
- || artifactCategory.equals(ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY));
- boolean isAlwaysLinkStaticLibrary =
- artifactCategory == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY;
-
- // input.disableWholeArchive() should only be true for libstdc++/libc++ etc.
- boolean inputIsWholeArchive =
- !input.disableWholeArchive() && (isAlwaysLinkStaticLibrary || needWholeArchive);
-
- String pathPrefix = input.isFake() ? Link.FAKE_OBJECT_PREFIX : "";
-
+ Preconditions.checkState(artifactCategory != ArtifactCategory.DYNAMIC_LIBRARY);
// If we had any LTO artifacts, ltoMap whould be non-null. In that case,
// we should have created a thinltoParamFile which the LTO indexing
// step will populate with the exec paths that correspond to the LTO
@@ -372,12 +335,6 @@
if (handledByLtoIndexing(a, allowLtoIndexing)) {
// The LTO artifacts that should be included in the final link
// are listed in the thinltoParamFile, generated by the LTO indexing.
-
- // Even if this object file is being skipped for exposure as a Build variable, it's
- // still an input to this action.
- expandedLinkerInputsBuilder.add(
- LinkerInputs.simpleLinkerInput(
- a, ArtifactCategory.OBJECT_FILE, /* disableWholeArchive = */ false));
continue;
}
// No LTO indexing step, so use the LTO backend's generated artifact directly
@@ -385,50 +342,29 @@
member = a;
}
nonLtoArchiveMembersBuilder.add(member);
- expandedLinkerInputsBuilder.add(
- LinkerInputs.simpleLinkerInput(
- member, ArtifactCategory.OBJECT_FILE, /* disableWholeArchive = */ false));
}
ImmutableList<Artifact> nonLtoArchiveMembers = nonLtoArchiveMembersBuilder.build();
if (!nonLtoArchiveMembers.isEmpty()) {
- if (inputIsWholeArchive) {
- for (Artifact member : nonLtoArchiveMembers) {
- if (member.isTreeArtifact()) {
- // TODO(b/78189629): This object filegroup is expanded at action time but wrapped
- // with --start/--end-lib. There's currently no way to force these objects to be
- // linked in.
- librariesToLink.addValue(
- LibraryToLinkValue.forObjectFileGroup(
- ImmutableList.<Artifact>of(member), /* isWholeArchive= */ true));
- } else {
- // TODO(b/78189629): These each need to be their own LibraryToLinkValue so they're
- // not wrapped in --start/--end-lib (which lets the linker leave out objects with
- // unreferenced code).
- librariesToLink.addValue(
- LibraryToLinkValue.forObjectFile(
- pathPrefix + member.getExecPathString(), /* isWholeArchive= */ true));
- }
- }
- } else {
- librariesToLink.addValue(
- LibraryToLinkValue.forObjectFileGroup(
- nonLtoArchiveMembers, /* isWholeArchive= */ false));
- }
+ librariesToLink.addValue(
+ LibraryToLinkValue.forObjectFileGroup(nonLtoArchiveMembers, needWholeArchive));
}
}
} else {
+ Preconditions.checkArgument(
+ artifactCategory.equals(ArtifactCategory.OBJECT_FILE)
+ || artifactCategory.equals(ArtifactCategory.STATIC_LIBRARY)
+ || artifactCategory.equals(ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY));
+ boolean isAlwaysLinkStaticLibrary =
+ artifactCategory == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY;
+ boolean inputIsWholeArchive =
+ !input.disableWholeArchive() && (isAlwaysLinkStaticLibrary || needWholeArchive);
+
Artifact inputArtifact = input.getArtifact();
Artifact a;
if (ltoMap != null && (a = ltoMap.remove(inputArtifact)) != null) {
if (handledByLtoIndexing(a, allowLtoIndexing)) {
// The LTO artifacts that should be included in the final link
// are listed in the thinltoParamFile, generated by the LTO indexing.
-
- // Even if this object file is being skipped for exposure as a Build variable, it's
- // still an input to this action.
- expandedLinkerInputsBuilder.add(
- LinkerInputs.simpleLinkerInput(
- a, ArtifactCategory.OBJECT_FILE, /* disableWholeArchive = */ false));
return;
}
// No LTO indexing step, so use the LTO backend's generated artifact directly
@@ -436,7 +372,12 @@
inputArtifact = a;
}
- String name = pathPrefix + inputArtifact.getExecPathString();
+ String name;
+ if (input.isFake()) {
+ name = Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString();
+ } else {
+ name = inputArtifact.getExecPathString();
+ }
if (artifactCategory.equals(ArtifactCategory.OBJECT_FILE)) {
if (inputArtifact.isTreeArtifact()) {
@@ -446,10 +387,8 @@
} else {
librariesToLink.addValue(LibraryToLinkValue.forObjectFile(name, inputIsWholeArchive));
}
- expandedLinkerInputsBuilder.add(input);
} else {
librariesToLink.addValue(LibraryToLinkValue.forStaticLibrary(name, inputIsWholeArchive));
- expandedLinkerInputsBuilder.add(input);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
index 30cdbbe..74142be 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/Link.java
@@ -14,7 +14,13 @@
package com.google.devtools.build.lib.rules.cpp;
+import com.google.common.collect.AbstractIterator;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.collect.CollectionUtils;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink;
import com.google.devtools.build.lib.util.FileTypeSet;
+import java.util.Iterator;
/**
* Utility types and methods for generating command lines for the linker, given
@@ -289,4 +295,124 @@
|| linkerInput.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY)
&& linkerInput.containsObjectFiles();
}
+
+ /**
+ * Replace always used archives with its members. This is used to build the linker cmd line.
+ */
+ public static Iterable<LinkerInput> mergeInputsCmdLine(NestedSet<LibraryToLink> inputs,
+ boolean globalNeedWholeArchive, ArchiveType archiveType) {
+ return new FilterMembersForLinkIterable(inputs, globalNeedWholeArchive, archiveType, false);
+ }
+
+ /**
+ * Add in any object files which are implicitly named as inputs by the linker.
+ */
+ public static Iterable<LinkerInput> mergeInputsDependencies(NestedSet<LibraryToLink> inputs,
+ boolean globalNeedWholeArchive, ArchiveType archiveType) {
+ return new FilterMembersForLinkIterable(inputs, globalNeedWholeArchive, archiveType, true);
+ }
+
+ /**
+ * On the fly implementation to filter the members.
+ */
+ private static final class FilterMembersForLinkIterable implements Iterable<LinkerInput> {
+ private final boolean globalNeedWholeArchive;
+ private final ArchiveType archiveType;
+ private final boolean deps;
+
+ private final Iterable<LibraryToLink> inputs;
+
+ private FilterMembersForLinkIterable(Iterable<LibraryToLink> inputs,
+ boolean globalNeedWholeArchive, ArchiveType archiveType, boolean deps) {
+ this.globalNeedWholeArchive = globalNeedWholeArchive;
+ this.archiveType = archiveType;
+ this.deps = deps;
+ this.inputs = CollectionUtils.makeImmutable(inputs);
+ }
+
+ @Override
+ public Iterator<LinkerInput> iterator() {
+ return new FilterMembersForLinkIterator(inputs.iterator(), globalNeedWholeArchive,
+ archiveType, deps);
+ }
+ }
+
+ /**
+ * On the fly implementation to filter the members.
+ */
+ private static final class FilterMembersForLinkIterator extends AbstractIterator<LinkerInput> {
+ private final boolean globalNeedWholeArchive;
+ private final ArchiveType archiveType;
+ private final boolean deps;
+
+ private final Iterator<LibraryToLink> inputs;
+ private Iterator<LinkerInput> delayList = ImmutableList.<LinkerInput>of().iterator();
+
+ private FilterMembersForLinkIterator(Iterator<LibraryToLink> inputs,
+ boolean globalNeedWholeArchive, ArchiveType archiveType, boolean deps) {
+ this.globalNeedWholeArchive = globalNeedWholeArchive;
+ this.archiveType = archiveType;
+ this.deps = deps;
+ this.inputs = inputs;
+ }
+
+ @Override
+ protected LinkerInput computeNext() {
+ if (delayList.hasNext()) {
+ return delayList.next();
+ }
+
+ while (inputs.hasNext()) {
+ LibraryToLink inputLibrary = inputs.next();
+
+ // True if the linker might use the members of this file, i.e., if the file is a thin or
+ // start_end_lib archive (aka static library). Also check if the library contains object
+ // files - otherwise getObjectFiles returns null, which would lead to an NPE in
+ // simpleLinkerInputs.
+ boolean needMembersForLink = archiveType != ArchiveType.REGULAR
+ && (inputLibrary.getArtifactCategory() == ArtifactCategory.STATIC_LIBRARY
+ || inputLibrary.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY)
+ && inputLibrary.containsObjectFiles();
+
+ // True if we will pass the members instead of the original archive.
+ boolean passMembersToLinkCmd = needMembersForLink && (globalNeedWholeArchive
+ || inputLibrary.getArtifactCategory() == ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY);
+
+ // If deps is false (when computing the inputs to be passed on the command line), then it's
+ // an if-then-else, i.e., the passMembersToLinkCmd flag decides whether to pass the object
+ // files or the archive itself. This flag in turn is based on whether the archives are fat
+ // or not (thin archives or start_end_lib) - we never expand fat archives, but we do expand
+ // non-fat archives if we need whole-archives for the entire link, or for the specific
+ // library (i.e., if alwayslink=1).
+ //
+ // If deps is true (when computing the inputs to be passed to the action as inputs), then it
+ // becomes more complicated. We always need to pass the members for thin and start_end_lib
+ // archives (needMembersForLink). And we _also_ need to pass the archive file itself unless
+ // it's a start_end_lib archive (unless it's an alwayslink library).
+
+ // A note about ordering: the order in which the object files and the library are returned
+ // does not currently matter - this code results in the library returned first, and the
+ // object files returned after, but only if both are returned, which can only happen if
+ // deps is true, in which case this code only computes the list of inputs for the link
+ // action (so the order isn't critical).
+ if (passMembersToLinkCmd || (deps && needMembersForLink)) {
+ delayList =
+ LinkerInputs.simpleLinkerInputs(
+ inputLibrary.getObjectFiles(),
+ ArtifactCategory.OBJECT_FILE,
+ /* disableWholeArchive= */ false)
+ .iterator();
+ }
+
+ if (!(passMembersToLinkCmd || (deps && useStartEndLib(inputLibrary, archiveType)))) {
+ return inputLibrary;
+ }
+
+ if (delayList.hasNext()) {
+ return delayList.next();
+ }
+ }
+ return endOfData();
+ }
+ }
}
