diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/cpp')
3 files changed, 125 insertions, 174 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 6662968cb3..81eeb0f500 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,15 +846,8 @@ public class CppLinkActionBuilder { } Iterable<Artifact> objectArtifacts = getArtifactsPossiblyLtoMapped(objectFileInputs, ltoMapping); - Iterable<Artifact> linkstampObjectArtifacts = + ImmutableSet<Artifact> linkstampObjectArtifacts = getArtifactsPossiblyLtoMapped(linkstampObjectFileInputs, ltoMapping); - Iterable<Artifact> expandedInputs = - getArtifactsPossiblyLtoMapped( - Link.mergeInputsDependencies( - uniqueLibraries, - needWholeArchive, - CppHelper.getArchiveType(cppConfiguration, toolchain)), - ltoMapping); ImmutableSet<Artifact> combinedObjectArtifacts = ImmutableSet.<Artifact>builder() @@ -928,16 +921,12 @@ public class CppLinkActionBuilder { symbolCounts); } - final Iterable<LinkerInput> linkerInputs = + // Linker inputs without any start/end lib expansions. + final Iterable<LinkerInput> nonExpandedLinkerInputs = IterablesChain.<LinkerInput>builder() .add(objectFileInputs) .add(linkstampObjectFileInputs) - .add( - ImmutableIterable.from( - Link.mergeInputsCmdLine( - uniqueLibraries, - needWholeArchive, - CppHelper.getArchiveType(cppConfiguration, toolchain)))) + .add(ImmutableIterable.from(uniqueLibraries)) .add( // Adding toolchain libraries without whole archive no-matter-what. People don't // want to include whole libstdc++ in their binary ever. @@ -982,10 +971,21 @@ public class CppLinkActionBuilder { featureConfiguration, thinltoParamFile, allowLtoIndexing, - linkerInputs, + nonExpandedLinkerInputs, 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(linkerInputs) + .setLinkerInputs(collectedLibrariesToLink.getExpandedLinkerInputs()) .setLinkTargetType(linkType) .setLinkStaticness(linkStaticness) .setToolchainLibrariesSolibDir( @@ -1108,7 +1108,7 @@ public class CppLinkActionBuilder { .add(ImmutableList.copyOf(objectArtifacts)) .add(ImmutableList.copyOf(nonCodeInputs)) .add(dependencyInputsBuilder.build()) - .add(ImmutableIterable.from(expandedInputs)); + .add(ImmutableSet.copyOf(expandedLinkerArtifactsNoLinkstamps)); if (thinltoParamFile != null && !isLtoIndexing) { inputsBuilder.add(ImmutableList.of(thinltoParamFile)); @@ -1117,9 +1117,8 @@ public class CppLinkActionBuilder { inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile())); // Pass along tree artifacts, so they can be properly expanded. ImmutableSet<Artifact> paramFileActionInputs = - ImmutableSet.<LinkerInput>copyOf(linkerInputs) + expandedLinkerArtifacts .stream() - .map(LinkerInput::getArtifact) .filter(a -> a.isTreeArtifact()) .collect(ImmutableSet.toImmutableSet()); @@ -1208,7 +1207,7 @@ public class CppLinkActionBuilder { return !isLtoIndexing && allLtoArtifacts != null; } - private Iterable<Artifact> getArtifactsPossiblyLtoMapped( + private ImmutableSet<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 dc7fb3bc6b..09a38094de 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,14 +117,17 @@ 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; } @@ -133,6 +136,11 @@ 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; } @@ -155,6 +163,7 @@ 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(); @@ -193,7 +202,11 @@ public class LibrariesToLinkCollector { } Pair<Boolean, Boolean> includeSolibsPair = - addLinkerInputs(librarySearchDirectories, rpathRootsForExplicitSoDeps, librariesToLink); + addLinkerInputs( + librarySearchDirectories, + rpathRootsForExplicitSoDeps, + librariesToLink, + expandedLinkerInputsBuilder); boolean includeSolibDir = includeSolibsPair.first; boolean includeToolchainLibrariesSolibDir = includeSolibsPair.second; Preconditions.checkState( @@ -211,6 +224,7 @@ public class LibrariesToLinkCollector { return new CollectedLibrariesToLink( librariesToLink, + expandedLinkerInputsBuilder.build(), librarySearchDirectories.build(), allRuntimeLibrarySearchDirectories.build()); } @@ -218,7 +232,8 @@ public class LibrariesToLinkCollector { private Pair<Boolean, Boolean> addLinkerInputs( Builder<String> librarySearchDirectories, Builder<String> rpathEntries, - SequenceBuilder librariesToLink) { + SequenceBuilder librariesToLink, + ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder) { boolean includeSolibDir = false; boolean includeToolchainLibrariesSolibDir = false; for (LinkerInput input : linkerInputs) { @@ -242,9 +257,14 @@ public class LibrariesToLinkCollector { includeToolchainLibrariesSolibDir = true; } } - addDynamicInputLinkOptions(input, librariesToLink, librarySearchDirectories, rpathEntries); + addDynamicInputLinkOptions( + input, + librariesToLink, + expandedLinkerInputsBuilder, + librarySearchDirectories, + rpathEntries); } else { - addStaticInputLinkOptions(input, librariesToLink); + addStaticInputLinkOptions(input, librariesToLink, expandedLinkerInputsBuilder); } } return Pair.of(includeSolibDir, includeToolchainLibrariesSolibDir); @@ -258,6 +278,7 @@ public class LibrariesToLinkCollector { private void addDynamicInputLinkOptions( LinkerInput input, SequenceBuilder librariesToLink, + ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder, ImmutableSet.Builder<String> librarySearchDirectories, ImmutableSet.Builder<String> rpathRootsForExplicitSoDeps) { Preconditions.checkState( @@ -267,6 +288,7 @@ 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) @@ -305,13 +327,42 @@ 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) { + private void addStaticInputLinkOptions( + LinkerInput input, + SequenceBuilder librariesToLink, + ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder) { ArtifactCategory artifactCategory = input.getArtifactCategory(); - Preconditions.checkState(artifactCategory != ArtifactCategory.DYNAMIC_LIBRARY); + 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); + // 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 @@ -335,6 +386,12 @@ 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 @@ -342,29 +399,51 @@ 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()) { - librariesToLink.addValue( - LibraryToLinkValue.forObjectFileGroup(nonLtoArchiveMembers, needWholeArchive)); + 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)); + } } } } 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 @@ -372,23 +451,22 @@ 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(name, inputIsWholeArchive)); + librariesToLink.addValue( + LibraryToLinkValue.forObjectFile( + effectiveObjectFilePath(inputArtifact, input.isFake()), inputIsWholeArchive)); } + expandedLinkerInputsBuilder.add(input); } else { - librariesToLink.addValue(LibraryToLinkValue.forStaticLibrary(name, inputIsWholeArchive)); + librariesToLink.addValue( + LibraryToLinkValue.forStaticLibrary( + effectiveObjectFilePath(inputArtifact, input.isFake()), 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 74142be962..30cdbbe7dd 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,13 +14,7 @@ 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 @@ -295,124 +289,4 @@ 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(); - } - } } |