Issue 1486: cgo: fix C++ dynamic initialization of static variables when using alwayslink = True by enscogitans · Pull Request #4438 · bazel-contrib/rules_go

enscogitans · 2025-09-02T17:48:07Z

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 #2584 and #2294

fmeum

Thanks for working on this, let me know if you need help with the tests.

go/private/rules/cgo.bzl

enscogitans · 2025-09-03T21:35:08Z

Hi, @fmeum!

Yes, I would need help with tests. I tried to fix them, but didn't succeed. Plus, I don't have debian and windows machines to debug tests locally

I've got 2 tests failing:

Windows

c:/tools/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/15.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: cannot find -llib_285ff768: No such file or directory
collect2.exe: error: ld returned 1 exit status

Obviously, something is wrong with the configuration. Maybe _alwayslink_lib_opts function produces options incompatible with mingw. Or my deduplication (seen_alwayslink_libs) doesn't work well with it. Maybe the order of options is important

Debian 11 Bullseye (OpenJDK 17, gcc 10.2.1)

Strangely, my //tests/core/cgo/cc_static_init:test_side_effect_cc fails. This is a reference C++ test.
The Go test //tests/core/cgo/cc_static_init:test_side_effect_go works well to my surprise.

So, the problem is not in rules_go, actually. But something is wrong with the C++ part only

The test produced 0 instead of 42. My theory was that it happened because the GetValue() from main.cpp executed faster than the one in side_effect.cpp. So, I added a sleep in this commit, but it didn't help. Maybe the problem is that alwayslink=True doesn't work well on Debian? It's unlikely though.
Maybe I should get rid of the cc test at all? Maybe there are some other rules_cc effects that lead to such a behavior

upd Let's see if my next commit fixes this error

enscogitans · 2025-09-05T10:32:18Z

Finally, I got everything fixed, as you can see

fmeum · 2025-09-05T12:08:36Z

Thanks and sorry for the late reply!

Avoiding the manual deduplication would require a very different approach to the entire cgo code. Since your change fixes the bug with minimal additional complexity, I think that it's fine as is.

