diff options
Diffstat (limited to 'src/main/java')
3 files changed, 174 insertions, 125 deletions
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 81eeb0f500..6662968cb3 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 @@ public class CppLinkActionBuilder { } Map<Artifact, Artifact> ltoMapping = new HashMap<>(); - + ; if (isFinalLinkOfLtoBuild()) { for (LtoBackendArtifacts a : allLtoArtifacts) { ltoMapping.put(a.getBitcodeFile(), a.getObjectFile()); @@ -846,8 +846,15 @@ public class CppLinkActionBuilder { } 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 @@ public class CppLinkActionBuilder { 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 @@ public class CppLinkActionBuilder { 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 @@ public class CppLinkActionBuilder { LinkCommandLine.Builder linkCommandLineBuilder = new LinkCommandLine.Builder(ruleContext) - .setLinkerInputs(collectedLibrariesToLink.getExpandedLinkerInputs()) + .setLinkerInputs(linkerInputs) .setLinkTargetType(linkType) .setLinkStaticness(linkStaticness) .setToolchainLibrariesSolibDir( @@ -1108,7 +1108,7 @@ public class CppLinkActionBuilder { .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)); @@ -1117,8 +1117,9 @@ public class CppLinkActionBuilder { 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()); @@ -1207,7 +1208,7 @@ public class CppLinkActionBuilder { 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 09a38094de..dc7fb3bc6b 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 class LibrariesToLinkCollector { */ 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 @@ public class LibrariesToLinkCollector { 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 @@ public class LibrariesToLinkCollector { 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 @@ public class LibrariesToLinkCollector { } 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 @@ public class LibrariesToLinkCollector { return new CollectedLibrariesToLink( librariesToLink, - expandedLinkerInputsBuilder.build(), librarySearchDirectories.build(), allRuntimeLibrarySearchDirectories.build()); } @@ -232,8 +218,7 @@ public class LibrariesToLinkCollector { 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 @@ public class LibrariesToLinkCollector { 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 @@ public class LibrariesToLinkCollector { 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 @@ public class LibrariesToLinkCollector { !Link.useStartEndLib( input, CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider))); - expandedLinkerInputsBuilder.add(input); Artifact inputArtifact = input.getArtifact(); PathFragment libDir = inputArtifact.getExecPath().getParentDirectory(); if (!libDir.equals(solibDir) @@ -327,42 +305,13 @@ public class LibrariesToLinkCollector { } /** - * Get the effective path for the object artifact, being careful not to create new strings if - * possible. - * - * @param: objectFile - Artifact representing an object file. - * @param -inputIsFake - Is this object being used for a cc_fake_binary. - */ - private static String effectiveObjectFilePath(Artifact objectFile, boolean inputIsFake) { - // Avoid making new strings as much as possible! This called for every object file used in the - // build. - if (inputIsFake) { - return Link.FAKE_OBJECT_PREFIX + objectFile.getExecPathString(); - } - return objectFile.getExecPathString(); - } - - /** * Adds command-line options for a static library or non-library input into options. * * @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); - + 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 @@ -386,12 +335,6 @@ public class LibrariesToLinkCollector { 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 @@ -399,51 +342,29 @@ public class LibrariesToLinkCollector { 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( - effectiveObjectFilePath(member, input.isFake()), - /* 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 @@ -451,22 +372,23 @@ public class LibrariesToLinkCollector { inputArtifact = a; } + 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()) { librariesToLink.addValue( LibraryToLinkValue.forObjectFileGroup( ImmutableList.<Artifact>of(inputArtifact), inputIsWholeArchive)); } else { - librariesToLink.addValue( - LibraryToLinkValue.forObjectFile( - effectiveObjectFilePath(inputArtifact, input.isFake()), inputIsWholeArchive)); + librariesToLink.addValue(LibraryToLinkValue.forObjectFile(name, inputIsWholeArchive)); } - expandedLinkerInputsBuilder.add(input); } else { - librariesToLink.addValue( - LibraryToLinkValue.forStaticLibrary( - effectiveObjectFilePath(inputArtifact, input.isFake()), inputIsWholeArchive)); - expandedLinkerInputsBuilder.add(input); + librariesToLink.addValue(LibraryToLinkValue.forStaticLibrary(name, inputIsWholeArchive)); } } } 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 30cdbbe7dd..74142be962 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 @@ public abstract class Link { || 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(); + } + } } |