Introduce strip_debug_symbols feature to crosstool
This cl removes hard coded -Wl,-S flag from Blaze and moves it to the
crosstool.
--
PiperOrigin-RevId: 149525225
MOS_MIGRATED_REVID=149525225
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 8ce165f..342ad72 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
@@ -1115,10 +1115,6 @@
List<String> result = new ArrayList<>();
result.addAll(commonLinkOptions);
- if (stripBinaries) {
- result.add("-Wl,-S");
- }
-
result.addAll(linkOptionsFromCompilationMode.get(compilationMode));
result.addAll(linkOptionsFromLipoMode.get(lipoMode));
result.addAll(linkOptionsFromLinkingMode.get(linkingMode));
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 f802739..9af79a7 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
@@ -124,6 +124,9 @@
/** A build variable whose presence indicates that PIC code should be generated. */
public static final String FORCE_PIC_VARIABLE = "force_pic";
+ /** A build variable whose presence indicates that the debug symbols should be stripped. */
+ public static final String STRIP_DEBUG_SYMBOLS_VARIABLE = "strip_debug_symbols";
+
/** A build variable whose presence indicates that this action is a cc_test linking action. */
public static final String IS_CC_TEST_LINK_ACTION_VARIABLE = "is_cc_test_link_action";
@@ -1313,6 +1316,10 @@
buildVariables.addStringVariable(FORCE_PIC_VARIABLE, "");
}
+ if (cppConfiguration.shouldStripBinaries()) {
+ buildVariables.addStringVariable(STRIP_DEBUG_SYMBOLS_VARIABLE, "");
+ }
+
if (useTestOnlyFlags()) {
buildVariables.addStringVariable(IS_CC_TEST_LINK_ACTION_VARIABLE, "");
} else {
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 2cb39ab..14a053b 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
@@ -63,6 +63,7 @@
" tool_path: 'DUMMY_TOOL'",
" }",
" implies: 'symbol_counts'",
+ " implies: 'strip_debug_symbols'",
" implies: 'linkstamps'",
" implies: 'output_execpath_flags_executable'",
" implies: 'runtime_library_search_directories'",
@@ -81,6 +82,7 @@
" implies: 'build_interface_libraries'",
" implies: 'dynamic_library_linker_tool'",
" implies: 'symbol_counts'",
+ " implies: 'strip_debug_symbols'",
" implies: 'shared_flag'",
" implies: 'linkstamps'",
" implies: 'output_execpath_flags'",
@@ -96,6 +98,7 @@
" tool {",
" tool_path: 'DUMMY_TOOL'",
" }",
+ " implies: 'strip_debug_symbols'",
" implies: 'runtime_library_search_directories'",
" implies: 'library_search_directories'",
" implies: 'libraries_to_link'",
@@ -107,6 +110,7 @@
" tool {",
" tool_path: 'DUMMY_TOOL'",
" }",
+ " implies: 'strip_debug_symbols'",
" implies: 'runtime_library_search_directories'",
" implies: 'library_search_directories'",
" implies: 'libraries_to_link'",
@@ -118,6 +122,7 @@
" tool {",
" tool_path: 'DUMMY_TOOL'",
" }",
+ " implies: 'strip_debug_symbols'",
" implies: 'runtime_library_search_directories'",
" implies: 'library_search_directories'",
" implies: 'libraries_to_link'",
@@ -129,6 +134,7 @@
" tool {",
" tool_path: 'DUMMY_TOOL'",
" }",
+ " implies: 'strip_debug_symbols'",
" implies: 'runtime_library_search_directories'",
" implies: 'library_search_directories'",
" implies: 'libraries_to_link'",
@@ -444,6 +450,18 @@
" }",
"}",
"feature {",
+ " name: 'strip_debug_symbols'",
+ " flag_set {",
+ " action: 'c++-link-executable'",
+ " action: 'c++-link-dynamic-library'",
+ " action: 'c++-link-interface-dynamic-library'",
+ " flag_group {",
+ " expand_if_all_available: 'strip_debug_symbols'",
+ " flag: '-Wl,-S'",
+ " }",
+ " }",
+ "}",
+ "feature {",
" name: 'linker_param_file'",
" flag_set {",
" expand_if_all_available: 'linker_param_file'",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
index 73c0119..fce9586 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
@@ -209,16 +209,16 @@
assertEquals(Arrays.<String>asList(), toolchain.getLinkOptions());
assertEquals(
- Arrays.asList("linker", "-Wl,-S", "linker-fastbuild", "fully static"),
+ Arrays.asList("linker", "linker-fastbuild", "fully static"),
toolchain.getFullyStaticLinkOptions(NO_FEATURES, false));
assertEquals(
- Arrays.asList("linker", "-Wl,-S", "linker-fastbuild", "dynamic"),
+ Arrays.asList("linker", "linker-fastbuild", "dynamic"),
toolchain.getDynamicLinkOptions(NO_FEATURES, false));
assertEquals(
- Arrays.asList("linker", "-Wl,-S", "linker-fastbuild", "mostly static", "solinker"),
+ Arrays.asList("linker", "linker-fastbuild", "mostly static", "solinker"),
toolchain.getFullyStaticLinkOptions(NO_FEATURES, true));
assertEquals(
- Arrays.asList("linker", "-Wl,-S", "linker-fastbuild", "dynamic", "solinker"),
+ Arrays.asList("linker", "linker-fastbuild", "dynamic", "solinker"),
toolchain.getDynamicLinkOptions(NO_FEATURES, true));
assertEquals(Arrays.asList("objcopy"), toolchain.getObjCopyOptionsForEmbedding());
@@ -514,7 +514,6 @@
Arrays.asList(
"linker-flag-A-1",
"linker-flag-A-2",
- "-Wl,-S",
"linker-fastbuild-flag-A-1",
"linker-fastbuild-flag-A-2",
"solinker-flag-A-1",
@@ -527,7 +526,6 @@
Arrays.asList(
"linker-flag-A-1",
"linker-flag-A-2",
- "-Wl,-S",
"linker-fastbuild-flag-A-1",
"linker-fastbuild-flag-A-2",
"fully-static-flag-A-1",
@@ -541,7 +539,6 @@
Arrays.asList(
"linker-flag-A-1",
"linker-flag-A-2",
- "-Wl,-S",
"linker-dbg-flag-A-1",
"linker-dbg-flag-A-2"),
toolchainA.configureLinkerOptions(
@@ -553,7 +550,6 @@
Arrays.asList(
"linker-flag-A-1",
"linker-flag-A-2",
- "-Wl,-S",
"fully-static-flag-A-1",
"fully-static-flag-A-2"),
toolchainA.configureLinkerOptions(
@@ -566,7 +562,6 @@
Arrays.asList(
"linker-flag-A-1",
"linker-flag-A-2",
- "-Wl,-S",
"fully-static-flag-A-1",
"fully-static-flag-A-2"),
toolchainA.configureLinkerOptions(
@@ -628,23 +623,23 @@
assertThat(toolchainC.getCOptions()).isEmpty();
assertThat(toolchainC.getCxxOptions(NO_FEATURES)).isEmpty();
assertThat(toolchainC.getUnfilteredCompilerOptions(NO_FEATURES)).isEmpty();
- assertEquals(Arrays.asList("-Wl,-S"), toolchainC.getDynamicLinkOptions(NO_FEATURES, true));
+ assertEquals(Collections.EMPTY_LIST, toolchainC.getDynamicLinkOptions(NO_FEATURES, true));
assertEquals(
- Arrays.asList("-Wl,-S"),
+ Collections.EMPTY_LIST,
toolchainC.configureLinkerOptions(
CompilationMode.FASTBUILD,
LipoMode.OFF,
LinkingMode.FULLY_STATIC,
new PathFragment("hello-world/ld")));
assertEquals(
- Arrays.asList("-Wl,-S"),
+ Collections.EMPTY_LIST,
toolchainC.configureLinkerOptions(
CompilationMode.DBG,
LipoMode.OFF,
LinkingMode.DYNAMIC,
new PathFragment("hello-world/ld")));
assertEquals(
- Arrays.asList("-Wl,-S"),
+ Collections.EMPTY_LIST,
toolchainC.configureLinkerOptions(
CompilationMode.OPT,
LipoMode.OFF,
@@ -682,7 +677,6 @@
Arrays.asList(
"linker-flag-B-1",
"linker-flag-B-2",
- "-Wl,-S",
"linker-dbg-flag-B-1",
"linker-dbg-flag-B-2",
"linker-lipo_" + lipoSuffix),
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
index 6ea0a13..ba5ef20 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
@@ -266,4 +266,29 @@
assertThat(binaryVariables.isAvailable(CppLinkActionBuilder.IS_CC_TEST_LINK_ACTION_VARIABLE))
.isFalse();
}
+
+ @Test
+ public void testStripBinariesIsEnabledWhenStripModeIsAlwaysNoMatterWhat() throws Exception {
+ scratch.file("x/BUILD", "cc_binary(name = 'foo', srcs = ['a.cc'])");
+ scratch.file("x/a.cc");
+
+ assertStripBinaryVariableIsPresent("always", "opt", true);
+ assertStripBinaryVariableIsPresent("always", "fastbuild", true);
+ assertStripBinaryVariableIsPresent("always", "dbg", true);
+ assertStripBinaryVariableIsPresent("sometimes", "opt", false);
+ assertStripBinaryVariableIsPresent("sometimes", "fastbuild", true);
+ assertStripBinaryVariableIsPresent("sometimes", "dbg", false);
+ assertStripBinaryVariableIsPresent("never", "opt", false);
+ assertStripBinaryVariableIsPresent("never", "fastbuild", false);
+ assertStripBinaryVariableIsPresent("never", "dbg", false);
+ }
+
+ private void assertStripBinaryVariableIsPresent(
+ String stripMode, String compilationMode, boolean isEnabled) throws Exception {
+ useConfiguration("--strip=" + stripMode, "--compilation_mode=" + compilationMode);
+ ConfiguredTarget target = getConfiguredTarget("//x:foo");
+ Variables variables = getLinkBuildVariables(target, LinkTargetType.EXECUTABLE);
+ assertThat(variables.isAvailable(CppLinkActionBuilder.STRIP_DEBUG_SYMBOLS_VARIABLE))
+ .isEqualTo(isEnabled);
+ }
}
diff --git a/tools/cpp/CROSSTOOL.tpl b/tools/cpp/CROSSTOOL.tpl
index 5b38292..1a18ebe 100644
--- a/tools/cpp/CROSSTOOL.tpl
+++ b/tools/cpp/CROSSTOOL.tpl
@@ -337,6 +337,7 @@
tool {
tool_path: 'wrapper/bin/msvc_link.bat'
}
+ implies: 'strip_debug_symbols'
implies: 'linkstamps'
implies: 'output_execpath_flags'
implies: 'input_param_flags'
@@ -350,6 +351,7 @@
tool {
tool_path: 'wrapper/bin/msvc_link.bat'
}
+ implies: 'strip_debug_symbols'
implies: 'shared_flag'
implies: 'linkstamps'
implies: 'output_execpath_flags'
@@ -407,6 +409,7 @@
tool {
tool_path: 'wrapper/bin/msvc_link.bat'
}
+ implies: 'strip_debug_symbols'
implies: 'linker_param_file'
}
@@ -415,6 +418,19 @@
}
feature {
+ name: 'strip_debug_symbols'
+ flag_set {
+ action: 'c++-link-executable'
+ action: 'c++-link-dynamic-library'
+ action: 'c++-link-interface-dynamic-library'
+ flag_group {
+ expand_if_all_available: 'strip_debug_symbols'
+ flag: '-Wl,-S'
+ }
+ }
+ }
+
+ feature {
name: 'shared_flag'
flag_set {
action: 'c++-link-dynamic-library'
