Skip to content

Commit

Permalink
Introduce flag --@bazel_tools//tools/cpp:link_extra_libs
Browse files Browse the repository at this point in the history
Numerous tools override --custom_malloc to add debugging or monitoring runtimes
(see e.g. sanitizers). While this is fine for cases where the tool must also
override malloc to function, in other cases it's simply misuse of
--custom_malloc where no other mechanism exists to link an extra library.

This becomes especially problematic where a runtime library is supposed to be
added in certain configurations that should run in production or other
performance sensitive builds. In these cases, we should _not_ override malloc,
which may also be specified by a cc_binary target. Doing so would introduce
unwanted changes, potentially affecting performance negatively.

To fix abuse of --custom_malloc, add --@bazel_tools//tools/cpp:link_extra_libs,
which is similar, but otherwise does not override other runtime libraries.

Finally, extra libraries are collected in //tools/cpp:link_extra_lib, which can
be used to also introduce libraries based on global configuration rather than
via the command line option.

PiperOrigin-RevId: 509191255
Change-Id: I932afafed574b0cf5f37a8358f6e12c43f59ba1c
  • Loading branch information
Googler authored and copybara-github committed Feb 13, 2023
1 parent f2ae8f8 commit 9f2c62a
Show file tree
Hide file tree
Showing 14 changed files with 143 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ filegroup(
srcs = ["ndk"],
)

cc_library(
name = "link_extra_lib",
srcs = [],
)

