Fix linker inputs for cc_shared_library linking
This CL makes cc_shared_library and cc_binary account for the possibility that
a single rule (i.e. a single owner) may place more than one linker_input in a
linking context. There is no rule saying that this cannot be the case but until
now the implementation of cc_library did place just a single linker input per
owner.
The Starlark implementation of cc_library is slightly different and has made
this bug surface. Rather than change Starlark cc_library we fix
cc_shared_library to have the right behavior. We won't control every C++ rule
people will write using the Starlark API and therefore can't guarantee that
some other rule won't place more than one linker_input per owner.
The existing tests already broke with the Starlark cc_library implementation.
RELNOTES:none
PiperOrigin-RevId: 430195224
diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
index 2e381ae..74a62ee 100644
--- a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
+++ b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
@@ -452,12 +452,13 @@
(link_statically_labels, link_dynamically_labels) = _separate_static_and_dynamic_link_libraries(graph_structure_aspect_nodes, can_be_linked_dynamically)
- owners_seen = {}
+ linker_inputs_seen = {}
for linker_input in linker_inputs:
- owner = str(linker_input.owner)
- if owner in owners_seen:
+ stringified_linker_input = cc_helper.stringify_linker_input(linker_input)
+ if stringified_linker_input in linker_inputs_seen:
continue
- owners_seen[owner] = True
+ linker_inputs_seen[stringified_linker_input] = True
+ owner = str(linker_input.owner)
if owner not in link_dynamically_labels and (owner in link_statically_labels or _get_canonical_form(ctx.label) == owner):
if owner in link_once_static_libs_map:
fail(owner + " is already linked statically in " + link_once_static_libs_map[owner] + " but not exported.")
diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
index 97a2691..640c332 100644
--- a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
+++ b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
@@ -435,6 +435,27 @@
inputs.extend(linker_input.additional_inputs)
return depset(inputs, order = "topological")
+def _stringify_linker_input(linker_input):
+ parts = []
+ parts.append(str(linker_input.owner))
+ for library in linker_input.libraries:
+ if library.static_library != None:
+ parts.append(library.static_library.path)
+ if library.pic_static_library != None:
+ parts.append(library.pic_static_library.path)
+ if library.dynamic_library != None:
+ parts.append(library.dynamic_library.path)
+ if library.interface_library != None:
+ parts.append(library.interface_library.path)
+
+ for additional_input in linker_input.additional_inputs:
+ parts.append(additional_input.path)
+
+ for linkstamp in linker_input.linkstamps:
+ parts.append(linkstamp.file.path)
+
+ return "".join(parts)
+
cc_helper = struct(
merge_cc_debug_contexts = _merge_cc_debug_contexts,
is_code_coverage_enabled = _is_code_coverage_enabled,
@@ -462,4 +483,5 @@
dll_hash_suffix = _dll_hash_suffix,
get_windows_def_file_for_linking = _get_windows_def_file_for_linking,
generate_def_file = _generate_def_file,
+ stringify_linker_input = _stringify_linker_input,
)
diff --git a/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl b/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl
index 7a8c59d..16baf79 100644
--- a/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl
+++ b/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl
@@ -263,12 +263,14 @@
precompiled_only_dynamic_libraries = []
unaccounted_for_libs = []
exports = {}
- owners_seen = {}
+ linker_inputs_seen = {}
+ dynamic_only_roots = {}
for linker_input in dependency_linker_inputs:
- owner = str(linker_input.owner)
- if owner in owners_seen:
+ stringified_linker_input = cc_helper.stringify_linker_input(linker_input)
+ if stringified_linker_input in linker_inputs_seen:
continue
- owners_seen[owner] = True
+ linker_inputs_seen[stringified_linker_input] = True
+ owner = str(linker_input.owner)
if owner in link_dynamically_labels:
dynamic_linker_input = transitive_exports[owner]
linker_inputs.append(dynamic_linker_input)
@@ -278,7 +280,6 @@
link_once_static_libs_map[owner] + " but not exported")
is_direct_export = owner in direct_exports
-
dynamic_only_libraries = []
static_libraries = []
for library in linker_input.libraries:
@@ -288,15 +289,14 @@
static_libraries.append(library)
if len(dynamic_only_libraries):
+ precompiled_only_dynamic_libraries.extend(dynamic_only_libraries)
if not len(static_libraries):
if is_direct_export:
- fail("Do not place libraries which only contain a precompiled dynamic library in roots.")
-
- precompiled_only_dynamic_libraries.extend(dynamic_only_libraries)
-
- if not len(static_libraries):
+ dynamic_only_roots[owner] = True
linker_inputs.append(linker_input)
continue
+ if len(static_libraries) and owner in dynamic_only_roots:
+ dynamic_only_roots.pop(owner)
if is_direct_export:
wrapped_library = _wrap_static_library_with_alwayslink(
@@ -334,6 +334,14 @@
else:
unaccounted_for_libs.append(linker_input.owner)
+ if dynamic_only_roots:
+ message = ("Do not place libraries which only contain a " +
+ "precompiled dynamic library in roots. The following " +
+ "libraries only have precompiled dynamic libraries:\n")
+ for dynamic_only_root in dynamic_only_roots:
+ message += dynamic_only_root + "\n"
+ fail(message)
+
_throw_error_if_unaccounted_libs(unaccounted_for_libs)
return (exports, linker_inputs, link_once_static_libs, precompiled_only_dynamic_libraries)
