Skip to content

Commit

Permalink
make pic_behavior a required arg for get_actual_link_style
Browse files Browse the repository at this point in the history
Summary:
Now that've verified the logic for `get_actual_link_style` is sound, lets start updating the callsites based on their nearest toolchain.

For `prebuild_*_library` there isn't much we can do (I think?) so I passed `PicBehavior("supported")`.

Reviewed By: milend

Differential Revision: D47197095

fbshipit-source-id: 75646c8d4242263b3b8d4af116a68968b7f9db42
  • Loading branch information
Dustin Shahidehpour authored and facebook-github-bot committed Jul 12, 2023
1 parent af043fa commit d634a94
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 31 deletions.
2 changes: 2 additions & 0 deletions prelude/apple/prebuilt_apple_framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# License, Version 2.0 found in the LICENSE-APACHE file in the root directory
# of this source tree.

load("@prelude//cxx:cxx_context.bzl", "get_cxx_toolchain_info")
load(
"@prelude//cxx:cxx_library_utility.bzl",
"cxx_attr_exported_linker_flags",
Expand Down Expand Up @@ -79,6 +80,7 @@ def prebuilt_apple_framework_impl(ctx: "context") -> ["provider"]:
)
providers.append(create_merged_link_info(
ctx,
get_cxx_toolchain_info(ctx).pic_behavior,
{link_style: LinkInfos(default = link) for link_style in LinkStyle},
))

Expand Down
7 changes: 5 additions & 2 deletions prelude/cxx/cxx.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,10 @@ def prebuilt_cxx_library_impl(ctx: "context") -> ["provider"]:
)]

