Skip to content

Commit

Permalink
Add action_config and feature for linking on Windows
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@meteorcloudy @katre
meteorcloudy authored and katre committed Oct 7, 2016
1 parent 2d03853 commit e61528c
Show file tree
Hide file tree
Showing 4 changed files with 229 additions and 9 deletions.
54 changes: 47 additions & 7 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -1120,6 +1128,7 @@ private static class LinkArgCollector {
Set<String> libopts;
List<String> linkerInputParams;
List<String> wholeArchiveLinkerInputParams;
List<String> wholeArchiveObjectFilesParams;
List<String> noWholeArchiveInputs;

public void setRpathRoot(String rPathRoot) {
Expand All @@ -1142,6 +1151,10 @@ public void setWholeArchiveLinkerInputParams(List<String> wholeArchiveInputParam
this.wholeArchiveLinkerInputParams = wholeArchiveInputParams;
}

public void setWholeArchiveObjectFilesParams(List<String> wholeArchiveObjectFilesParams) {
this.wholeArchiveObjectFilesParams = wholeArchiveObjectFilesParams;
}

public void setNoWholeArchiveInputs(List<String> noWholeArchiveInputs) {
this.noWholeArchiveInputs = noWholeArchiveInputs;
}
Expand All @@ -1166,6 +1179,10 @@ public List<String> getWholeArchiveLinkerInputParams() {
return wholeArchiveLinkerInputParams;
}

public List<String> getWholeArchiveObjectFilesParams() {
return wholeArchiveObjectFilesParams;
}

public List<String> getNoWholeArchiveInputs() {
return noWholeArchiveInputs;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1417,6 +1437,7 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {
}

List<String> wholeArchiveInputParams = new ArrayList<>();
List<String> wholeArchiveObjectFilesParams = new ArrayList<>();
List<String> standardArchiveInputParams = new ArrayList<>();

for (LinkerInput input : linkerInputs) {
Expand All @@ -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);
}
}

Expand All @@ -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);
}
Expand All @@ -1476,6 +1503,7 @@ private void addInputFileLinkOptions(LinkArgCollector linkArgCollector) {

linkArgCollector.setLinkerInputParams(standardArchiveInputParams);
linkArgCollector.setWholeArchiveLinkerInputParams(wholeArchiveInputParams);
linkArgCollector.setWholeArchiveObjectFilesParams(wholeArchiveObjectFilesParams);
linkArgCollector.setNoWholeArchiveInputs(noWholeArchiveInputs);

if (ltoMap != null) {
Expand Down Expand Up @@ -1530,7 +1558,10 @@ private void addDynamicInputLinkOptions(
* be supplied for LTO final links.
*/
private void addStaticInputLinkOptions(
LinkerInput input, List<String> wholeArchiveOptions, List<String> standardOptions,
LinkerInput input,
List<String> wholeArchiveOptions,
List<String> wholeArchiveObjectFilesOptions,
List<String> standardOptions,
@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,
Expand Down Expand Up @@ -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());
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
}
157 changes: 157 additions & 0 deletions tools/cpp/CROSSTOOL.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 20 additions & 2 deletions tools/cpp/cc_configure.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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}": "",
})


Expand Down

0 comments on commit e61528c

Please sign in to comment.