cc_library(
name = "malloc",
srcs = [],
Expand Down
8 changes: 8 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary_attrs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ cc_binary_attrs_with_aspects = {
default = configuration_field(fragment = "cpp", name = "custom_malloc"),
aspects = [graph_structure_aspect],
),
"_link_extra_lib": attr.label(
default = Label("@" + semantics.get_repo() + "//tools/cpp:link_extra_lib"),
providers = [CcInfo],
aspects = [graph_structure_aspect],
),
"stamp": attr.int(
values = [-1, 0, 1],
default = -1,
Expand Down Expand Up @@ -112,3 +117,6 @@ cc_binary_attrs_without_aspects["malloc"] = attr.label(
cc_binary_attrs_without_aspects["_default_malloc"] = attr.label(
default = configuration_field(fragment = "cpp", name = "custom_malloc"),
)
cc_binary_attrs_without_aspects["_link_extra_lib"] = attr.label(
default = Label("@" + semantics.get_repo() + "//tools/cpp:link_extra_lib"),
)
10 changes: 8 additions & 2 deletions src/main/starlark/builtins_bzl/common/cc/semantics.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,15 @@ def _get_coverage_env(ctx):
def _get_cc_runtimes(ctx, is_library):
if is_library:
return []

runtimes = [ctx.attr._link_extra_lib]

if ctx.fragments.cpp.custom_malloc != None:
return [ctx.attr._default_malloc]
return [ctx.attr.malloc]
runtimes.append(ctx.attr._default_malloc)
else:
runtimes.append(ctx.attr.malloc)

return runtimes

def _should_use_legacy_cc_test(_):
return True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,8 @@ public void write() throws IOException {
" name = 'interface_library_builder',",
" srcs = ['build_interface_so'],",
")",
// We add an empty :link_extra_lib target in case we need it.
"cc_library(name = 'link_extra_lib')",
// We add an empty :malloc target in case we need it.
"cc_library(name = 'malloc')",
// Fake targets to get us through loading/analysis.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,8 @@ public void testDeps() throws Exception {
}
String expectedDependencies =
helper.getToolsRepository()
+ "//tools/cpp:link_extra_lib + "
+ helper.getToolsRepository()
+ "//tools/cpp:malloc + //configurable:main + "
+ "//configurable:main.cc + //configurable:adep + //configurable:bdep + "
+ "//configurable:defaultdep + //conditions:a + //conditions:b "
Expand Down Expand Up @@ -1016,6 +1018,7 @@ public void testNoImplicitDeps() throws Exception {

// Implicit dependencies:
String hostDepsExpr = helper.getToolsRepository() + "//tools/cpp:malloc";
hostDepsExpr += " + " + helper.getToolsRepository() + "//tools/cpp:link_extra_lib";
if (!analysisMock.isThisBazel()) {
hostDepsExpr += " + //tools/cpp:malloc.cc";
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/shell/bazel/cc_flags_supplier_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ function write_crosstool() {
package(default_visibility = ["//visibility:public"])
load(":cc_toolchain_config.bzl", "cc_toolchain_config")
cc_library(
name = "link_extra_lib",
)
cc_library(
name = "malloc",
)
Expand Down
4 changes: 4 additions & 0 deletions src/test/shell/integration/cpp_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ package(default_visibility = ["//visibility:public"])
load(":toolchain.bzl", "toolchains")
cc_library(
name = "link_extra_lib",
)
cc_library(
name = "malloc",
)
Expand Down
4 changes: 4 additions & 0 deletions tools/cpp/BUILD.empty.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ load(":cc_toolchain_config.bzl", "cc_toolchain_config")

package(default_visibility = ["//visibility:public"])

cc_library(
name = "link_extra_lib",
)

cc_library(
name = "malloc",
)
Expand Down
18 changes: 18 additions & 0 deletions tools/cpp/BUILD.static.bsd
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@ load("@rules_cc//cc:defs.bzl", "cc_library", "cc_toolchain", "cc_toolchain_suite

package(default_visibility = ["//visibility:public"])

cc_library(name = "empty_lib")

# Label flag for extra libraries to be linked into every binary.
# TODO(bazel-team): Support passing flag multiple times to build a list.
label_flag(
name = "link_extra_libs",
build_setting_default = ":empty_lib",
)

# The final extra library to be linked into every binary target. This collects
# the above flag, but may also include more libraries depending on config.
cc_library(
name = "link_extra_lib",
deps = [
":link_extra_libs",
],
)

cc_library(
name = "malloc",
)
Expand Down
22 changes: 22 additions & 0 deletions tools/cpp/BUILD.tools
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,28 @@ cc_host_toolchain_alias(name = "current_cc_host_toolchain")

cc_libc_top_alias(name = "current_libc_top")

cc_library(
name = "empty_lib",
tags = ["__DONT_DEPEND_ON_DEF_PARSER__"],
)

# Label flag for extra libraries to be linked into every binary.
# TODO(bazel-team): Support passing flag multiple times to build a list.
label_flag(
name = "link_extra_libs",
build_setting_default = ":empty_lib",
)

# The final extra library to be linked into every binary target. This collects
# the above flag, but may also include more libraries depending on config.
cc_library(
name = "link_extra_lib",
tags = ["__DONT_DEPEND_ON_DEF_PARSER__"],
deps = [
":link_extra_libs",
],
)

cc_library(
name = "malloc",
tags = ["__DONT_DEPEND_ON_DEF_PARSER__"],
Expand Down
18 changes: 18 additions & 0 deletions tools/cpp/BUILD.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,24 @@ package(default_visibility = ["//visibility:public"])

licenses(["notice"]) # Apache 2.0

cc_library(name = "empty_lib")

# Label flag for extra libraries to be linked into every binary.
# TODO(bazel-team): Support passing flag multiple times to build a list.
label_flag(
name = "link_extra_libs",
build_setting_default = ":empty_lib",
)

# The final extra library to be linked into every binary target. This collects
# the above flag, but may also include more libraries depending on config.
cc_library(
name = "link_extra_lib",
deps = [
":link_extra_libs",
],
)

cc_library(
name = "malloc",
)
Expand Down
18 changes: 18 additions & 0 deletions tools/cpp/BUILD.windows.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,24 @@ load(":armeabi_cc_toolchain_config.bzl", "armeabi_cc_toolchain_config")

package(default_visibility = ["//visibility:public"])

cc_library(name = "empty_lib")

# Label flag for extra libraries to be linked into every binary.
# TODO(bazel-team): Support passing flag multiple times to build a list.
label_flag(
name = "link_extra_libs",
build_setting_default = ":empty_lib",
)

# The final extra library to be linked into every binary target. This collects
# the above flag, but may also include more libraries depending on config.
cc_library(
name = "link_extra_lib",
deps = [
":link_extra_libs",
],
)

cc_library(
name = "malloc",
)
Expand Down
39 changes: 24 additions & 15 deletions tools/jdk/BUILD.java_tools
Original file line number Diff line number Diff line change
Expand Up @@ -135,21 +135,25 @@ config_setting(
visibility = ["//visibility:public"],
)

# Create intermediate cc_library, which does not implicitly depend on "malloc"
# and "link_extra_lib" in @bazel_tools//tools/cpp, and thereby avoids include
# path /Iexternal/tools being used in compiling actions which would result in
# the wrong headers being picked up.
cc_library(
name = "malloc",
)

cc_binary(
name = "ijar_cc_binary",
name = "ijar_cc_binary_main",
srcs = [
"java_tools/ijar/classfile.cc",
"java_tools/ijar/ijar.cc",
],
copts = SUPRESSED_WARNINGS,
# Remove dependency on @bazel_tools//tools/cpp:malloc, which avoid /Iexternal/tools being used
# in compiling actions.
malloc = ":malloc",
linkstatic = 1, # provides main()
deps = [":zip"],
alwayslink = 1,
)

cc_binary(
name = "ijar_cc_binary",
deps = [":ijar_cc_binary_main"],
)

cc_library(
Expand Down Expand Up @@ -364,8 +368,9 @@ cc_library(

##################### singlejar

cc_binary(
name = "singlejar_cc_bin",
# See comment for ":ijar_cc_binary_main".
cc_library(
name = "singlejar_cc_bin_main",
srcs = [
"java_tools/src/tools/singlejar/singlejar_main.cc",
],
Expand All @@ -375,18 +380,22 @@ cc_binary(
":openbsd": ["-lm"],
"//conditions:default": [],
}),
linkstatic = 1,
# Remove dependency on @bazel_tools//tools/cpp:malloc, which avoid /Iexternal/tools being used
# in compiling actions.
malloc = ":malloc",
visibility = ["//visibility:public"],
linkstatic = 1, # provides main()
deps = [
":combiners",
":diag",
":options",
":output_jar",
"//java_tools/zlib",
],
alwayslink = 1,
)

cc_binary(
name = "singlejar_cc_bin",
visibility = ["//visibility:public"],
linkstatic = 1,
deps = [":singlejar_cc_bin_main"],
)

cc_binary(
Expand Down
4 changes: 4 additions & 0 deletions tools/osx/crosstool/BUILD.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ CC_TOOLCHAINS = [(
("armeabi-v7a", ":cc-compiler-armeabi-v7a"),
]

cc_library(
name = "link_extra_lib",
)

cc_library(
name = "malloc",
)
Expand Down

0 comments on commit 9f2c62a

Please sign in to comment.