Skip to content

Commit ba5c740

Browse files
oquenchilcopybara-github
oquenchil
authored and
copybara-github
committed
Link unused targets listed in cc_shared_library.dynamic_deps
It used to be an invariant in the cc_shared_library design that the rule would only link the dynamic_deps (other cc_shared_libraries) that were exporting cc_library targets coming from the cc_library graph (in other words targets reachable from cc_shared_library.deps, formerly cc_shared_library.roots). However, these were not being linked silently without giving an error. It also turns out that it's a valid use case not to require the library in the cc_library graph when for example owners of the cc_shared_library target want users to only depend on the dynamic library and make their cc_library private so that it's never linked statically. cc_binary already allowed this and linked the unused direct dynamic_deps. RELNOTES:none PiperOrigin-RevId: 544076791 Change-Id: I78668c6cc26676922cd1478e290019ca4fccd675
1 parent d14a56f commit ba5c740

File tree

5 files changed

+83
-59
lines changed

5 files changed

+83
-59
lines changed

‎src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"""cc_binary Starlark implementation replacing native"""
1616

1717
load(":common/cc/semantics.bzl", "semantics")
18-
load(":common/cc/cc_shared_library.bzl", "CcSharedLibraryInfo", "GraphNodeInfo", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "sort_linker_inputs", "throw_linked_but_not_exported_errors")
18+
load(":common/cc/cc_shared_library.bzl", "GraphNodeInfo", "add_unused_dynamic_deps", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "sort_linker_inputs", "throw_linked_but_not_exported_errors")
1919
load(":common/cc/cc_helper.bzl", "cc_helper", "linker_mode")
2020
load(":common/cc/cc_info.bzl", "CcInfo")
2121
load(":common/cc/cc_common.bzl", "cc_common")
@@ -331,22 +331,6 @@ def _collect_transitive_dwo_artifacts(cc_compilation_outputs, cc_debug_context,
331331
transitive_dwo_files = cc_debug_context.files
332332
return depset(dwo_files, transitive = [transitive_dwo_files])
333333

334-
def _build_map_direct_dynamic_dep_to_transitive_dynamic_deps(ctx):
335-
all_dynamic_dep_linker_inputs = {}
336-
direct_dynamic_dep_to_transitive_dynamic_deps = {}
337-
for dep in ctx.attr.dynamic_deps:
338-
owner = dep[CcSharedLibraryInfo].linker_input.owner
339-
all_dynamic_dep_linker_inputs[owner] = dep[CcSharedLibraryInfo].linker_input
340-
transitive_dynamic_dep_labels = []
341-
for dynamic_dep in dep[CcSharedLibraryInfo].dynamic_deps.to_list():
342-
all_dynamic_dep_linker_inputs[dynamic_dep[1].owner] = dynamic_dep[1]
343-
transitive_dynamic_dep_labels.append(dynamic_dep[1].owner)
344-
transitive_dynamic_dep_labels_set = depset(transitive_dynamic_dep_labels, order = "topological")
345-
for export in dep[CcSharedLibraryInfo].exports:
346-
direct_dynamic_dep_to_transitive_dynamic_deps[export] = transitive_dynamic_dep_labels_set
347-
348-
return direct_dynamic_dep_to_transitive_dynamic_deps, all_dynamic_dep_linker_inputs
349-
350334
def _filter_libraries_that_are_linked_dynamically(ctx, feature_configuration, cc_linking_context):
351335
merged_cc_shared_library_infos = merge_cc_shared_library_infos(ctx)
352336
link_once_static_libs_map = build_link_once_static_libs_map(merged_cc_shared_library_infos)
@@ -365,19 +349,15 @@ def _filter_libraries_that_are_linked_dynamically(ctx, feature_configuration, cc
365349

366350
# Entries in unused_dynamic_linker_inputs will be marked None if they are
367351
# used
368-
(
369-
transitive_dynamic_dep_labels,
370-
unused_dynamic_linker_inputs,
371-
) = _build_map_direct_dynamic_dep_to_transitive_dynamic_deps(ctx)
372-
373352
(
374353
targets_to_be_linked_statically_map,
375354
targets_to_be_linked_dynamically_set,
376355
topologically_sorted_labels,
356+
unused_dynamic_linker_inputs,
377357
) = separate_static_and_dynamic_link_libraries(
358+
ctx,
378359
graph_structure_aspect_nodes,
379360
can_be_linked_dynamically,
380-
transitive_dynamic_dep_labels,
381361
)
382362

383363
topologically_sorted_labels = [ctx.label] + topologically_sorted_labels
@@ -411,19 +391,7 @@ def _filter_libraries_that_are_linked_dynamically(ctx, feature_configuration, cc
411391
# main binary, even indirect ones that are dependencies of direct
412392
# dynamic dependencies of this binary.
413393
link_indirect_deps = cc_common.is_enabled(feature_configuration = feature_configuration, feature_name = "targets_windows")
414-
direct_dynamic_dep_labels = {dep[CcSharedLibraryInfo].linker_input.owner: True for dep in ctx.attr.dynamic_deps}
415-
topologically_sorted_labels_set = {label: True for label in topologically_sorted_labels}
416-
for dynamic_linker_input_owner, unused_linker_input in unused_dynamic_linker_inputs.items():
417-
should_link_input = (unused_linker_input and
418-
(link_indirect_deps or dynamic_linker_input_owner in direct_dynamic_dep_labels))
419-
if should_link_input:
420-
_add_linker_input_to_dict(
421-
dynamic_linker_input_owner,
422-
unused_dynamic_linker_inputs[dynamic_linker_input_owner],
423-
)
424-
linker_inputs_count += 1
425-
if dynamic_linker_input_owner not in topologically_sorted_labels_set:
426-
topologically_sorted_labels.append(dynamic_linker_input_owner)
394+
linker_inputs_count += add_unused_dynamic_deps(ctx, unused_dynamic_linker_inputs, _add_linker_input_to_dict, topologically_sorted_labels, link_indirect_deps)
427395

428396
throw_linked_but_not_exported_errors(linked_statically_but_not_exported)
429397

‎src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,14 @@ def _sort_linker_inputs(topologically_sorted_labels, label_to_linker_inputs, lin
8383
# dynamically. The transitive_dynamic_dep_labels parameter is only needed for
8484
# binaries because they link all dynamic_deps (cc_binary|cc_test).
8585
def _separate_static_and_dynamic_link_libraries(
86+
ctx,
8687
direct_children,
87-
can_be_linked_dynamically,
88-
transitive_dynamic_dep_labels = {}):
88+
can_be_linked_dynamically):
89+
(
90+
transitive_dynamic_dep_labels,
91+
all_dynamic_dep_linker_inputs,
92+
) = _build_map_direct_dynamic_dep_to_transitive_dynamic_deps(ctx)
93+
8994
node = None
9095
all_children = reversed(direct_children)
9196
targets_to_be_linked_statically_map = {}
@@ -209,7 +214,7 @@ def _separate_static_and_dynamic_link_libraries(
209214
transitive.append(first_owner_to_depset[child.owners[0]])
210215
topologically_sorted_labels = depset(transitive = transitive, order = "topological").to_list()
211216

212-
return (targets_to_be_linked_statically_map, targets_to_be_linked_dynamically_set, topologically_sorted_labels)
217+
return (targets_to_be_linked_statically_map, targets_to_be_linked_dynamically_set, topologically_sorted_labels, all_dynamic_dep_linker_inputs)
213218

214219
def _create_linker_context(ctx, linker_inputs):
215220
return cc_common.create_linking_context(
@@ -389,11 +394,15 @@ def _filter_inputs(
389394

390395
# The targets_to_be_linked_statically_map points to whether the target to
391396
# be linked statically can be linked more than once.
397+
# Entries in unused_dynamic_linker_inputs will be marked None if they are
398+
# used
392399
(
393400
targets_to_be_linked_statically_map,
394401
targets_to_be_linked_dynamically_set,
395402
topologically_sorted_labels,
403+
unused_dynamic_linker_inputs,
396404
) = _separate_static_and_dynamic_link_libraries(
405+
ctx,
397406
graph_structure_aspect_nodes,
398407
can_be_linked_dynamically,
399408
)
@@ -437,6 +446,8 @@ def _filter_inputs(
437446
linker_inputs_seen[stringified_linker_input] = True
438447
owner = str(linker_input.owner)
439448
if owner in targets_to_be_linked_dynamically_set:
449+
unused_dynamic_linker_inputs[transitive_exports[owner].owner] = None
450+
440451
# Link the library in this iteration dynamically,
441452
# transitive_exports contains the artifacts produced by a
442453
# cc_shared_library
@@ -502,6 +513,8 @@ def _filter_inputs(
502513
message += dynamic_only_root + "\n"
503514
fail(message)
504515

516+
linker_inputs_count += _add_unused_dynamic_deps(ctx, unused_dynamic_linker_inputs, _add_linker_input_to_dict, topologically_sorted_labels, link_indirect_deps = False)
517+
505518
if ctx.attr.experimental_disable_topo_sort_do_not_use_remove_before_7_0:
506519
linker_inputs = experimental_remove_before_7_0_linker_inputs
507520
else:
@@ -567,6 +580,39 @@ def _get_deps(ctx):
567580

568581
return deps
569582

583+
def _build_map_direct_dynamic_dep_to_transitive_dynamic_deps(ctx):
584+
all_dynamic_dep_linker_inputs = {}
585+
direct_dynamic_dep_to_transitive_dynamic_deps = {}
586+
for dep in ctx.attr.dynamic_deps:
587+
owner = dep[CcSharedLibraryInfo].linker_input.owner
588+
all_dynamic_dep_linker_inputs[owner] = dep[CcSharedLibraryInfo].linker_input
589+
transitive_dynamic_dep_labels = []
590+
for dynamic_dep in dep[CcSharedLibraryInfo].dynamic_deps.to_list():
591+
all_dynamic_dep_linker_inputs[dynamic_dep[1].owner] = dynamic_dep[1]
592+
transitive_dynamic_dep_labels.append(dynamic_dep[1].owner)
593+
transitive_dynamic_dep_labels_set = depset(transitive_dynamic_dep_labels, order = "topological")
594+
for export in dep[CcSharedLibraryInfo].exports:
595+
direct_dynamic_dep_to_transitive_dynamic_deps[export] = transitive_dynamic_dep_labels_set
596+
597+
return direct_dynamic_dep_to_transitive_dynamic_deps, all_dynamic_dep_linker_inputs
598+
599+
def _add_unused_dynamic_deps(ctx, unused_dynamic_linker_inputs, add_linker_inputs_lambda, topologically_sorted_labels, link_indirect_deps):
600+
linker_inputs_count = 0
601+
direct_dynamic_dep_labels = {dep[CcSharedLibraryInfo].linker_input.owner: True for dep in ctx.attr.dynamic_deps}
602+
topologically_sorted_labels_set = {label: True for label in topologically_sorted_labels}
603+
for dynamic_linker_input_owner, unused_linker_input in unused_dynamic_linker_inputs.items():
604+
should_link_input = (unused_linker_input and
605+
(link_indirect_deps or dynamic_linker_input_owner in direct_dynamic_dep_labels))
606+
if should_link_input:
607+
add_linker_inputs_lambda(
608+
dynamic_linker_input_owner,
609+
unused_dynamic_linker_inputs[dynamic_linker_input_owner],
610+
)
611+
linker_inputs_count += 1
612+
if dynamic_linker_input_owner not in topologically_sorted_labels_set:
613+
topologically_sorted_labels.append(dynamic_linker_input_owner)
614+
return linker_inputs_count
615+
570616
def _cc_shared_library_impl(ctx):
571617
if not cc_common.check_experimental_cc_shared_library():
572618
if len(ctx.attr.static_deps):
@@ -806,3 +852,4 @@ build_exports_map_from_only_dynamic_deps = _build_exports_map_from_only_dynamic_
806852
throw_linked_but_not_exported_errors = _throw_linked_but_not_exported_errors
807853
separate_static_and_dynamic_link_libraries = _separate_static_and_dynamic_link_libraries
808854
sort_linker_inputs = _sort_linker_inputs
855+
add_unused_dynamic_deps = _add_unused_dynamic_deps

‎src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ cc_binary(
6161
"foo_so",
6262
"bar_so",
6363
],
64-
deps = ["foo", "bar"],
64+
deps = ["foo"],
6565
)
6666

6767
cc_shared_library(
@@ -77,32 +77,18 @@ cc_shared_library(
7777
deps = [":a_suffix"],
7878
)
7979

80-
cc_library(
81-
name = "diamond_lib1",
82-
deps = [
83-
":a_suffix",
84-
],
85-
)
86-
87-
cc_library(
88-
name = "diamond_lib2",
89-
deps = [
90-
":a_suffix",
91-
],
92-
)
93-
9480
cc_shared_library(
9581
name = "diamond_so",
9682
dynamic_deps = [":a_so"],
9783
features = ["windows_export_all_symbols"],
98-
deps = [":qux", "diamond_lib1"],
84+
deps = [":qux"],
9985
)
10086

10187
cc_shared_library(
10288
name = "diamond2_so",
10389
dynamic_deps = [":a_so"],
10490
features = ["windows_export_all_symbols"],
105-
deps = [":bar", "diamond_lib2"],
91+
deps = [":bar"],
10692
)
10793

10894
cc_binary(
@@ -129,7 +115,8 @@ cc_shared_library(
129115
}),
130116
dynamic_deps = [
131117
"bar_so",
132-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so"
118+
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so",
119+
"private_lib_so",
133120
],
134121
features = ["windows_export_all_symbols"],
135122
exports_filter = [
@@ -445,6 +432,24 @@ cc_library(
445432
],
446433
)
447434

435+
cc_shared_library(
436+
name = "private_lib_so",
437+
deps = [
438+
":private_lib",
439+
],
440+
)
441+
442+
genrule(
443+
name = "private_cc_lib_source",
444+
outs = ["private_cc_library.cc"],
445+
cmd = "touch $@",
446+
)
447+
448+
cc_library(
449+
name = "private_lib",
450+
srcs = [":private_cc_library.cc"]
451+
)
452+
448453
build_failure_test(
449454
name = "two_dynamic_deps_same_export_in_so_test",
450455
message = "Two shared libraries in dependencies export the same symbols",

‎src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ cc_binary(
1212
dynamic_deps = ["//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar_so"],
1313
tags = TAGS,
1414
deps = [
15-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar",
1615
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
1716
],
1817
)
@@ -29,7 +28,6 @@ cc_shared_library(
2928
cc_library(
3029
name = "intermediate",
3130
deps = [
32-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar",
3331
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
3432
],
3533
)

‎src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ def _linking_order_test_impl(env, target):
5858
matching.contains("foo.pic.o"),
5959
matching.contains("baz.pic.o"),
6060
]).in_order()
61+
62+
env.expect.that_collection(args).contains_at_least([
63+
"-lprivate_lib_so",
64+
])
65+
6166
env.expect.where(
6267
detail = "liba_suffix.pic.o should be the last user library linked",
6368
).that_str(user_libs[-1]).equals("a_suffix.pic.o")
@@ -181,6 +186,7 @@ def _runfiles_test_impl(env, target):
181186
"libfoo_so.so",
182187
"libbar_so.so",
183188
"libdiff_pkg_so.so",
189+
"libprivate_lib_so.so",
184190
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so",
185191
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so",
186192
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3_Slibdiff_Upkg_Uso.so",

0 commit comments

Comments
 (0)