# Create the default output for the library rule given it's link style and preferred linkage
link_style = get_cxx_toolchain_info(ctx).linker_info.link_style
actual_link_style = get_actual_link_style(link_style, preferred_linkage)
cxx_toolchain = get_cxx_toolchain_info(ctx)
pic_behavior = cxx_toolchain.pic_behavior
link_style = cxx_toolchain.linker_info.link_style
actual_link_style = get_actual_link_style(link_style, preferred_linkage, pic_behavior)
output = outputs[actual_link_style]
providers.append(DefaultInfo(
default_output = output,
Expand All @@ -462,6 +464,7 @@ def prebuilt_cxx_library_impl(ctx: "context") -> ["provider"]:
# Propagate link info provider.
providers.append(create_merged_link_info(
ctx,
pic_behavior,
# Add link info for each link style,
libraries,
preferred_linkage = preferred_linkage,
Expand Down
4 changes: 4 additions & 0 deletions prelude/cxx/cxx_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ def cxx_executable(ctx: "context", impl_params: CxxRuleConstructorParams.type, i
children = [d.link_group_lib_info for d in link_deps],
)

pic_behavior = get_cxx_toolchain_info(ctx).pic_behavior

# TODO(T110378098): Similar to shared libraries, we need to identify all the possible
# scenarios for which we need to propagate up link info and simplify this logic. For now
# base which links to use based on whether link groups are defined.
Expand All @@ -310,6 +312,7 @@ def cxx_executable(ctx: "context", impl_params: CxxRuleConstructorParams.type, i
link_group,
link_group_mappings,
link_group_preferred_linkage,
pic_behavior = pic_behavior,
link_group_libs = {
name: (lib.label, lib.shared_link_infos)
for name, lib in link_group_libs.items()
Expand Down Expand Up @@ -338,6 +341,7 @@ def cxx_executable(ctx: "context", impl_params: CxxRuleConstructorParams.type, i
link_group_mappings,
link_group_preferred_linkage,
link_style,
pic_behavior = pic_behavior,
roots = [d.linkable_graph.nodes.value.label for d in link_deps],
is_executable_link = True,
prefer_stripped = impl_params.prefer_stripped_objects,
Expand Down
22 changes: 18 additions & 4 deletions prelude/cxx/cxx_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,15 @@ load(
"Linkage",
"LinkedObject", # @unused Used as a type
"ObjectsLinkable",
"STATIC_LINK_STYLES",
"SharedLibLinkable",
"SwiftRuntimeLinkable", # @unused Used as a type
"SwiftmoduleLinkable", # @unused Used as a type
"create_merged_link_info",
"get_actual_link_style",
"get_link_args",
"get_link_styles_for_linkage",
"process_link_style_for_pic_behavior",
"unpack_link_args",
"wrap_link_info",
)
Expand Down Expand Up @@ -439,7 +441,8 @@ def cxx_library_parameterized(ctx: "context", impl_params: "CxxRuleConstructorPa

providers.extend(library_outputs.providers)

actual_link_style = get_actual_link_style(cxx_attr_link_style(ctx), preferred_linkage)
pic_behavior = get_cxx_toolchain_info(ctx).pic_behavior
actual_link_style = get_actual_link_style(cxx_attr_link_style(ctx), preferred_linkage, pic_behavior)

# Output sub-targets for all link-styles.
if impl_params.generate_sub_targets.link_style_outputs or impl_params.generate_providers.link_style_outputs:
Expand Down Expand Up @@ -510,6 +513,7 @@ def cxx_library_parameterized(ctx: "context", impl_params: "CxxRuleConstructorPa

merged_native_link_info = create_merged_link_info(
ctx,
pic_behavior,
# Add link info for each link style,
library_outputs.libraries,
preferred_linkage = preferred_linkage,
Expand Down Expand Up @@ -551,6 +555,7 @@ def cxx_library_parameterized(ctx: "context", impl_params: "CxxRuleConstructorPa
if impl_params.generate_sub_targets.headers:
sub_targets["headers"] = [propagated_preprocessor, create_merged_link_info(
ctx,
pic_behavior = pic_behavior,
preferred_linkage = Linkage("static"),
frameworks_linkable = frameworks_linkable,
), LinkGroupLibInfo(libs = {}), SharedLibraryInfo(set = None)] + additional_providers
Expand Down Expand Up @@ -755,8 +760,8 @@ def cxx_library_parameterized(ctx: "context", impl_params: "CxxRuleConstructorPa

def get_default_cxx_library_product_name(ctx, impl_params) -> str:
preferred_linkage = cxx_attr_preferred_linkage(ctx)
link_style = get_actual_link_style(cxx_attr_link_style(ctx), preferred_linkage)
if link_style in (LinkStyle("static"), LinkStyle("static_pic")):
link_style = get_actual_link_style(cxx_attr_link_style(ctx), preferred_linkage, get_cxx_toolchain_info(ctx).pic_behavior)
if link_style in STATIC_LINK_STYLES:
return _base_static_library_name(ctx, False)
else:
return _soname(ctx, impl_params)
Expand Down Expand Up @@ -1008,6 +1013,8 @@ def _get_shared_library_links(
logic here, simply diverge behavior on whether link groups are defined.
"""

pic_behavior = get_cxx_toolchain_info(ctx).pic_behavior

# If we're not filtering for link groups, link against the shared dependencies
if not link_group_mappings and not force_link_group_linking:
link = cxx_inherited_link_info(ctx, dedupe(flatten([non_exported_deps, exported_deps])))
Expand Down Expand Up @@ -1036,14 +1043,20 @@ def _get_shared_library_links(
ctx,
link,
frameworks_linkable,
LinkStyle(link_style_value),
# fPIC behaves differently on various combinations of toolchains + platforms.
# To get the link_style, we have to check the link_style against the toolchain's pic_behavior.
#
# For more info, check the PicBehavior.type docs.
process_link_style_for_pic_behavior(LinkStyle(link_style_value), pic_behavior),
swiftmodule_linkable = swiftmodule_linkable,
swift_runtime_linkable = swift_runtime_linkable,
), None, link_execution_preference

# Else get filtered link group links
prefer_stripped = cxx_is_gnu(ctx) and ctx.attrs.prefer_stripped_objects

link_style = cxx_attr_link_style(ctx) if cxx_attr_link_style(ctx) != LinkStyle("static") else LinkStyle("static_pic")
link_style = process_link_style_for_pic_behavior(link_style, pic_behavior)
filtered_labels_to_links_map = get_filtered_labels_to_links_map(
linkable_graph_node_map_func(),
link_group,
Expand All @@ -1055,6 +1068,7 @@ def _get_shared_library_links(
},
link_style = link_style,
roots = linkable_deps(non_exported_deps + exported_deps),
pic_behavior = pic_behavior,
prefer_stripped = prefer_stripped,
force_static_follows_dependents = force_static_follows_dependents,
)
Expand Down
17 changes: 11 additions & 6 deletions prelude/cxx/link_groups.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ load(
"cxx_is_gnu",
"cxx_mk_shlib_intf",
)
load(":cxx_toolchain_types.bzl", "PicBehavior")
load(
":groups.bzl",
"Group", # @unused Used as a type
Expand Down Expand Up @@ -204,12 +205,13 @@ def get_link_group_preferred_linkage(link_groups: [Group.type]) -> {"label": Lin
if mapping.root != None and mapping.preferred_linkage != None
}

def transitively_update_shared_linkage(
def _transitively_update_shared_linkage(
linkable_graph_node_map: {"label": LinkableNode.type},
link_group: [str, None],
link_style: LinkStyle.type,
link_group_preferred_linkage: {"label": Linkage.type},
link_group_roots: {"label": str}):
link_group_roots: {"label": str},
pic_behavior: PicBehavior.type):
# Identify targets whose shared linkage style may be propagated to
# dependencies. Implicitly created root libraries are skipped.
shared_lib_roots = []
Expand All @@ -218,7 +220,7 @@ def transitively_update_shared_linkage(
node = linkable_graph_node_map.get(target)
if node == None:
continue
actual_link_style = get_actual_link_style(link_style, link_group_preferred_linkage.get(target, node.preferred_linkage))
actual_link_style = get_actual_link_style(link_style, link_group_preferred_linkage.get(target, node.preferred_linkage), pic_behavior)
if actual_link_style == LinkStyle("shared"):
target_link_group = link_group_roots.get(target)
if target_link_group == None or target_link_group == link_group:
Expand All @@ -229,7 +231,7 @@ def transitively_update_shared_linkage(
linkable_node = linkable_graph_node_map[node]
if linkable_node.preferred_linkage == Linkage("any"):
link_group_preferred_linkage[node] = Linkage("shared")
return get_deps_for_link(linkable_node, link_style)
return get_deps_for_link(linkable_node, link_style, pic_behavior)

breadth_first_traversal_by(
linkable_graph_node_map,
Expand All @@ -244,6 +246,7 @@ def get_filtered_labels_to_links_map(
link_group_preferred_linkage: {"label": Linkage.type},
link_style: LinkStyle.type,
roots: ["label"],
pic_behavior: PicBehavior.type,
link_group_libs: {str: (["label", None], LinkInfos.type)} = {},
prefer_stripped: bool = False,
is_executable_link: bool = False,
Expand Down Expand Up @@ -291,12 +294,13 @@ def get_filtered_labels_to_links_map(

# Transitively update preferred linkage to avoid runtime issues from
# missing dependencies (e.g. for prebuilt shared libs).
transitively_update_shared_linkage(
_transitively_update_shared_linkage(
linkable_graph_node_map,
link_group,
link_style,
link_group_preferred_linkage,
link_group_roots,
pic_behavior,
)

linkable_map = {}
Expand Down Expand Up @@ -338,7 +342,7 @@ def get_filtered_labels_to_links_map(

for target in linkables:
node = linkable_graph_node_map[target]
actual_link_style = get_actual_link_style(link_style, link_group_preferred_linkage.get(target, node.preferred_linkage))
actual_link_style = get_actual_link_style(link_style, link_group_preferred_linkage.get(target, node.preferred_linkage), pic_behavior)

# Always link any shared dependencies
if actual_link_style == LinkStyle("shared"):
Expand Down Expand Up @@ -573,6 +577,7 @@ def _create_link_group(
spec.group.name,
link_group_mappings,
link_group_preferred_linkage,
pic_behavior = get_cxx_toolchain_info(ctx).pic_behavior,
link_group_libs = link_group_libs,
link_style = link_style,
roots = roots,
Expand Down
Loading

0 comments on commit d634a94

Please sign in to comment.