From 3be23361e3d5a8409628aeeff06ce6fa1793ec3d Mon Sep 17 00:00:00 2001 From: Cal Peyser Date: Wed, 1 Jun 2016 14:08:22 +0000 Subject: If an action_config is given for a particular type of link action, use that action_config to configure the build. Otherwise, revert to hard-coded behavior. This change is designed to provide backwards-compatible support for crosstools that describe platform-specific linking semantics. -- MOS_MIGRATED_REVID=123748494 --- .../build/lib/rules/cpp/CcLibraryHelper.java | 24 +++++- .../build/lib/rules/cpp/CppConfiguration.java | 23 +++++- .../build/lib/rules/cpp/CppLinkAction.java | 8 +- .../google/devtools/build/lib/rules/cpp/Link.java | 26 ++++-- .../build/lib/rules/cpp/LinkCommandLine.java | 92 +++++++++++++--------- 5 files changed, 115 insertions(+), 58 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/rules/cpp') diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index d4c4da31e6..aaf5e95410 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -92,7 +92,16 @@ public final class CcLibraryHelper { CppCompileAction.CPP_HEADER_PREPROCESSING, CppCompileAction.CPP_MODULE_COMPILE, CppCompileAction.ASSEMBLE, - CppCompileAction.PREPROCESS_ASSEMBLE)), + CppCompileAction.PREPROCESS_ASSEMBLE, + Link.LinkTargetType.STATIC_LIBRARY.getActionName(), + // We need to create pic-specific actions for link actions, as they will produce + // differently named outputs. + Link.LinkTargetType.PIC_STATIC_LIBRARY.getActionName(), + Link.LinkTargetType.INTERFACE_DYNAMIC_LIBRARY.getActionName(), + Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(), + Link.LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY.getActionName(), + Link.LinkTargetType.ALWAYS_LINK_PIC_STATIC_LIBRARY.getActionName(), + Link.LinkTargetType.EXECUTABLE.getActionName())), CC_AND_OBJC( FileTypeSet.of( CppFileTypes.CPP_SOURCE, @@ -111,8 +120,17 @@ public final class CcLibraryHelper { CppCompileAction.CPP_HEADER_PREPROCESSING, CppCompileAction.CPP_MODULE_COMPILE, CppCompileAction.ASSEMBLE, - CppCompileAction.PREPROCESS_ASSEMBLE)); - + CppCompileAction.PREPROCESS_ASSEMBLE, + Link.LinkTargetType.STATIC_LIBRARY.getActionName(), + // We need to create pic-specific actions for link actions, as they will produce + // differently named outputs. + Link.LinkTargetType.PIC_STATIC_LIBRARY.getActionName(), + Link.LinkTargetType.INTERFACE_DYNAMIC_LIBRARY.getActionName(), + Link.LinkTargetType.DYNAMIC_LIBRARY.getActionName(), + Link.LinkTargetType.ALWAYS_LINK_STATIC_LIBRARY.getActionName(), + Link.LinkTargetType.ALWAYS_LINK_PIC_STATIC_LIBRARY.getActionName(), + Link.LinkTargetType.EXECUTABLE.getActionName())); + private final FileTypeSet sourceTypeSet; private final Set actionConfigSet; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 319610243b..07e87246ee 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -802,7 +802,13 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " flag_set {" + " action: 'c-compile'" + " action: 'c++-compile'" - + " action: 'c++-link'" + + " action: 'c++-link-static-library'" + + " action: 'c++-link-pic-static-library'" + + " action: 'c++-link-interface-dynamic-library'" + + " action: 'c++-link-dynamic-library'" + + " action: 'c++-link-alwayslink-static-library'" + + " action: 'c++-link-alwayslink-pic-static-library'" + + " action: 'c++-link-executable'" + " flag_group {" + " flag: '-Xgcc-only=-fprofile-generate=%{fdo_instrument_path}'" + " flag: '-Xclang-only=-fprofile-instr-generate=%{fdo_instrument_path}'" @@ -891,7 +897,13 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " }" + " }" + " flag_set {" - + " action: 'c++-link'" + + " action: 'c++-link-static-library'" + + " action: 'c++-link-pic-static-library'" + + " action: 'c++-link-interface-dynamic-library'" + + " action: 'c++-link-dynamic-library'" + + " action: 'c++-link-always-link-static-library'" + + " action: 'c++-link-always-link-pic-static-library'" + + " action: 'c++-link-executable'" + " flag_group {" + " flag: '-lgcov'" + " }" @@ -1025,6 +1037,13 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return toolchainIdentifier; } + /** + * Returns the path of the crosstool. + */ + public PathFragment getCrosstoolTopPathFragment() { + return crosstoolTopPathFragment; + } + /** * Returns the system name which is required by the toolchain to run. */ 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 3c49d0a2ac..c774f1d187 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 @@ -108,11 +108,6 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo private static final String LINK_GUID = "58ec78bd-1176-4e36-8143-439f656b181d"; private static final String FAKE_LINK_GUID = "da36f819-5a15-43a9-8a45-e01b60e10c8b"; - - /** - * The name of this action for the purpose of crosstool features/action_configs - */ - private static final String ACTION_NAME = "c++-link"; private final CppConfiguration cppConfiguration; private final LibraryToLink outputLibrary; @@ -655,9 +650,8 @@ public final class CppLinkAction extends AbstractAction implements ExecutionInfo /** * Returns the action name for purposes of querying the crosstool. */ - // TODO(b/28791924): Expand action types to values in Link.LinkTargetType. private String getActionName() { - return ACTION_NAME; + return linkType.getActionName(); } public CppLinkAction.Builder setLinkArtifactFactory(LinkArtifactFactory linkArtifactFactory) { 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 c51a06e533..38ab7e8ccf 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 @@ -94,32 +94,34 @@ public abstract class Link { */ public enum LinkTargetType { /** A normal static archive. */ - STATIC_LIBRARY(".a", true), + STATIC_LIBRARY(".a", true, "c++-link-static-library"), /** A static archive with .pic.o object files (compiled with -fPIC). */ - PIC_STATIC_LIBRARY(".pic.a", true), + PIC_STATIC_LIBRARY(".pic.a", true, "c++-link-pic-static-library"), /** An interface dynamic library. */ - INTERFACE_DYNAMIC_LIBRARY(".ifso", false), + INTERFACE_DYNAMIC_LIBRARY(".ifso", false, "c++-link-interface-dynamic-library"), /** A dynamic library. */ - DYNAMIC_LIBRARY(".so", false), + DYNAMIC_LIBRARY(".so", false, "c++-link-dynamic-library"), /** A static archive without removal of unused object files. */ - ALWAYS_LINK_STATIC_LIBRARY(".lo", true), + ALWAYS_LINK_STATIC_LIBRARY(".lo", true, "c++-link-alwayslink-static-library"), /** A PIC static archive without removal of unused object files. */ - ALWAYS_LINK_PIC_STATIC_LIBRARY(".pic.lo", true), + ALWAYS_LINK_PIC_STATIC_LIBRARY(".pic.lo", true, "c++-link-alwayslink-pic-static-library"), /** An executable binary. */ - EXECUTABLE("", false); + EXECUTABLE("", false, "c++-link-executable"); private final String extension; private final boolean staticLibraryLink; + private final String actionName; - private LinkTargetType(String extension, boolean staticLibraryLink) { + private LinkTargetType(String extension, boolean staticLibraryLink, String actionName) { this.extension = extension; this.staticLibraryLink = staticLibraryLink; + this.actionName = actionName; } public String getExtension() { @@ -129,6 +131,14 @@ public abstract class Link { public boolean isStaticLibraryLink() { return staticLibraryLink; } + + /** + * The name of a link action with this LinkTargetType, for the purpose of crosstool feature + * selection. + */ + public String getActionName() { + return actionName; + } } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index 2a6d88d664..96b5a3bb3c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java @@ -381,45 +381,61 @@ public final class LinkCommandLine extends CommandLine { */ public List getRawLinkArgv() { List argv = new ArrayList<>(); - switch (linkTargetType) { - case EXECUTABLE: - addCppArgv(argv); - break; - - case DYNAMIC_LIBRARY: - if (interfaceOutput != null) { - argv.add(configuration.getShExecutable().getPathString()); - argv.add("-c"); - argv.add("build_iface_so=\"$0\"; impl=\"$1\"; iface=\"$2\"; cmd=\"$3\"; shift 3; " - + "\"$cmd\" \"$@\" && \"$build_iface_so\" \"$impl\" \"$iface\""); - argv.add(interfaceSoBuilder.getExecPathString()); + + // We create the command line from the feature configuration. If no configuration is present, + // we use hard-coded flags. + if (featureConfiguration.actionIsConfigured(actionName)) { + argv.add(featureConfiguration + .getToolForAction(actionName) + .getToolPath(cppConfiguration.getCrosstoolTopPathFragment()) + .getPathString()); + argv.addAll(featureConfiguration.getCommandLine(actionName, variables)); + } else { + // TODO(b/28928350): In order to simplify the extraction of this logic into the linux + // crosstool, this switch statement should be reframed as a group of action_configs and + // features. Until they can be migrated into the linux crosstool, those features should + // be added to CppConfiguration.addLegacyFeatures(). + switch (linkTargetType) { + case EXECUTABLE: + addCppArgv(argv); + break; + + case DYNAMIC_LIBRARY: + if (interfaceOutput != null) { + argv.add(configuration.getShExecutable().getPathString()); + argv.add("-c"); + argv.add( + "build_iface_so=\"$0\"; impl=\"$1\"; iface=\"$2\"; cmd=\"$3\"; shift 3; " + + "\"$cmd\" \"$@\" && \"$build_iface_so\" \"$impl\" \"$iface\""); + argv.add(interfaceSoBuilder.getExecPathString()); + argv.add(output.getExecPathString()); + argv.add(interfaceOutput.getExecPathString()); + } + addCppArgv(argv); + // -pie is not compatible with -shared and should be + // removed when the latter is part of the link command. Should we need to further + // distinguish between shared libraries and executables, we could add additional + // command line / CROSSTOOL flags that distinguish them. But as long as this is + // the only relevant use case we're just special-casing it here. + Iterables.removeIf(argv, Predicates.equalTo("-pie")); + break; + + case STATIC_LIBRARY: + case PIC_STATIC_LIBRARY: + case ALWAYS_LINK_STATIC_LIBRARY: + case ALWAYS_LINK_PIC_STATIC_LIBRARY: + // The static library link command follows this template: + // ar + argv.add(cppConfiguration.getArExecutable().getPathString()); + argv.addAll( + cppConfiguration.getArFlags(cppConfiguration.archiveType() == Link.ArchiveType.THIN)); argv.add(output.getExecPathString()); - argv.add(interfaceOutput.getExecPathString()); - } - addCppArgv(argv); - // -pie is not compatible with -shared and should be - // removed when the latter is part of the link command. Should we need to further - // distinguish between shared libraries and executables, we could add additional - // command line / CROSSTOOL flags that distinguish them. But as long as this is - // the only relevant use case we're just special-casing it here. - Iterables.removeIf(argv, Predicates.equalTo("-pie")); - break; - - case STATIC_LIBRARY: - case PIC_STATIC_LIBRARY: - case ALWAYS_LINK_STATIC_LIBRARY: - case ALWAYS_LINK_PIC_STATIC_LIBRARY: - // The static library link command follows this template: - // ar - argv.add(cppConfiguration.getArExecutable().getPathString()); - argv.addAll( - cppConfiguration.getArFlags(cppConfiguration.archiveType() == Link.ArchiveType.THIN)); - argv.add(output.getExecPathString()); - addInputFileLinkOptions(argv, /*needWholeArchive=*/false); - break; - - default: - throw new IllegalArgumentException(); + addInputFileLinkOptions(argv, /*needWholeArchive=*/ false); + break; + + default: + throw new IllegalArgumentException(); + } } // Fission mode: debug info is in .dwo files instead of .o files. Inform the linker of this. -- cgit v1.2.3