…uplicate symbol errors introduced by rules_go v0.58.1 (#2311) Summary: Consolidate `all_scripts_test.go` to use a single CGO target to fix duplicate symbol errors introduced by rules_go v0.58.1 bazel-contrib/rules_go#4438, included in `rules_go` v0.58.1, causes certain statically linked CGO binaries to fail with duplicate symbol errors. This occurs when a binary depends on more than one CGO library that transitively depends on a common set of object files. `all_scripts_test.go` previously depended on two CGO targets: - `//src/carnot/planner` - `//src/e2e_test/vizier/planner/dump_schemas/godumpschemas` This PR solves this issue by removing the src/e2e_test/vizier/planner/dump_schemas/godumpschemas CGO library and instead generate the protobuf export directly in C++, loading it in the main application. This approach mirrors the existing pattern used in [src/vizier/funcs](https://github.com/pixie-io/pixie/blob/a6349a90b1e4b30f0bb13872ad03dff83a53f363/src/vizier/funcs/BUILD.bazel#L50-L66). **Why not fix `rules_go`?** The `rules_go` change that causes the issue explains that it doesn't include the necessary deduplication logic to avoid these duplicate symbol errors (bazel-contrib/rules_go#4438 (comment)). This tradeoff was [deemed acceptable](bazel-contrib/rules_go#4438 (comment)) since it solved the c++ initialization problem with minimal complexity. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Build should pass Signed-off-by: Dom Del Nano <ddelnano@gmail.com>

…uplicate symbol errors introduced by rules_go v0.58.1 (#2311) Summary: Consolidate `all_scripts_test.go` to use a single CGO target to fix duplicate symbol errors introduced by rules_go v0.58.1 bazel-contrib/rules_go#4438, included in `rules_go` v0.58.1, causes certain statically linked CGO binaries to fail with duplicate symbol errors. This occurs when a binary depends on more than one CGO library that transitively depends on a common set of object files. `all_scripts_test.go` previously depended on two CGO targets: - `//src/carnot/planner` - `//src/e2e_test/vizier/planner/dump_schemas/godumpschemas` This PR solves this issue by removing the src/e2e_test/vizier/planner/dump_schemas/godumpschemas CGO library and instead generate the protobuf export directly in C++, loading it in the main application. This approach mirrors the existing pattern used in [src/vizier/funcs](https://github.com/pixie-io/pixie/blob/a6349a90b1e4b30f0bb13872ad03dff83a53f363/src/vizier/funcs/BUILD.bazel#L50-L66). **Why not fix `rules_go`?** The `rules_go` change that causes the issue explains that it doesn't include the necessary deduplication logic to avoid these duplicate symbol errors (bazel-contrib/rules_go#4438 (comment)). This tradeoff was [deemed acceptable](bazel-contrib/rules_go#4438 (comment)) since it solved the c++ initialization problem with minimal complexity. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Build should pass Signed-off-by: Dom Del Nano <ddelnano@gmail.com> GitOrigin-RevId: bc01887bc88ba9860de93fc03764176837825982

…iables when using alwayslink = True (#4438)" This reverts commit 20f7e2b as it causes a regression (see #4548)

…iables when using alwayslink = True (#4438)" (#4551) This reverts commit 20f7e2b as it causes a regression (see #4548)

…uplicate symbol errors introduced by rules_go v0.58.1 (pixie-io#2311) Summary: Consolidate `all_scripts_test.go` to use a single CGO target to fix duplicate symbol errors introduced by rules_go v0.58.1 bazel-contrib/rules_go#4438, included in `rules_go` v0.58.1, causes certain statically linked CGO binaries to fail with duplicate symbol errors. This occurs when a binary depends on more than one CGO library that transitively depends on a common set of object files. `all_scripts_test.go` previously depended on two CGO targets: - `//src/carnot/planner` - `//src/e2e_test/vizier/planner/dump_schemas/godumpschemas` This PR solves this issue by removing the src/e2e_test/vizier/planner/dump_schemas/godumpschemas CGO library and instead generate the protobuf export directly in C++, loading it in the main application. This approach mirrors the existing pattern used in [src/vizier/funcs](https://github.com/pixie-io/pixie/blob/a6349a90b1e4b30f0bb13872ad03dff83a53f363/src/vizier/funcs/BUILD.bazel#L50-L66). **Why not fix `rules_go`?** The `rules_go` change that causes the issue explains that it doesn't include the necessary deduplication logic to avoid these duplicate symbol errors (bazel-contrib/rules_go#4438 (comment)). This tradeoff was [deemed acceptable](bazel-contrib/rules_go#4438 (comment)) since it solved the c++ initialization problem with minimal complexity. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Build should pass Signed-off-by: Dom Del Nano <ddelnano@gmail.com>

fmeum reviewed Sep 2, 2025

View reviewed changes

enscogitans commented Sep 3, 2025

View reviewed changes

go/private/rules/cgo.bzl Show resolved Hide resolved

go/private/rules/cgo.bzl Show resolved Hide resolved

go/private/rules/cgo.bzl Show resolved Hide resolved

enscogitans changed the title ~~Issue 1486: fix C++ static initialization when using cgo~~ Issue 1486: cgo: fix C++ dynamic initialization of static variables when using alwayslink = True Sep 3, 2025

enscogitans marked this pull request as ready for review September 3, 2025 21:35

enscogitans requested a review from fmeum September 3, 2025 21:35

enscogitans mentioned this pull request Sep 3, 2025

cgo: C++ static initialization not working #1486

Open

fmeum approved these changes Sep 5, 2025

View reviewed changes

enscogitans added 11 commits September 5, 2025 14:08

add test

1b319b0

add whole-archive clinkopts

14e5126

update contributors

ff4d84a

fixes

5c0ac81

drop msvc's .lo.ib

d1bd8f8

undo sleep

0141ae1

more robust side effect impl

f39afbe

simplify test

61f9af8

use static storage variable again

fbbb97b

upd readme, upd dir name

dfd6401

readme

55a9c90

fmeum force-pushed the 1486-fix-static-linking branch from 701f560 to 55a9c90 Compare September 5, 2025 12:08

fmeum enabled auto-merge (squash) September 5, 2025 12:09

fmeum merged commit 20f7e2b into bazel-contrib:master Sep 5, 2025
1 check passed

ddelnano mentioned this pull request Jan 7, 2026

Consolidate all_scripts_test.go to use a single CGO target to fix duplicate symbol errors introduced by rules_go v0.58.1 pixie-io/pixie#2311

Merged

mering mentioned this pull request Feb 3, 2026

[Regression] cgo linking error with rules_go 0.58 #4548

Closed

fmeum added a commit that referenced this pull request Feb 9, 2026

Revert "Issue 1486: cgo: fix C++ dynamic initialization of static var…

0a9f25f

…iables when using alwayslink = True (#4438)" This reverts commit 20f7e2b as it causes a regression (see #4548)

fmeum added a commit that referenced this pull request Feb 9, 2026

Revert "Issue 1486: cgo: fix C++ dynamic initialization of static var…

fbbafef

…iables when using alwayslink = True (#4438)" (#4551) This reverts commit 20f7e2b as it causes a regression (see #4548)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Issue 1486: cgo: fix C++ dynamic initialization of static variables when using alwayslink = True#4438

Issue 1486: cgo: fix C++ dynamic initialization of static variables when using alwayslink = True#4438
fmeum merged 11 commits intobazel-contrib:masterfrom
enscogitans:1486-fix-static-linking

enscogitans commented Sep 2, 2025 •

edited

Loading

Uh oh!

fmeum left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enscogitans commented Sep 3, 2025 •

edited

Loading

Uh oh!

enscogitans commented Sep 5, 2025

Uh oh!

fmeum commented Sep 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

enscogitans commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fmeum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enscogitans commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enscogitans commented Sep 5, 2025

Uh oh!

fmeum commented Sep 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

enscogitans commented Sep 2, 2025 •

edited

Loading

enscogitans commented Sep 3, 2025 •

edited

Loading