Issue 1486: cgo: fix C++ dynamic initialization of static variables when using alwayslink = True (#4438)
<!-- Thanks for sending a PR! Before submitting:
1. If this is your first PR, please read CONTRIBUTING.md and sign the
CLA
first. We cannot review code without a signed CLA.
2. Please file an issue *first*. All features and most bug fixes should
have
an associated issue with a design discussed and decided upon. Small bug
fixes and documentation improvements don't need issues.
3. New features and bug fixes must have tests. Documentation may need to
be updated. If you're unsure what to update, send the PR, and we'll
discuss
in review.
4. Note that PRs updating dependencies and new Go versions are not
accepted.
Please file an issue instead.
-->
**What type of PR is this?**
Bug fix
**What does this PR do? Why is it needed?**
Fixes static linking against libraries that have `alwayslink = True`
specified. Fixes C++'s dynamic initialization of static variables when
using cgo
**Which issues(s) does this PR fix?**
Fixes #1486
---
Related to https://github.com/bazel-contrib/rules_go/pull/2584 and
https://github.com/bazel-contrib/rules_go/pull/2294
diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 1e2b4ee..7b7a9d2 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -9,6 +9,7 @@
# Names should be added to this file as:
# Name <email address>
+Andrei Reshetkov <enscogitans.v@gmail.com>
Benjamin Staffin <benley@gmail.com>
Brian Silverman <bsilver16384@gmail.com>
Damien Martin-Guillerez <dmarting@google.com>
diff --git a/go/private/rules/cgo.bzl b/go/private/rules/cgo.bzl
index b8fc93a..f64f313 100644
--- a/go/private/rules/cgo.bzl
+++ b/go/private/rules/cgo.bzl
@@ -92,6 +92,7 @@
deps_direct = []
lib_opts = []
runfiles = go._ctx.runfiles(collect_data = True)
+ seen_alwayslink_libs = {}
# Always include the sandbox as part of the build. Bazel does this, but it
# doesn't appear in the CompilationContext.
@@ -144,7 +145,13 @@
# libclntsh.dylib.12.1, users have to create a unversioned symbolic link,
# so it can be treated as a simple shared library too.
continue
- lib_opts.append(lib.path)
+
+ if lib.basename.endswith(".lo") and lib.path not in seen_alwayslink_libs:
+ seen_alwayslink_libs[lib.path] = True
+ lib_opts.extend(_alwayslink_lib_opts(go, lib.path))
+ else:
+ lib_opts.append(lib.path)
+
clinkopts.extend(cc_link_flags)
elif hasattr(d, "objc"):
@@ -201,6 +208,11 @@
libs.append(library_to_link.dynamic_library)
return libs, flags
+def _alwayslink_lib_opts(go, lib_path):
+ if go.mode.goos == "darwin":
+ return ["-Wl,-force_load,{}".format(lib_path)]
+ return ["-Wl,-whole-archive", lib_path, "-Wl,-no-whole-archive"]
+
def _include_unique(opts, flag, include, seen):
if include in seen:
return
diff --git a/tests/core/cgo/cgo_alwayslink_init/BUILD.bazel b/tests/core/cgo/cgo_alwayslink_init/BUILD.bazel
new file mode 100644
index 0000000..fab4926
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/BUILD.bazel
@@ -0,0 +1,39 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_test")
+
+cc_library(
+ name = "lib",
+ srcs = [
+ "lib.cpp",
+ "side_effect.cpp",
+ ],
+ hdrs = [
+ "lib.h",
+ ],
+ alwayslink = True,
+)
+
+# used to check that we don't get a "multiple definition" linker errors, and that the side effect
+# is performed exactly once, even if imported several times through different targets
+cc_library(
+ name = "lib_wrapper",
+ deps = [":lib"],
+)
+
+cc_test(
+ name = "test_side_effect_cc",
+ srcs = ["test_side_effect.cpp"],
+ deps = [
+ ":lib",
+ ":lib_wrapper",
+ ],
+)
+
+go_test(
+ name = "test_side_effect_go",
+ srcs = ["test_side_effect.go"],
+ cdeps = [
+ ":lib",
+ ":lib_wrapper",
+ ],
+ cgo = True,
+)
diff --git a/tests/core/cgo/cgo_alwayslink_init/README.rst b/tests/core/cgo/cgo_alwayslink_init/README.rst
new file mode 100644
index 0000000..ad08898
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/README.rst
@@ -0,0 +1,18 @@
+.. _#1486 : https://github.com/bazel-contrib/rules_go/issues/1486
+
+Cgo and dynamic initialization with `alwayslink = True`
+========================================================
+
+test_side_effect_go
+-------------------
+This test verifies that the dynamic initialization of C++ variables with static storage duration in
+`cc_library` targets with `alwayslink = True` is performed when linked into a `go_test` or
+`go_binary` with `cgo = True`. This is a regression test for issue `#1486`_. The test also
+includes a `lib_wrapper` library to ensure that there are no linker errors and that the side effect
+is performed exactly once, even if it is imported multiple times through different targets.
+
+test_side_effect_cc
+-------------------
+This is a C++ test included as a reference. It has the same dependencies as `test_side_effect_go`
+and confirms the expected behavior in a pure C++ environment, where the dynamic initialization of
+variables with static storage duration is always performed.
diff --git a/tests/core/cgo/cgo_alwayslink_init/lib.cpp b/tests/core/cgo/cgo_alwayslink_init/lib.cpp
new file mode 100644
index 0000000..7b0c66f
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/lib.cpp
@@ -0,0 +1,3 @@
+#include "tests/core/cgo/cgo_alwayslink_init/lib.h"
+
+int value = 0;
diff --git a/tests/core/cgo/cgo_alwayslink_init/lib.h b/tests/core/cgo/cgo_alwayslink_init/lib.h
new file mode 100644
index 0000000..e9a9719
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/lib.h
@@ -0,0 +1 @@
+extern int value;
diff --git a/tests/core/cgo/cgo_alwayslink_init/side_effect.cpp b/tests/core/cgo/cgo_alwayslink_init/side_effect.cpp
new file mode 100644
index 0000000..fa70126
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/side_effect.cpp
@@ -0,0 +1,13 @@
+#include "tests/core/cgo/cgo_alwayslink_init/lib.h"
+
+namespace {
+
+struct SideEffect {
+ SideEffect() {
+ value += 42;
+ }
+};
+
+SideEffect effect;
+
+} // namespace
diff --git a/tests/core/cgo/cgo_alwayslink_init/test_side_effect.cpp b/tests/core/cgo/cgo_alwayslink_init/test_side_effect.cpp
new file mode 100644
index 0000000..2413943
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/test_side_effect.cpp
@@ -0,0 +1,13 @@
+#include <iostream>
+
+#include "tests/core/cgo/cgo_alwayslink_init/lib.h"
+
+int main() {
+ const int expected = 42;
+ const int actual = value;
+ if (expected == actual) {
+ return 0;
+ }
+ std::cout << "Expected " << expected << ", got " << actual << '\n';
+ return 1;
+}
diff --git a/tests/core/cgo/cgo_alwayslink_init/test_side_effect.go b/tests/core/cgo/cgo_alwayslink_init/test_side_effect.go
new file mode 100644
index 0000000..541207f
--- /dev/null
+++ b/tests/core/cgo/cgo_alwayslink_init/test_side_effect.go
@@ -0,0 +1,13 @@
+package ccstaticinit
+
+// #include "tests/core/cgo/cgo_alwayslink_init/lib.h"
+import "C"
+import "testing"
+
+func TestValue(t *testing.T) {
+ const expected = 42
+ actual := int(C.value)
+ if expected != actual {
+ t.Errorf("Expected %v, got %v", expected, actual)
+ }
+}
