Add C++ runtime libs to cgo action inputs (#4594)
With @llvm toolchain, libc++ is provided via cc_toolchain's
`static_runtime_library`/`dynamic_runtime_library`. These groups should
be added to link actions otherwise cgo code that attempts to link libc++
will not work properly. It seems there were previous attempts to drop
libc++ - I'm not sure the best way to harmonize these, some tweaking of
this PR may be necessary. @jayconrod @fmeum wdyt?
diff --git a/go/private/actions/archive.bzl b/go/private/actions/archive.bzl
index 47fed32..3a5a76b 100644
--- a/go/private/actions/archive.bzl
+++ b/go/private/actions/archive.bzl
@@ -140,7 +140,7 @@
cxxopts = cgo.cxxopts,
objcopts = cgo.objcopts,
objcxxopts = cgo.objcxxopts,
- clinkopts = cgo.clinkopts,
+ ldflags = cgo.ldflags,
testfilter = testfilter,
is_external_pkg = is_external_pkg,
)
diff --git a/go/private/actions/compilepkg.bzl b/go/private/actions/compilepkg.bzl
index b0769cc..3b8d775 100644
--- a/go/private/actions/compilepkg.bzl
+++ b/go/private/actions/compilepkg.bzl
@@ -66,7 +66,7 @@
cxxopts = [],
objcopts = [],
objcxxopts = [],
- clinkopts = [],
+ ldflags = None,
out_lib = None,
out_export = None,
out_facts = None,
@@ -191,19 +191,21 @@
compile_args.add("-objcflags", quote_opts(objcopts))
if objcxxopts:
compile_args.add("-objcxxflags", quote_opts(objcxxopts))
- if clinkopts:
- compile_args.add("-ldflags", quote_opts(clinkopts))
if go.mode.pgoprofile:
compile_args.add("-pgoprofile", go.mode.pgoprofile)
inputs_direct.append(go.mode.pgoprofile)
+ arguments = ["compilepkg", shared_args, compile_args]
+ if ldflags:
+ arguments.append(ldflags)
+
go.actions.run(
inputs = depset(inputs_direct, transitive = inputs_transitive),
outputs = outputs,
mnemonic = "GoCompilePkgExternal" if is_external_pkg else "GoCompilePkg",
executable = go.toolchain._builder,
- arguments = ["compilepkg", shared_args, compile_args],
+ arguments = arguments,
env = env,
toolchain = GO_TOOLCHAIN_LABEL,
execution_requirements = execution_requirements,
diff --git a/go/private/context.bzl b/go/private/context.bzl
index d669983..d23f0c6 100644
--- a/go/private/context.bzl
+++ b/go/private/context.bzl
@@ -57,8 +57,11 @@
)
load(
":mode.bzl",
+ "LINKMODE_C_SHARED",
"LINKMODE_NORMAL",
"LINKMODE_PIE",
+ "LINKMODE_PLUGIN",
+ "LINKMODE_SHARED",
"installsuffix",
"validate_mode",
)
@@ -521,6 +524,11 @@
export_stdlib = False,
)
+def _cc_runtime_libs_for_mode(mode, cgo_tools):
+ if mode.linkmode in (LINKMODE_SHARED, LINKMODE_PLUGIN, LINKMODE_C_SHARED):
+ return cgo_tools.cc_toolchain.dynamic_runtime_lib(feature_configuration = cgo_tools.feature_configuration)
+ return cgo_tools.cc_toolchain.static_runtime_lib(feature_configuration = cgo_tools.feature_configuration)
+
def _defaults_to_pie(goos, race):
# based on DefaultPIE in src/internal/platform/supported.go
if goos in ["android", "darwin", "ios"]:
@@ -650,8 +658,11 @@
if cgo_context_info:
env.update(cgo_context_info.env)
- cc_toolchain_files = cgo_context_info.cc_toolchain_files
cgo_tools = cgo_context_info.cgo_tools
+ cc_toolchain_files = depset(transitive = [
+ cgo_context_info.cc_toolchain_files,
+ _cc_runtime_libs_for_mode(mode, cgo_tools),
+ ])
else:
cc_toolchain_files = depset()
cgo_tools = None
diff --git a/go/private/mode.bzl b/go/private/mode.bzl
index dbe36ac..4a74dc3 100644
--- a/go/private/mode.bzl
+++ b/go/private/mode.bzl
@@ -168,6 +168,16 @@
# in each package. We use the executable options for this.
return go.cgo_tools.ld_executable_options
+def runtime_libs_from_cc_toolchain(go):
+ if not go.cgo_tools:
+ return depset()
+ elif go.mode.linkmode in (LINKMODE_SHARED, LINKMODE_PLUGIN, LINKMODE_C_SHARED):
+ return go.cgo_tools.cc_toolchain.dynamic_runtime_lib(feature_configuration = go.cgo_tools.feature_configuration)
+ else:
+ # Match extldflags_from_cc_toolchain: c-archive still uses executable
+ # link semantics for cgo's per-package link probe.
+ return go.cgo_tools.cc_toolchain.static_runtime_lib(feature_configuration = go.cgo_tools.feature_configuration)
+
def extld_from_cc_toolchain(go):
if not go.cgo_tools:
return []
diff --git a/go/private/rules/cgo.bzl b/go/private/rules/cgo.bzl
index 7ebb9fa..11affdb 100644
--- a/go/private/rules/cgo.bzl
+++ b/go/private/rules/cgo.bzl
@@ -13,6 +13,7 @@
# limitations under the License.
load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")
+load("//go/private/actions:utils.bzl", "quote_opts")
load(
"//go/private:common.bzl",
"get_versioned_shared_lib_extension",
@@ -23,8 +24,22 @@
"//go/private:mode.bzl",
"LINKMODE_NORMAL",
"extldflags_from_cc_toolchain",
+ "runtime_libs_from_cc_toolchain",
)
+_CXX_SOURCE_EXTENSIONS = {
+ "cc": None,
+ "cpp": None,
+ "cxx": None,
+ "mm": None,
+}
+
+def _has_cxx_sources(srcs):
+ for src in srcs:
+ if src.extension in _CXX_SOURCE_EXTENSIONS:
+ return True
+ return False
+
def cgo_configure(go, srcs, cdeps, cppopts, copts, cxxopts, clinkopts):
"""cgo_configure returns the inputs and compile / link options
that are required to build a cgo archive.
@@ -49,7 +64,7 @@
cxxopts: complete list of C++ compiler options.
objcopts: complete list of Objective-C compiler options.
objcxxopts: complete list of Objective-C++ compiler options.
- clinkopts: complete list of linker options.
+ ldflags: Args object containing complete C linker options.
"""
if not go.cgo_tools:
fail("Go toolchain does not support cgo")
@@ -59,15 +74,9 @@
cxxopts = go.cgo_tools.cxx_compile_options + cxxopts
objcopts = go.cgo_tools.objc_compile_options + copts
objcxxopts = go.cgo_tools.objcxx_compile_options + cxxopts
- clinkopts = extldflags_from_cc_toolchain(go) + clinkopts
-
- # NOTE(#2545): avoid unnecessary dynamic link
- if "-static-libstdc++" in clinkopts:
- clinkopts = [
- option
- for option in clinkopts
- if option not in ("-lstdc++", "-lc++")
- ]
+ toolchain_clinkopts = extldflags_from_cc_toolchain(go)
+ runtime_libs = runtime_libs_from_cc_toolchain(go)
+ needs_cxx_runtime = _has_cxx_sources(srcs)
if go.mode.linkmode != LINKMODE_NORMAL:
for opt_list in (copts, cxxopts, objcopts, objcxxopts):
@@ -149,6 +158,9 @@
# so it can be treated as a simple shared library too.
continue
lib_opts.append(lib.path)
+ if lib.basename.endswith(".a"):
+ # Match cgo2's heuristic: static cdeps may need the C++ runtime.
+ needs_cxx_runtime = True
clinkopts.extend(cc_link_flags)
elif hasattr(d, "objc"):
@@ -174,7 +186,26 @@
# specified with -l flags) unless they appear after .o or .a files with
# undefined symbols they provide. Put all the .a files from cdeps first,
# so that we actually link with -lstdc++ and others.
- clinkopts = lib_opts + clinkopts
+ pre_runtime_clinkopts = lib_opts + toolchain_clinkopts
+
+ # NOTE(#2545): avoid unnecessary dynamic link
+ if "-static-libstdc++" in pre_runtime_clinkopts + clinkopts:
+ pre_runtime_clinkopts = [
+ option
+ for option in pre_runtime_clinkopts
+ if option not in ("-lstdc++", "-lc++")
+ ]
+ clinkopts = [
+ option
+ for option in clinkopts
+ if option not in ("-lstdc++", "-lc++")
+ ]
+
+ ldflags = go.actions.args()
+ _add_ldflags(ldflags, pre_runtime_clinkopts)
+ if needs_cxx_runtime:
+ ldflags.add_all(runtime_libs, before_each = "-ldflags")
+ _add_ldflags(ldflags, clinkopts)
return struct(
inputs = inputs,
@@ -186,9 +217,13 @@
cxxopts = cxxopts,
objcopts = objcopts,
objcxxopts = objcxxopts,
- clinkopts = clinkopts,
+ ldflags = ldflags,
)
+def _add_ldflags(args, opts):
+ if opts:
+ args.add("-ldflags", quote_opts(opts))
+
def _cc_libs_and_flags(target):
# Copied from get_libs_for_static_executable in migration instructions
# from bazelbuild/bazel#7036.
diff --git a/tests/core/starlark/cgo/cgo_test.bzl b/tests/core/starlark/cgo/cgo_test.bzl
index b56accc..b5de750 100644
--- a/tests/core/starlark/cgo/cgo_test.bzl
+++ b/tests/core/starlark/cgo/cgo_test.bzl
@@ -1,6 +1,47 @@
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
+load("@rules_cc//cc:cc_toolchain_config_lib.bzl", "feature", "tool_path") # buildifier: disable=deprecated-function
+load("@rules_cc//cc:defs.bzl", "cc_toolchain")
+load("@rules_cc//cc/common:cc_common.bzl", "cc_common")
load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_cross_binary")
+def _test_cc_config_impl(ctx):
+ tool_paths = [
+ tool_path(name = name, path = "/bin/false")
+ for name in [
+ "ar",
+ "cpp",
+ "dwp",
+ "gcc",
+ "gcov",
+ "ld",
+ "nm",
+ "objcopy",
+ "objdump",
+ "strip",
+ ]
+ ]
+
+ return cc_common.create_cc_toolchain_config_info(
+ ctx = ctx,
+ toolchain_identifier = "runtime-libs-test-toolchain",
+ host_system_name = "local",
+ target_system_name = "local",
+ target_cpu = "local",
+ target_libc = "local",
+ compiler = "gcc",
+ abi_version = "local",
+ abi_libc_version = "local",
+ tool_paths = tool_paths,
+ features = [
+ feature(name = "static_link_cpp_runtimes", enabled = True),
+ ],
+ )
+
+test_cc_config = rule(
+ implementation = _test_cc_config_impl,
+ provides = [CcToolchainConfigInfo],
+)
+
def _missing_cc_toolchain_explicit_pure_off_test(ctx):
env = analysistest.begin(ctx)
@@ -16,7 +57,83 @@
},
)
+def _runtime_lib_inputs_test_impl(ctx):
+ env = analysistest.begin(ctx)
+ actions = analysistest.target_actions(env)
+ link_actions = [action for action in actions if action.mnemonic == "GoLink"]
+ asserts.equals(env, 1, len(link_actions), "expected exactly one GoLink action")
+
+ inputs = link_actions[0].inputs.to_list()
+ for expected in ctx.attr.expected_inputs:
+ asserts.true(
+ env,
+ any([input.path.endswith("/" + expected) for input in inputs]),
+ "expected '{}' to be in inputs: '{}'".format(expected, inputs),
+ )
+ for unexpected in ctx.attr.unexpected_inputs:
+ asserts.false(
+ env,
+ any([input.path.endswith("/" + unexpected) for input in inputs]),
+ "did not expect '{}' to be in inputs: '{}'".format(unexpected, inputs),
+ )
+
+ compile_actions = [action for action in actions if action.mnemonic == "GoCompilePkg"]
+ asserts.equals(env, 1, len(compile_actions), "expected exactly one GoCompilePkg action")
+ compile_argv = " ".join(compile_actions[0].argv)
+ for expected in ctx.attr.expected_linkopts:
+ asserts.true(
+ env,
+ expected in compile_argv,
+ "expected '{}' to be in compile argv: '{}'".format(expected, compile_argv),
+ )
+ for unexpected in ctx.attr.unexpected_linkopts:
+ asserts.false(
+ env,
+ unexpected in compile_argv,
+ "did not expect '{}' to be in compile argv: '{}'".format(unexpected, compile_argv),
+ )
+
+ return analysistest.end(env)
+
+runtime_lib_inputs_test = analysistest.make(
+ _runtime_lib_inputs_test_impl,
+ attrs = {
+ "expected_inputs": attr.string_list(),
+ "expected_linkopts": attr.string_list(),
+ "unexpected_inputs": attr.string_list(),
+ "unexpected_linkopts": attr.string_list(),
+ },
+ config_settings = {
+ "//command_line_option:extra_toolchains": str(Label("//tests/core/starlark/cgo:runtime_libs_test_cc_toolchain")),
+ },
+)
+
def cgo_test_suite():
+ test_cc_config(
+ name = "runtime_libs_test_cc_toolchain_config",
+ )
+
+ cc_toolchain(
+ name = "runtime_libs_test_cc_toolchain_impl",
+ all_files = ":empty",
+ compiler_files = ":empty",
+ dwp_files = ":empty",
+ dynamic_runtime_lib = ":dummy.so",
+ linker_files = ":empty",
+ objcopy_files = ":empty",
+ static_runtime_lib = ":dummy.a",
+ strip_files = ":empty",
+ supports_param_files = 0,
+ toolchain_config = ":runtime_libs_test_cc_toolchain_config",
+ toolchain_identifier = "runtime-libs-test-toolchain",
+ )
+
+ native.toolchain(
+ name = "runtime_libs_test_cc_toolchain",
+ toolchain = ":runtime_libs_test_cc_toolchain_impl",
+ toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
+ )
+
go_binary(
name = "cross_impure",
srcs = ["main.go"],
@@ -36,8 +153,53 @@
target_under_test = ":go_cross_impure_cgo",
)
+ go_binary(
+ name = "runtime_libs_static_binary",
+ srcs = [
+ "runtime_libs.cc",
+ "runtime_libs.go",
+ ],
+ cgo = True,
+ pure = "off",
+ tags = ["manual"],
+ )
+
+ runtime_lib_inputs_test(
+ name = "static_runtime_lib_inputs_test",
+ expected_inputs = ["dummy.a"],
+ expected_linkopts = ["dummy.a"],
+ target_under_test = ":runtime_libs_static_binary",
+ unexpected_inputs = ["dummy.so"],
+ unexpected_linkopts = ["dummy.so"],
+ )
+
+ go_binary(
+ name = "runtime_libs_dynamic_binary",
+ srcs = [
+ "runtime_libs.cc",
+ "runtime_libs.go",
+ ],
+ cgo = True,
+ linkmode = "c-shared",
+ pure = "off",
+ tags = ["manual"],
+ )
+
+ runtime_lib_inputs_test(
+ name = "dynamic_runtime_lib_inputs_test",
+ expected_inputs = ["dummy.so"],
+ expected_linkopts = ["dummy.so"],
+ target_under_test = ":runtime_libs_dynamic_binary",
+ unexpected_inputs = ["dummy.a"],
+ unexpected_linkopts = ["dummy.a"],
+ )
+
"""Creates the test targets and test suite for cgo.bzl tests."""
native.test_suite(
name = "cgo_tests",
- tests = [":missing_cc_toolchain_explicit_pure_off_test"],
+ tests = [
+ ":dynamic_runtime_lib_inputs_test",
+ ":missing_cc_toolchain_explicit_pure_off_test",
+ ":static_runtime_lib_inputs_test",
+ ],
)
diff --git a/tests/core/starlark/cgo/dummy.a b/tests/core/starlark/cgo/dummy.a
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/tests/core/starlark/cgo/dummy.a
@@ -0,0 +1 @@
+
diff --git a/tests/core/starlark/cgo/dummy.so b/tests/core/starlark/cgo/dummy.so
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/tests/core/starlark/cgo/dummy.so
@@ -0,0 +1 @@
+
diff --git a/tests/core/starlark/cgo/empty b/tests/core/starlark/cgo/empty
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/tests/core/starlark/cgo/empty
@@ -0,0 +1 @@
+
diff --git a/tests/core/starlark/cgo/main.go b/tests/core/starlark/cgo/main.go
new file mode 100644
index 0000000..38dd16d
--- /dev/null
+++ b/tests/core/starlark/cgo/main.go
@@ -0,0 +1,3 @@
+package main
+
+func main() {}
diff --git a/tests/core/starlark/cgo/runtime_libs.cc b/tests/core/starlark/cgo/runtime_libs.cc
new file mode 100644
index 0000000..b2791d0
--- /dev/null
+++ b/tests/core/starlark/cgo/runtime_libs.cc
@@ -0,0 +1 @@
+extern "C" void runtime_libs_test(void) {}
diff --git a/tests/core/starlark/cgo/runtime_libs.go b/tests/core/starlark/cgo/runtime_libs.go
new file mode 100644
index 0000000..5aa8e1f
--- /dev/null
+++ b/tests/core/starlark/cgo/runtime_libs.go
@@ -0,0 +1,8 @@
+package main
+
+// void runtime_libs_test(void);
+import "C"
+
+func main() {
+ C.runtime_libs_test()
+}
