Move hardcoded toolchain linker flags to CROSSTOOL

This cl introduces another build variable: toolchain_flags and updates all the
affected crosstools (those which started using action_configs, because the
defaults from CppLinkActionConfigs are not applied then). This build variable
is a requirement for follow-up refactoring exposing param_files build variable.
With toolchain_flags and param_files we will have full control over flags
placement on the link command line.

--
PiperOrigin-RevId: 142741060
MOS_MIGRATED_REVID=142741060

3 files changed, 45 insertions, 13 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 6f6939dbb5..49b7bcad42 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
@@ -95,6 +95,8 @@ public class CppLinkActionBuilder {
   /** A build variable for the path where to generate interface library using the builder tool. */
   public static final String INTERFACE_LIBRARY_OUTPUT_VARIABLE = "interface_library_output_path";
 
+  /** A build variable for the custom flags specified in the crosstool. */
+  public static final String TOOLCHAIN_FLAGS_VARIABLE = "toolchain_flags";
   /**
    * A build variable that is set to indicate a mostly static linking for which the linked binary
    * should be piped to /dev/null.
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 a74bc2a0cb..ba90b65dfa 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
@@ -67,6 +67,7 @@ public class CppLinkActionConfigs {
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
                 "   implies: 'force_pic_flags'",
+                "   implies: 'toolchain_flags'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-dynamic-library'",
@@ -84,6 +85,7 @@ public class CppLinkActionConfigs {
                 "   implies: 'library_search_directories'",
                 "   implies: 'input_param_flags'",
                 "   implies: 'libraries_to_link'",
+                "   implies: 'toolchain_flags'",
                 "}",
                 "action_config {",
                 "   config_name: 'c++-link-static-library'",
@@ -331,6 +333,18 @@ public class CppLinkActionConfigs {
                 "           flag: '-pie'",
                 "       }",
                 "   }",
+                "}",
+                "feature {",
+                "   name: 'toolchain_flags'",
+                "   flag_set {",
+                "       expand_if_all_available: 'toolchain_flags'",
+                "       action: 'c++-link-executable'",
+                "       action: 'c++-link-dynamic-library'",
+                "       flag_group {",
+                "           iterate_over: 'toolchain_flags'",
+                "           flag: '%{toolchain_flags}'",
+                "       }",
+                "   }",
                 "}"));
   }
 
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 145088e321..6b5a6ec296 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
@@ -308,7 +308,7 @@ public final class LinkCommandLine extends CommandLine {
     }
   }
   
-  private void addToolchainFlags(List<String> argv) {
+  private ImmutableList<String> getToolchainFlags() {
     boolean fullyStatic = (linkStaticness == LinkStaticness.FULLY_STATIC);
     boolean mostlyStatic = (linkStaticness == LinkStaticness.MOSTLY_STATIC);
     boolean sharedLinkopts =
@@ -316,6 +316,8 @@ public final class LinkCommandLine extends CommandLine {
             || linkopts.contains("-shared")
             || cppConfiguration.getLinkOptions().contains("-shared");
 
+    List<String> toolchainFlags = new ArrayList<>();
+
     /*
      * For backwards compatibility, linkopts come _after_ inputFiles.
      * This is needed to allow linkopts to contain libraries and
@@ -328,22 +330,22 @@ public final class LinkCommandLine extends CommandLine {
      * (global defaults, per-target linkopts, and command-line linkopts),
      * we have no idea what the right order should be, or if anyone cares.
      */
-    argv.addAll(linkopts);
+    toolchainFlags.addAll(linkopts);
     // Extra toolchain link options based on the output's link staticness.
     if (fullyStatic) {
-      argv.addAll(cppConfiguration.getFullyStaticLinkOptions(features, sharedLinkopts));
+      toolchainFlags.addAll(cppConfiguration.getFullyStaticLinkOptions(features, sharedLinkopts));
     } else if (mostlyStatic) {
-      argv.addAll(cppConfiguration.getMostlyStaticLinkOptions(features, sharedLinkopts));
+      toolchainFlags.addAll(cppConfiguration.getMostlyStaticLinkOptions(features, sharedLinkopts));
     } else {
-      argv.addAll(cppConfiguration.getDynamicLinkOptions(features, sharedLinkopts));
+      toolchainFlags.addAll(cppConfiguration.getDynamicLinkOptions(features, sharedLinkopts));
     }
 
     // Extra test-specific link options.
     if (useTestOnlyFlags) {
-      argv.addAll(cppConfiguration.getTestOnlyLinkOptions());
+      toolchainFlags.addAll(cppConfiguration.getTestOnlyLinkOptions());
     }
 
-    argv.addAll(cppConfiguration.getLinkOptions());
+    toolchainFlags.addAll(cppConfiguration.getLinkOptions());
 
     // -pie is not compatible with shared and should be
     // removed when the latter is part of the link command. Should we need to further
@@ -351,13 +353,15 @@ public final class LinkCommandLine extends CommandLine {
     // 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.
     if (linkTargetType == LinkTargetType.DYNAMIC_LIBRARY) {
-      Iterables.removeIf(argv, Predicates.equalTo("-pie"));
+      Iterables.removeIf(toolchainFlags, Predicates.equalTo("-pie"));
     }
 
     // Fission mode: debug info is in .dwo files instead of .o files. Inform the linker of this.
     if (linkTargetType.staticness() == Staticness.DYNAMIC && cppConfiguration.useFission()) {
-      argv.add("-Wl,--gdb-index");
+      toolchainFlags.add("-Wl,--gdb-index");
     }
+
+    return ImmutableList.copyOf(toolchainFlags);
   }
 
   /**
@@ -375,14 +379,26 @@ public final class LinkCommandLine extends CommandLine {
     switch (linkTargetType) {
       case EXECUTABLE:
         argv.add(cppConfiguration.getCppExecutable().getPathString());
-        argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
-        addToolchainFlags(argv);
+        argv.addAll(
+            featureConfiguration.getCommandLine(
+                actionName,
+                new Variables.Builder()
+                    .addAll(variables)
+                    .addStringSequenceVariable(
+                        CppLinkActionBuilder.TOOLCHAIN_FLAGS_VARIABLE, getToolchainFlags())
+                    .build()));
         break;
 
       case DYNAMIC_LIBRARY:
         argv.add(toolPath);
-        argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
-        addToolchainFlags(argv);
+        argv.addAll(
+            featureConfiguration.getCommandLine(
+                actionName,
+                new Variables.Builder()
+                    .addAll(variables)
+                    .addStringSequenceVariable(
+                        CppLinkActionBuilder.TOOLCHAIN_FLAGS_VARIABLE, getToolchainFlags())
+                    .build()));
         break;
 
       case STATIC_LIBRARY: