Rollback of commit 1da8ac3a8bb0780251976e0dbcfebd3a7ba6a066.

--
PiperOrigin-RevId: 144207746
MOS_MIGRATED_REVID=144207746

3 files changed, 45 insertions, 118 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 e42b6a4d84..f82c35b162 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
@@ -80,20 +80,6 @@ public class CppLinkActionBuilder {
   /** A build variable for flags providing files to link as inputs in the linker invocation */
   public static final String LIBRARIES_TO_LINK_VARIABLE = "libraries_to_link";
 
-  /**
-   * A build variable for thinlto params file produced by thinlto-indexing action and consumed by
-   * normal linking actions.
-   */
-  public static final String THINLTO_PARAMS_FILE_VARIABLE = "thinlto_params_file";
-
-  public static final String THINLTO_PREFIX_REPLACE_VARIABLE = "thinlto_prefix_replace";
-
-  /**
-   * A build variable for linker param file created by Bazel to overcome the command line length
-   * limit.
-   */
-  public static final String LINKER_PARAM_FILE_VARIABLE = "linker_param_file";
-
   /** A build variable for the execpath of the output of the linker. */
   public static final String OUTPUT_EXECPATH_VARIABLE = "output_execpath";
 
@@ -525,7 +511,7 @@ public class CppLinkActionBuilder {
     }
 
     PathFragment linkerParamsFileRootPath = null;
-    @Nullable Artifact thinltoParamsFile = null;
+    @Nullable Artifact linkerParamsFile = null;
     if (allLTOArtifacts != null) {
       // Create artifact for the file that the LTO indexing step will emit
       // object file names into for any that were included in the link as
@@ -536,7 +522,7 @@ public class CppLinkActionBuilder {
       // of the native object files not the input bitcode files.
       linkerParamsFileRootPath =
           ParameterFile.derivePath(output.getRootRelativePath(), "lto-final");
-      thinltoParamsFile =
+      linkerParamsFile =
           linkArtifactFactory.create(ruleContext, configuration, linkerParamsFileRootPath);
     }
 
@@ -546,8 +532,8 @@ public class CppLinkActionBuilder {
       for (LTOBackendArtifacts ltoA : allLTOArtifacts) {
         ltoA.addIndexingOutputs(builder);
       }
-      if (thinltoParamsFile != null) {
-        builder.add(thinltoParamsFile);
+      if (linkerParamsFile != null) {
+        builder.add(linkerParamsFile);
       }
       actionOutputs = builder.build();
     } else {
@@ -562,16 +548,6 @@ public class CppLinkActionBuilder {
     ImmutableList<LinkerInput> runtimeLinkerInputs =
         ImmutableList.copyOf(LinkerInputs.simpleLinkerInputs(runtimeInputs, runtimeType));
 
-
-    PathFragment paramRootPath =
-        ParameterFile.derivePath(output.getRootRelativePath(), (isLTOIndexing) ? "lto-index" : "2");
-
-    @Nullable
-    final Artifact paramFile =
-        canSplitCommandLine()
-            ? linkArtifactFactory.create(ruleContext, configuration, paramRootPath)
-            : null;
-
     // Add build variables necessary to template link args into the crosstool.
     Variables.Builder buildVariablesBuilder = new Variables.Builder();
     CppLinkVariablesExtension variablesExtension =
@@ -583,8 +559,7 @@ public class CppLinkActionBuilder {
                 linkerInputs,
                 runtimeLinkerInputs,
                 null,
-                paramFile,
-                thinltoParamsFile,
+                linkerParamsFile,
                 ltoOutputRootPrefix,
                 null,
                 null)
@@ -595,8 +570,7 @@ public class CppLinkActionBuilder {
                 linkerInputs,
                 runtimeLinkerInputs,
                 output,
-                paramFile,
-                thinltoParamsFile,
+                linkerParamsFile,
                 PathFragment.EMPTY_FRAGMENT,
                 getInterfaceSoBuilder(),
                 interfaceOutput);
@@ -606,6 +580,15 @@ public class CppLinkActionBuilder {
     }
     Variables buildVariables = buildVariablesBuilder.build();
 
+    PathFragment paramRootPath =
+        ParameterFile.derivePath(output.getRootRelativePath(), (isLTOIndexing) ? "lto-index" : "2");
+
+    @Nullable
+    final Artifact paramFile =
+        canSplitCommandLine()
+            ? linkArtifactFactory.create(ruleContext, configuration, paramRootPath)
+            : null;
+
     Preconditions.checkArgument(
         linkType != LinkTargetType.INTERFACE_DYNAMIC_LIBRARY,
         "you can't link an interface dynamic library directly");
@@ -721,8 +704,8 @@ public class CppLinkActionBuilder {
             .add(dependencyInputsBuilder.build())
             .add(ImmutableIterable.from(expandedInputs));
 
-    if (thinltoParamsFile != null && !isLTOIndexing) {
-      inputsBuilder.add(ImmutableList.of(thinltoParamsFile));
+    if (linkerParamsFile != null && !isLTOIndexing) {
+      inputsBuilder.add(ImmutableList.of(linkerParamsFile));
     }
     if (linkCommandLine.getParamFile() != null) {
       inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile()));
@@ -1234,6 +1217,7 @@ public class CppLinkActionBuilder {
     public ImmutableSet<String> getLibrarySearchDirectories() {
       return librarySearchDirectories;
     }
+
   }
 
   private class CppLinkVariablesExtension implements VariablesExtension {
@@ -1246,8 +1230,7 @@ public class CppLinkActionBuilder {
     private final Artifact outputArtifact;
     private final Artifact interfaceLibraryBuilder;
     private final Artifact interfaceLibraryOutput;
-    private final Artifact paramFile;
-    private final Artifact thinltoParamsFile;
+    private final Artifact linkerParamsFile;
     private final PathFragment ltoOutputRootPrefix;
 
     private final LinkArgCollector linkArgCollector = new LinkArgCollector();
@@ -1259,8 +1242,7 @@ public class CppLinkActionBuilder {
         Iterable<LinkerInput> linkerInputs,
         ImmutableList<LinkerInput> runtimeLinkerInputs,
         Artifact output,
-        Artifact paramFile,
-        Artifact thinltoParamsFile,
+        Artifact linkerParamsFile,
         PathFragment ltoOutputRootPrefix,
         Artifact interfaceLibraryBuilder,
         Artifact interfaceLibraryOutput) {
@@ -1272,8 +1254,7 @@ public class CppLinkActionBuilder {
       this.outputArtifact = output;
       this.interfaceLibraryBuilder = interfaceLibraryBuilder;
       this.interfaceLibraryOutput = interfaceLibraryOutput;
-      this.paramFile = paramFile;
-      this.thinltoParamsFile = thinltoParamsFile;
+      this.linkerParamsFile = linkerParamsFile;
       this.ltoOutputRootPrefix = ltoOutputRootPrefix;
 
       addInputFileLinkOptions(linkArgCollector);
@@ -1323,10 +1304,6 @@ public class CppLinkActionBuilder {
       buildVariables.addCustomBuiltVariable(
           LIBRARIES_TO_LINK_VARIABLE, linkArgCollector.getLibrariesToLink());
 
-      if (paramFile != null) {
-        buildVariables.addStringVariable(LINKER_PARAM_FILE_VARIABLE, paramFile.getExecPathString());
-      }
-
       buildVariables.addStringSequenceVariable(
           LIBRARY_SEARCH_DIRECTORIES_VARIABLE, linkArgCollector.getLibrarySearchDirectories());
 
@@ -1341,30 +1318,18 @@ public class CppLinkActionBuilder {
             OUTPUT_EXECPATH_VARIABLE, outputArtifact.getExecPathString());
       }
 
-      if (isLTOIndexing()) {
-        if (thinltoParamsFile != null) {
-          // This is a lto-indexing action and we want it to populate params file.
-          buildVariables.addStringVariable(
-              THINLTO_PARAMS_FILE_VARIABLE, thinltoParamsFile.getExecPathString());
-          // TODO(b/33846234): Remove once all the relevant crosstools don't depend on the variable.
+      if (!ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT)) {
+        if (linkerParamsFile != null) {
           buildVariables.addStringVariable(
-              "thinlto_optional_params_file", "=" + thinltoParamsFile.getExecPathString());
+              "thinlto_optional_params_file", "=" + linkerParamsFile.getExecPathString());
         } else {
-          buildVariables.addStringVariable(THINLTO_PARAMS_FILE_VARIABLE, "");
-          // TODO(b/33846234): Remove once all the relevant crosstools don't depend on the variable.
           buildVariables.addStringVariable("thinlto_optional_params_file", "");
         }
         buildVariables.addStringVariable(
-            THINLTO_PREFIX_REPLACE_VARIABLE,
+            "thinlto_prefix_replace",
             configuration.getBinDirectory().getExecPathString()
                 + ";"
                 + configuration.getBinDirectory().getExecPath().getRelative(ltoOutputRootPrefix));
-      } else {
-        if (thinltoParamsFile != null) {
-          // This is a normal link action and we need to use params file created by lto-indexing.
-          buildVariables.addStringVariable(
-              THINLTO_PARAMS_FILE_VARIABLE, thinltoParamsFile.getExecPathString());
-        }
       }
       boolean shouldGenerateInterfaceLibrary =
           outputArtifact != null
@@ -1389,10 +1354,6 @@ public class CppLinkActionBuilder {
       CppHelper.getFdoSupport(ruleContext).getLinkOptions(featureConfiguration, buildVariables);
     }
 
-    private boolean isLTOIndexing() {
-      return !ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT);
-    }
-
     private boolean isSharedNativeLibrary() {
       return isNativeDeps && cppConfiguration.shareNativeDeps();
     }
@@ -1551,6 +1512,11 @@ public class CppLinkActionBuilder {
           addStaticInputLinkOptions(input, librariesToLink, true, ltoMap);
         }
       }
+      if (linkerParamsFile != null && ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT)) {
+        librariesToLink.addValue(
+            new LibraryToLinkValue(
+                "-Wl,@" + linkerParamsFile.getExecPathString(), needWholeArchive));
+      }
 
       // rpath ordering matters for performance; first add the one where most libraries are found.
       if (includeSolibDir && rpathRoot != null) {
@@ -1638,13 +1604,13 @@ public class CppLinkActionBuilder {
         @Nullable Map<Artifact, Artifact> ltoMap) {
       Preconditions.checkState(!(input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY));
       // If we had any LTO artifacts, ltoMap whould be non-null. In that case,
-      // we should have created a thinltoParamsFile which the LTO indexing
+      // we should have created a linkerParamsFile which the LTO indexing
       // step will populate with the exec paths that correspond to the LTO
       // artifacts that the linker decided to include based on symbol resolution.
       // Those files will be included directly in the link (and not wrapped
       // in --start-lib/--end-lib) to ensure consistency between the two link
       // steps.
-      Preconditions.checkState(ltoMap == null || thinltoParamsFile != null);
+      Preconditions.checkState(ltoMap == null || linkerParamsFile != null);
 
       // start-lib/end-lib library: adds its input object files.
       if (Link.useStartEndLib(input, cppConfiguration.archiveType())) {
@@ -1654,7 +1620,7 @@ public class CppLinkActionBuilder {
           for (Artifact member : archiveMembers) {
             if (ltoMap != null && ltoMap.remove(member) != null) {
               // The LTO artifacts that should be included in the final link
-              // are listed in the thinltoParamsFile. When ltoMap is non-null
+              // are listed in the linkerParamsFile. When ltoMap is non-null
               // the backend artifact may be missing due to libraries that list .o
               // files explicitly, or generate .o files from assembler.
               continue;
@@ -1679,7 +1645,7 @@ public class CppLinkActionBuilder {
 
         if (ltoMap != null && ltoMap.remove(inputArtifact) != null) {
           // The LTO artifacts that should be included in the final link
-          // are listed in the thinltoParamsFile.
+          // are listed in the linkerParamsFile.
           return;
         }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
index f94435494c..8b8ae161b9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionConfigs.java
@@ -68,7 +68,6 @@ public class CppLinkActionConfigs {
                 "   implies: 'libraries_to_link'",
                 "   implies: 'force_pic_flags'",
                 "   implies: 'legacy_link_flags'",
-                "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-dynamic-library'",
@@ -87,7 +86,6 @@ public class CppLinkActionConfigs {
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'legacy_link_flags'",
-                "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-static-library'",
@@ -99,7 +97,6 @@ public class CppLinkActionConfigs {
                 "   implies: 'library_search_directories'",
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
-                "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-alwayslink-static-library'",
@@ -111,7 +108,6 @@ public class CppLinkActionConfigs {
                 "   implies: 'library_search_directories'",
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
-                "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-pic-static-library'",
@@ -123,7 +119,6 @@ public class CppLinkActionConfigs {
                 "   implies: 'library_search_directories'",
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
-                "   implies: 'linker_param_file'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-alwayslink-pic-static-library'",
@@ -135,7 +130,6 @@ public class CppLinkActionConfigs {
                 "   implies: 'library_search_directories'",
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
-                "   implies: 'linker_param_file'",
                 "}",
                 "feature {",
                 "   name: 'build_interface_libraries'",
@@ -328,10 +322,6 @@ public class CppLinkActionConfigs {
                 "               flag: '-Wl,--end-lib'",
                 "           }",
                 "       }",
-                "       flag_group {",
-                "           expand_if_true: 'thinlto_params_file'",
-                "           flag: '-Wl,@%{thinlto_params_file}'",
-                "       }",
                 "   }",
                 "}",
                 "feature {",
@@ -355,27 +345,6 @@ public class CppLinkActionConfigs {
                 "           flag: '%{legacy_link_flags}'",
                 "       }",
                 "   }",
-                "}",
-                "feature {",
-                "   name: 'linker_param_file'",
-                "   flag_set {",
-                "       expand_if_all_available: 'linker_param_file'",
-                "       action: 'c++-link-executable'",
-                "       action: 'c++-link-dynamic-library'",
-                "       flag_group {",
-                "           flag: '-Wl,@%{linker_param_file}'",
-                "       }",
-                "   }",
-                "   flag_set {",
-                "       expand_if_all_available: 'linker_param_file'",
-                "       action: 'c++-link-static-library'",
-                "       action: 'c++-link-alwayslink-static-library'",
-                "       action: 'c++-link-pic-static-library'",
-                "       action: 'c++-link-alwayslink-pic-static-library'",
-                "       flag_group {",
-                "           flag: '@%{linker_param_file}'",
-                "       }",
-                "   }",
                 "}"));
   }
 
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 69d8bb9773..b253826683 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
@@ -218,16 +218,19 @@ public final class LinkCommandLine extends CommandLine {
    * Splits the link command-line into a part to be written to a parameter file, and the remaining
    * actual command line to be executed (which references the parameter file). Should only be used
    * if getParamFile() is not null.
+   *
+   * @throws IllegalStateException if the command-line cannot be split
    */
   @VisibleForTesting
   final Pair<List<String>, List<String>> splitCommandline() {
     List<String> args = getRawLinkArgv();
     if (linkTargetType.staticness() == Staticness.STATIC) {
       // Ar link commands can also generate huge command lines.
-      List<String> paramFileArgs = new ArrayList<>();
+      List<String> paramFileArgs = args.subList(1, args.size());
       List<String> commandlineArgs = new ArrayList<>();
-      extractArgumentsForStaticLinkParamsFile(args, commandlineArgs, paramFileArgs);
+      commandlineArgs.add(args.get(0));
 
+      commandlineArgs.add("@" + paramFile.getExecPath().getPathString());
       return Pair.of(commandlineArgs, paramFileArgs);
     } else {
       // Gcc link commands tend to generate humongous commandlines for some targets, which may
@@ -235,14 +238,17 @@ public final class LinkCommandLine extends CommandLine {
       // a parameter file and pass any linker options through it.
       List<String> paramFileArgs = new ArrayList<>();
       List<String> commandlineArgs = new ArrayList<>();
-      extractArgumentsForDynamicLinkParamsFile(args, commandlineArgs, paramFileArgs);
+      extractArgumentsForParamFile(args, commandlineArgs, paramFileArgs);
 
+      commandlineArgs.add("-Wl,@" + paramFile.getExecPath().getPathString());
       return Pair.of(commandlineArgs, paramFileArgs);
     }
   }
 
   /**
    * Returns just the .params file portion of the command-line as a {@link CommandLine}.
+   *
+   * @throws IllegalStateException if the command-line cannot be split
    */
   CommandLine paramCmdLine() {
     Preconditions.checkNotNull(paramFile);
@@ -255,22 +261,8 @@ public final class LinkCommandLine extends CommandLine {
   }
 
 
-  public static void extractArgumentsForStaticLinkParamsFile(
-      List<String> args, List<String> commandlineArgs, List<String> paramFileArgs) {
-    commandlineArgs.add(args.get(0)); // ar command, must not be moved!
-    int argsSize = args.size();
-    for (int i = 1; i < argsSize; i++) {
-      String arg = args.get(i);
-      if (arg.startsWith("@")) {
-        commandlineArgs.add(arg); // params file, keep it in the command line
-      } else {
-        paramFileArgs.add(arg); // the rest goes to the params file
-      }
-    }
-  }
-
-  public static void extractArgumentsForDynamicLinkParamsFile(
-      List<String> args, List<String> commandlineArgs, List<String> paramFileArgs) {
+  public static void extractArgumentsForParamFile(List<String> args, List<String> commandlineArgs,
+      List<String> paramFileArgs) {
     // Note, that it is not important that all linker arguments are extracted so that
     // they can be moved into a parameter file, but the vast majority should.
     commandlineArgs.add(args.get(0));   // gcc command, must not be moved!