From e61528cc8dcb98faa2549c0e790f9b1a8664c46b Mon Sep 17 00:00:00 2001 From: Yun Peng Date: Thu, 6 Oct 2016 10:55:54 +0000 Subject: [PATCH] Add action_config and feature for linking on Windows Also implemented whole archive feature on Windows 1. Pulled action_configs and features from CppLinkActionConfigs.java 2. Deleted many features not working on Windows. (AFAIK) including: symbol_counts output_execpath_flags_executable global_whole_archive_open runtime_root_flags global_whole_archive_close force_pic_flags 3. Added c++-link-interface-dynamic-library action config Not sure there is such thing on Windows, but it's currently in MANDATORY_LINK_TARGET_TYPES 4. Added build variable "whole_archive_object_files_params" 5. Automatically detect if linker supports /WHOLEARCHIVE and use the corresponding build variable -- Change-Id: I232798a0eb1a3291f972b313a81e678b0121d58c Reviewed-on: https://bazel-review.googlesource.com/#/c/6414 MOS_MIGRATED_REVID=135342093 --- .../lib/rules/cpp/CppLinkActionBuilder.java | 54 +++++- .../lib/rules/cpp/LinkBuildVariablesTest.java | 5 + tools/cpp/CROSSTOOL.tpl | 157 ++++++++++++++++++ tools/cpp/cc_configure.bzl | 22 ++- 4 files changed, 229 insertions(+), 9 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 f0ca362f9a0ea0..178d0688fb6968 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 @@ -86,6 +86,14 @@ public class CppLinkActionBuilder { public static final String WHOLE_ARCHIVE_LINKER_INPUT_PARAMS_VARIABLE = "whole_archive_linker_params"; + /** + * A build variable for flags providing object files from libraries that were supposed to go in a + * -whole_archive block. Old MSVC linker doesn't support /WHOLEARCHIVE:[LIB] option, so we link + * object files directly as a workaround. + */ + public static final String WHOLE_ARCHIVE_OBJECT_FILES_PARAMS_VARIABLE = + "whole_archive_object_files_params"; + /** A build variable whose presence indicates that whole archive flags should be applied. */ public static final String GLOBAL_WHOLE_ARCHIVE_VARIABLE = "global_whole_archive"; @@ -1120,6 +1128,7 @@ private static class LinkArgCollector { Set libopts; List linkerInputParams; List wholeArchiveLinkerInputParams; + List wholeArchiveObjectFilesParams; List noWholeArchiveInputs; public void setRpathRoot(String rPathRoot) { @@ -1142,6 +1151,10 @@ public void setWholeArchiveLinkerInputParams(List wholeArchiveInputParam this.wholeArchiveLinkerInputParams = wholeArchiveInputParams; } + public void setWholeArchiveObjectFilesParams(List wholeArchiveObjectFilesParams) { + this.wholeArchiveObjectFilesParams = wholeArchiveObjectFilesParams; + } + public void setNoWholeArchiveInputs(List noWholeArchiveInputs) { this.noWholeArchiveInputs = noWholeArchiveInputs; } @@ -1166,6 +1179,10 @@ public List getWholeArchiveLinkerInputParams() { return wholeArchiveLinkerInputParams; } + public List getWholeArchiveObjectFilesParams() { + return wholeArchiveObjectFilesParams; + } + public List getNoWholeArchiveInputs() { return noWholeArchiveInputs; } @@ -1253,6 +1270,9 @@ public void addVariables(Variables.Builder buildVariables) { buildVariables.addSequenceVariable( WHOLE_ARCHIVE_LINKER_INPUT_PARAMS_VARIABLE, linkArgCollector.getWholeArchiveLinkerInputParams()); + buildVariables.addSequenceVariable( + WHOLE_ARCHIVE_OBJECT_FILES_PARAMS_VARIABLE, + linkArgCollector.getWholeArchiveObjectFilesParams()); // global archive if (needWholeArchive) { @@ -1417,6 +1437,7 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) { } List wholeArchiveInputParams = new ArrayList<>(); + List wholeArchiveObjectFilesParams = new ArrayList<>(); List standardArchiveInputParams = new ArrayList<>(); for (LinkerInput input : linkerInputs) { @@ -1434,7 +1455,11 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) { input, standardArchiveInputParams, libOpts, solibDir, rpathRoot); } else { addStaticInputLinkOptions( - input, wholeArchiveInputParams, standardArchiveInputParams, ltoMap); + input, + wholeArchiveInputParams, + wholeArchiveObjectFilesParams, + standardArchiveInputParams, + ltoMap); } } @@ -1454,8 +1479,10 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) { includeRuntimeSolibDir = true; addDynamicInputLinkOptions(input, optionsList, libOpts, solibDir, rpathRoot); } else { - addStaticInputLinkOptions(input, + addStaticInputLinkOptions( + input, needWholeArchive ? noWholeArchiveInputs : wholeArchiveInputParams, + needWholeArchive ? null : wholeArchiveObjectFilesParams, needWholeArchive ? noWholeArchiveInputs : standardArchiveInputParams, ltoMap); } @@ -1476,6 +1503,7 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) { linkArgCollector.setLinkerInputParams(standardArchiveInputParams); linkArgCollector.setWholeArchiveLinkerInputParams(wholeArchiveInputParams); + linkArgCollector.setWholeArchiveObjectFilesParams(wholeArchiveObjectFilesParams); linkArgCollector.setNoWholeArchiveInputs(noWholeArchiveInputs); if (ltoMap != null) { @@ -1530,7 +1558,10 @@ private void addDynamicInputLinkOptions( * be supplied for LTO final links. */ private void addStaticInputLinkOptions( - LinkerInput input, List wholeArchiveOptions, List standardOptions, + LinkerInput input, + List wholeArchiveOptions, + List wholeArchiveObjectFilesOptions, + List standardOptions, @Nullable Map ltoMap) { Preconditions.checkState(!(input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY)); // If we had any LTO artifacts, ltoMap whould be non-null. In that case, @@ -1575,10 +1606,19 @@ private void addStaticInputLinkOptions( return; } - if (input.isFake()) { - options.add(Link.FAKE_OBJECT_PREFIX + inputArtifact.getExecPathString()); - } else { - options.add(inputArtifact.getExecPathString()); + final String fakePrefix = input.isFake() ? Link.FAKE_OBJECT_PREFIX : ""; + + options.add(fakePrefix + inputArtifact.getExecPathString()); + + if (input.containsObjectFiles() && inputNeedsWholeArchive(input)) { + for (Artifact objectFile : input.getObjectFiles()) { + if (ltoMap != null && ltoMap.remove(objectFile) != null) { + // The LTO artifacts that should be included in the final link + // are listed in the linkerParamsFile. + continue; + } + wholeArchiveObjectFilesOptions.add(fakePrefix + objectFile.getExecPathString()); + } } } } 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 056fb3bb8340ba..7e9a9dce856b9c 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 @@ -160,4 +160,9 @@ public void testGlobalWholeArchiveOrWholeArchiveBuildVariables() throws Exceptio assertThat(wholeArchiveInputVariableValue).isEmpty(); ; } + + /** + * TODO(pcloudy): Add test for testing that necessary build variables are populated + * when alwayslink=1. + */ } diff --git a/tools/cpp/CROSSTOOL.tpl b/tools/cpp/CROSSTOOL.tpl index 740b68cbeeeba0..f33e9afbe1c471 100644 --- a/tools/cpp/CROSSTOOL.tpl +++ b/tools/cpp/CROSSTOOL.tpl @@ -288,6 +288,163 @@ toolchain { } } + action_config { + config_name: 'c++-link-executable' + action_name: 'c++-link-executable' + tool { + tool_path: 'DUMMY_TOOL' + } + implies: 'linkstamps' + implies: 'output_execpath_flags' + implies: 'input_param_flags' + } + + action_config { + config_name: 'c++-link-dynamic-library' + action_name: 'c++-link-dynamic-library' + tool { + tool_path: 'DUMMY_TOOL' + } + implies: 'shared_flag' + implies: 'linkstamps' + implies: 'output_execpath_flags' + implies: 'input_param_flags' + } + + action_config { + config_name: 'c++-link-static-library' + action_name: 'c++-link-static-library' + tool { + tool_path: 'DUMMY_TOOL' + } + implies: 'input_param_flags' + } + + action_config { + config_name: 'c++-link-alwayslink-static-library' + action_name: 'c++-link-alwayslink-static-library' + tool { + tool_path: 'DUMMY_TOOL' + } + implies: 'input_param_flags' + } + + # TODO(pcloudy): The following action_config is listed in MANDATORY_LINK_TARGET_TYPES. + # But do we really need them on Windows? + action_config { + config_name: 'c++-link-pic-static-library' + action_name: 'c++-link-pic-static-library' + tool { + tool_path: 'DUMMY_TOOL' + } + implies: 'input_param_flags' + } + + action_config { + config_name: 'c++-link-alwayslink-pic-static-library' + action_name: 'c++-link-alwayslink-pic-static-library' + tool { + tool_path: 'DUMMY_TOOL' + } + implies: 'input_param_flags' + } + + action_config { + config_name: 'c++-link-interface-dynamic-library' + action_name: 'c++-link-interface-dynamic-library' + tool { + tool_path: 'DUMMY_TOOL' + } + } + + feature { + name: 'shared_flag' + flag_set { + action: 'c++-link-dynamic-library' + flag_group { + flag: '/DLL' + } + } + } + + feature { + name: 'linkstamps' + flag_set { + action: 'c++-link-executable' + action: 'c++-link-dynamic-library' + expand_if_all_available: 'linkstamp_paths' + flag_group { + flag: '%{linkstamp_paths}' + } + } + } + + feature { + name: 'output_execpath_flags' + flag_set { + expand_if_all_available: 'output_execpath' + action: 'c++-link-executable' + action: 'c++-link-dynamic-library' + flag_group { + flag: '/OUT:%{output_execpath}' + } + } + } + + feature { + name: 'input_param_flags' + flag_set { + expand_if_all_available: 'libopts' + action: 'c++-link-executable' + action: 'c++-link-dynamic-library' + 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: '%{libopts}' + } + } + flag_set { + expand_if_all_available: 'whole_archive_linker_params' + action: 'c++-link-executable' + action: 'c++-link-dynamic-library' + 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 { + # If MSVC linker supports /WHOLEARCHIVE, this field will be enabled + %{whole_archive_linker_params} + } + } + flag_set { + expand_if_all_available: 'whole_archive_object_files_params' + action: 'c++-link-executable' + action: 'c++-link-dynamic-library' + 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 { + # If MSVC linker doesn't support /WHOLEARCHIVE, this field will be enabled + %{whole_archive_object_files_params} + } + } + flag_set { + expand_if_all_available: 'linker_input_params' + action: 'c++-link-executable' + action: 'c++-link-dynamic-library' + 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_input_params}' + } + } + } + compilation_mode_flags { mode: DBG compiler_flag: "/DDEBUG=1" diff --git a/tools/cpp/cc_configure.bzl b/tools/cpp/cc_configure.bzl index 69daae582211c4..aeb07155367007 100644 --- a/tools/cpp/cc_configure.bzl +++ b/tools/cpp/cc_configure.bzl @@ -428,6 +428,12 @@ def _find_env_vars(repository_ctx, vs_path): return env_map +def _is_support_whole_archive(repository_ctx, vs_dir): + """Run MSVC linker alone to see if it supports /WHOLEARCHIVE.""" + result = _execute(repository_ctx, [vs_dir + "/VC/BIN/amd64/link"]) + return result.find("/WHOLEARCHIVE") != -1 + + def _tpl(repository_ctx, tpl, substitutions={}, out=None): if not out: out = tpl @@ -488,12 +494,22 @@ def _impl(repository_ctx): for path in include_paths.split(";"): if path: cxx_include_directories.append(("cxx_builtin_include_directory: \"%s\"" % path)) + + if _is_support_whole_archive(repository_ctx, vs_path): + whole_archive_linker_params = "flag: '/WHOLEARCHIVE:%{whole_archive_linker_params}'" + whole_archive_object_files_params = "" + else: + whole_archive_linker_params = "" + whole_archive_object_files_params = "flag: '%{whole_archive_object_files_params}'" + _tpl(repository_ctx, "CROSSTOOL", { "%{cpu}": cpu_value, "%{content}": _get_windows_crosstool_content(repository_ctx), "%{opt_content}": "", "%{dbg_content}": "", - "%{cxx_builtin_include_directory}": "\n".join(cxx_include_directories) + "%{cxx_builtin_include_directory}": "\n".join(cxx_include_directories), + "%{whole_archive_linker_params}": whole_archive_linker_params, + "%{whole_archive_object_files_params}": whole_archive_object_files_params, }) else: darwin = cpu_value == "darwin" @@ -518,7 +534,9 @@ def _impl(repository_ctx): _build_tool_path(tool_paths), "%{opt_content}": _build_crosstool(opt_content, " "), "%{dbg_content}": _build_crosstool(dbg_content, " "), - "%{cxx_builtin_include_directory}": "" + "%{cxx_builtin_include_directory}": "", + "%{whole_archive_linker_params}": "", + "%{whole_archive_object_files_params}": "", })