Expose thinlto_params_file and linker_param_file as build variables

This cl exposes param files that were hard-coded before. This enables more
precise placement on the link command line.

This is a roll-forward of commit db7a9ea7f6b2af3c4c1f43ad4aa50cd4eca02921

--
PiperOrigin-RevId: 144708270
MOS_MIGRATED_REVID=144708270

3 files changed, 123 insertions, 48 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 f82c35b162..6bd3ca64c3 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,6 +80,20 @@ 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 param file produced by thinlto-indexing action and consumed by
+   * normal linking actions.
+   */
+  public static final String THINLTO_PARAM_FILE_VARIABLE = "thinlto_param_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";
 
@@ -416,6 +430,10 @@ public class CppLinkActionBuilder {
 
   @VisibleForTesting
   boolean canSplitCommandLine() {
+    if (fake) {
+      return false;
+    }
+
     if (toolchain == null || !toolchain.supportsParamFiles()) {
       return false;
     }
@@ -510,8 +528,8 @@ public class CppLinkActionBuilder {
       allLTOArtifacts = createLTOArtifacts(ltoOutputRootPrefix, uniqueLibraries);
     }
 
-    PathFragment linkerParamsFileRootPath = null;
-    @Nullable Artifact linkerParamsFile = null;
+    PathFragment linkerParamFileRootPath = null;
+    @Nullable Artifact thinltoParamFile = 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
@@ -520,10 +538,10 @@ public class CppLinkActionBuilder {
       // Note that the paths emitted into this file will have their prefixes
       // replaced with the final output directory, so they will be the paths
       // of the native object files not the input bitcode files.
-      linkerParamsFileRootPath =
+      linkerParamFileRootPath =
           ParameterFile.derivePath(output.getRootRelativePath(), "lto-final");
-      linkerParamsFile =
-          linkArtifactFactory.create(ruleContext, configuration, linkerParamsFileRootPath);
+      thinltoParamFile =
+          linkArtifactFactory.create(ruleContext, configuration, linkerParamFileRootPath);
     }
 
     final ImmutableList<Artifact> actionOutputs;
@@ -532,8 +550,8 @@ public class CppLinkActionBuilder {
       for (LTOBackendArtifacts ltoA : allLTOArtifacts) {
         ltoA.addIndexingOutputs(builder);
       }
-      if (linkerParamsFile != null) {
-        builder.add(linkerParamsFile);
+      if (thinltoParamFile != null) {
+        builder.add(thinltoParamFile);
       }
       actionOutputs = builder.build();
     } else {
@@ -548,6 +566,15 @@ 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 =
@@ -559,7 +586,8 @@ public class CppLinkActionBuilder {
                 linkerInputs,
                 runtimeLinkerInputs,
                 null,
-                linkerParamsFile,
+                paramFile,
+                thinltoParamFile,
                 ltoOutputRootPrefix,
                 null,
                 null)
@@ -570,7 +598,8 @@ public class CppLinkActionBuilder {
                 linkerInputs,
                 runtimeLinkerInputs,
                 output,
-                linkerParamsFile,
+                paramFile,
+                thinltoParamFile,
                 PathFragment.EMPTY_FRAGMENT,
                 getInterfaceSoBuilder(),
                 interfaceOutput);
@@ -580,15 +609,6 @@ 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");
@@ -704,8 +724,8 @@ public class CppLinkActionBuilder {
             .add(dependencyInputsBuilder.build())
             .add(ImmutableIterable.from(expandedInputs));
 
-    if (linkerParamsFile != null && !isLTOIndexing) {
-      inputsBuilder.add(ImmutableList.of(linkerParamsFile));
+    if (thinltoParamFile != null && !isLTOIndexing) {
+      inputsBuilder.add(ImmutableList.of(thinltoParamFile));
     }
     if (linkCommandLine.getParamFile() != null) {
       inputsBuilder.add(ImmutableList.of(linkCommandLine.getParamFile()));
@@ -1230,7 +1250,8 @@ public class CppLinkActionBuilder {
     private final Artifact outputArtifact;
     private final Artifact interfaceLibraryBuilder;
     private final Artifact interfaceLibraryOutput;
-    private final Artifact linkerParamsFile;
+    private final Artifact paramFile;
+    private final Artifact thinltoParamFile;
     private final PathFragment ltoOutputRootPrefix;
 
     private final LinkArgCollector linkArgCollector = new LinkArgCollector();
@@ -1242,7 +1263,8 @@ public class CppLinkActionBuilder {
         Iterable<LinkerInput> linkerInputs,
         ImmutableList<LinkerInput> runtimeLinkerInputs,
         Artifact output,
-        Artifact linkerParamsFile,
+        Artifact paramFile,
+        Artifact thinltoParamFile,
         PathFragment ltoOutputRootPrefix,
         Artifact interfaceLibraryBuilder,
         Artifact interfaceLibraryOutput) {
@@ -1254,7 +1276,8 @@ public class CppLinkActionBuilder {
       this.outputArtifact = output;
       this.interfaceLibraryBuilder = interfaceLibraryBuilder;
       this.interfaceLibraryOutput = interfaceLibraryOutput;
-      this.linkerParamsFile = linkerParamsFile;
+      this.paramFile = paramFile;
+      this.thinltoParamFile = thinltoParamFile;
       this.ltoOutputRootPrefix = ltoOutputRootPrefix;
 
       addInputFileLinkOptions(linkArgCollector);
@@ -1307,6 +1330,10 @@ public class CppLinkActionBuilder {
       buildVariables.addStringSequenceVariable(
           LIBRARY_SEARCH_DIRECTORIES_VARIABLE, linkArgCollector.getLibrarySearchDirectories());
 
+      if (paramFile != null) {
+        buildVariables.addStringVariable(LINKER_PARAM_FILE_VARIABLE, paramFile.getExecPathString());
+      }
+
       // mostly static
       if (linkStaticness == LinkStaticness.MOSTLY_STATIC && cppConfiguration.skipStaticOutputs()) {
         buildVariables.addStringVariable(SKIP_MOSTLY_STATIC_VARIABLE, "");
@@ -1318,18 +1345,30 @@ public class CppLinkActionBuilder {
             OUTPUT_EXECPATH_VARIABLE, outputArtifact.getExecPathString());
       }
 
-      if (!ltoOutputRootPrefix.equals(PathFragment.EMPTY_FRAGMENT)) {
-        if (linkerParamsFile != null) {
+      if (isLTOIndexing()) {
+        if (thinltoParamFile != null) {
+          // This is a lto-indexing action and we want it to populate param file.
+          buildVariables.addStringVariable(
+              THINLTO_PARAM_FILE_VARIABLE, thinltoParamFile.getExecPathString());
+          // TODO(b/33846234): Remove once all the relevant crosstools don't depend on the variable.
           buildVariables.addStringVariable(
-              "thinlto_optional_params_file", "=" + linkerParamsFile.getExecPathString());
+              "thinlto_optional_params_file", "=" + thinltoParamFile.getExecPathString());
         } else {
+          buildVariables.addStringVariable(THINLTO_PARAM_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",
+            THINLTO_PREFIX_REPLACE_VARIABLE,
             configuration.getBinDirectory().getExecPathString()
                 + ";"
                 + configuration.getBinDirectory().getExecPath().getRelative(ltoOutputRootPrefix));
+      } else {
+        if (thinltoParamFile != null) {
+          // This is a normal link action and we need to use param file created by lto-indexing.
+          buildVariables.addStringVariable(
+              THINLTO_PARAM_FILE_VARIABLE, thinltoParamFile.getExecPathString());
+        }
       }
       boolean shouldGenerateInterfaceLibrary =
           outputArtifact != null
@@ -1354,6 +1393,10 @@ 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();
     }
@@ -1512,11 +1555,6 @@ 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) {
@@ -1604,13 +1642,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 linkerParamsFile which the LTO indexing
+      // we should have created a thinltoParamFile 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 || linkerParamsFile != null);
+      Preconditions.checkState(ltoMap == null || thinltoParamFile != null);
 
       // start-lib/end-lib library: adds its input object files.
       if (Link.useStartEndLib(input, cppConfiguration.archiveType())) {
@@ -1620,7 +1658,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 linkerParamsFile. When ltoMap is non-null
+              // are listed in the thinltoParamFile. 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;
@@ -1645,7 +1683,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 linkerParamsFile.
+          // are listed in the thinltoParamFile.
           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 8b8ae161b9..f0bd59a746 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,6 +68,7 @@ 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'",
@@ -86,6 +87,7 @@ 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'",
@@ -97,6 +99,7 @@ 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'",
@@ -108,6 +111,7 @@ 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'",
@@ -119,6 +123,7 @@ 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'",
@@ -130,6 +135,7 @@ public class CppLinkActionConfigs {
                 "   implies: 'library_search_directories'",
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
+                "   implies: 'linker_param_file'",
                 "}",
                 "feature {",
                 "   name: 'build_interface_libraries'",
@@ -322,6 +328,10 @@ public class CppLinkActionConfigs {
                 "               flag: '-Wl,--end-lib'",
                 "           }",
                 "       }",
+                "       flag_group {",
+                "           expand_if_true: 'thinlto_param_file'",
+                "           flag: '-Wl,@%{thinlto_param_file}'",
+                "       }",
                 "   }",
                 "}",
                 "feature {",
@@ -345,6 +355,27 @@ 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 b253826683..67cb8a3e8c 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,19 +218,15 @@ 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 = args.subList(1, args.size());
+      List<String> paramFileArgs = new ArrayList<>();
       List<String> commandlineArgs = new ArrayList<>();
-      commandlineArgs.add(args.get(0));
-
-      commandlineArgs.add("@" + paramFile.getExecPath().getPathString());
+      extractArgumentsForStaticLinkParamFile(args, commandlineArgs, paramFileArgs);
       return Pair.of(commandlineArgs, paramFileArgs);
     } else {
       // Gcc link commands tend to generate humongous commandlines for some targets, which may
@@ -238,17 +234,14 @@ 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<>();
-      extractArgumentsForParamFile(args, commandlineArgs, paramFileArgs);
+      extractArgumentsForDynamicLinkParamFile(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);
@@ -260,9 +253,22 @@ public final class LinkCommandLine extends CommandLine {
     };
   }
 
+  public static void extractArgumentsForStaticLinkParamFile(
+      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 extractArgumentsForParamFile(List<String> args, List<String> commandlineArgs,
-      List<String> paramFileArgs) {
+  public static void extractArgumentsForDynamicLinkParamFile(
+      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!