Rules_go: go: link error with libmsvcrt.a when race mode enabled on Windows

[c-parsons]

Example failure:
https://buildkite.com/bazel/rules-go-golang/builds/2327#145a71ba-dee2-4eb7-b7ab-ff045ded70de

Looks like tests/core/race/race_test.go is failing with (snip):

        race_test.go:269: running: bazel test --features=race //:racy_test --test_arg=-wantrace=true
        race_test.go:277: unexpected build failure: exit status 1
            stderr:
            Loading: 
            Loading: 0 packages loaded
            INFO: Build option --features has changed, discarding analysis cache.
            Analyzing: target //:racy_test (0 packages loaded, 0 targets configured)
            INFO: Analyzed target //:racy_test (0 packages loaded, 7299 targets configured).
            INFO: Found 1 test target...
            [0 / 5] [Prepa] BazelWorkspaceStatusAction stable-status.txt
            ERROR: C:/b/ksckfvdp/bazel_testing/bazel_go_test/main/BUILD.bazel:27:8: GoLink racy_test_/racy_test.exe failed (Exit 1)
            c:/tools/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/lib/../lib/libmsvcrt.a(/2203): duplicate symbol reference: _unlock_file in both libgcc(.text) and libgcc(.data)
            libgcc(.text): relocation target ___chkstk_ms not defined
            libgcc(.text): relocation target atexit not defined
            link: error running the following subcommand: exit status 2

Not sure if this is due to an infra change; maybe someone more familiar with the tests can chime in?

[jayconrod]

This appears to be caused by a change in CI configuration. We've been discussing this in bazelbuild/bazel#11885.

cc @meteorcloudy

[jayconrod]

@meteorcloudy Any update? rules_go is still red on Windows on every run.

[meteorcloudy]

Sorry, spent yesterday on perf, will look into this now!

[meteorcloudy]

@jayconrod I suspect it's because the upgrade of mingw-w64-x86_64-gcc package. Currently, I have 10.2 installed on CI, 8.3 installed locally. The test passed locally.
Can you try to update this package and see if you can reproduce the failure? If so, maybe we'll have to disable this test temporarily and find out a solution in rules_go.

[meteorcloudy]

We cannot downgrade version since pacman only support installing the latest version available..

[jayconrod]

I'll try upgrading to 10.2 today to reproduce locally. Will let you know what I find.

[meteorcloudy]

OK, I also tried to reproduce locally, this is getting very weird.

I now have 10.2 installed both locally and on CI. But it still passed locally and failed on CI.

I narrowed down the failing command to

external\go_sdk\pkg\tool\windows_amd64\link.exe -v -o C:\b\2jfjgelz\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd_\racy_cmd.exe -extld c:/tools/msys64/mingw64/bin/gcc -race -buildid=redacted C:\b\2jfjgelz\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd.a

Locally, I have:

google\pcloudy@PCLOUDY2-W C:\src\tmp\3ddvlgkp\execroot\__main__>external\go_sdk\pkg\tool\windows_amd64\link.exe -v -o bazel-out\x64_windows-fastbuild\bin\racy_cmd_\racy_cmd.exe -extld c:/tools/msys64/mingw64/bin/gcc -race -buildid=redacted bazel-out\x6
4_windows-fastbuild\bin\racy_cmd.a
HEADER = -H10 -T0xffffffffffffffff -R0xffffffff
searching for runtime/race.a in bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime\race.a
searching for runtime.a in bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime.a
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-libgcc-file-name]
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-file-name=libmingwex.a]
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-file-name=libmingw32.a]
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-file-name=libmsvcrt.a]
pclntab=623041 bytes, funcdata total 156632 bytes
symsize = 0
59884 symbols, 18404 reachable
108132 liveness data

But on CI:

C:\b\2jfjgelz\execroot\__main__>external\go_sdk\pkg\tool\windows_amd64\link.exe -v -o C:\b\2jfjgelz\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd_\racy_cmd.exe -extld c:/tools/msys64/mingw64/bin/gcc -race -buildid=redacted C:\b\2jfjgelz\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd.a
HEADER = -H10 -T0xffffffffffffffff -R0xffffffff
searching for runtime/race.a in bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime\race.a
searching for runtime.a in bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime.a
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-libgcc-file-name]
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-file-name=libmingwex.a]
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-file-name=libmingw32.a]
c:/tools/msys64/mingw64/bin/gcc [-m64 --print-file-name=libmsvcrt.a]
c:/tools/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/lib/../lib/libmsvcrt.a(/2203): duplicate symbol reference: _unlock_file in both libgcc(.text) and libgcc(.data)
pclntab=626673 bytes, funcdata total 156632 bytes
libgcc(.text): relocation target ___chkstk_ms not defined
libgcc(.text): relocation target atexit not defined
symsize = 0
60040 symbols, 18505 reachable
108132 liveness data

Besides environment variables, I also checked the sha256sum of libmsvcrt.a, libgcc.a, link.exe and race_cmd.a, they all match between local and CI!

I'm running out of ideas.. 😔

[meteorcloudy]

Oh, I just check race.a and runtime.a. It turned out race.a is different:

google\pcloudy@PCLOUDY2-W C:\src\tmp\3ddvlgkp\execroot\__main__>dir bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime\race.a
 Volume in drive C is gWindows
 Volume Serial Number is 8600-387C

 Directory of C:\src\tmp\3ddvlgkp\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime

08/25/2020  01:21 PM           468,008 race.a
               1 File(s)        468,008 bytes
               0 Dir(s)  647,950,585,856 bytes free

VS

C:\b\2jfjgelz\execroot\__main__>dir bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime\race.a
 Volume in drive C has no label.
 Volume Serial Number is CEAA-B826

 Directory of C:\b\2jfjgelz\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime

08/25/2020  09:06 AM           467,410 race.a
               1 File(s)        467,410 bytes
               0 Dir(s)  493,852,413,952 bytes free

[jayconrod]

I verified this test passes locally both before and after an upgrade. The old version I had was:

mingw-w64-x86_64-gcc 8.2.1+20181214-1

The new version was:

mingw-w64-x86_64-gcc 10.2.0-1

After upgrading, I still got a cache hit on //tests/core/race:race_test, even though the C toolchain had changed. race_test is weird because it runs another instance of Bazel in a temporary directory, but I think the test binary is still configured to depend on the C toolchain, so I'm surprised to get a cache hit there. I verified it passed with --nocache_test_result, then again after bazel clean --expunge.

I'm not sure how cache keys are built for the C toolchain, but it doesn't seem like they take the C compiler version or file hash into account. Is it possible that would cause old and new artifacts to be mixed together when the remote cache is used? I think (but am not sure) that CI uses a remote cache on Windows and local builds generally don't.

[meteorcloudy]

Indeed, the cc toolchain doesn't verify the compiler version installed, that's why you'll get the cache hit. Users has to do a clean rebuild after upgrading their compiler.

But in this case, I don't think remote caching was the problem since I was manually building without caching at all on a CI Windows VM.

As I discovered bazel-out\x64_windows-fastbuild\bin\external\io_bazel_rules_go\stdlib_\pkg\windows_amd64_race\runtime\race.a is different between local and CI, can you tell me how this file is built? Maybe I can figure out what the difference is.

[jayconrod]

That file is built as part of the Go standard library, which is rebuilt for non-default configurations like race mode. bazel build --@io_bazel_rules_go//go/config:race @io_bazel_rules_go//:stdlib should build it in that configuration. A few extra flags may be needed on Windows since the mingw compiler is not configured by default; they should all be in presubmit.yml.

An odd thing here is that //tests/core/cgo:race_test succeeds. That should build the standard library in the same configuration. //tests/core/cgo:race_test just tests whether targets can build in race mode; it's a regular go_test. //tests/core/race:race_test is a go_bazel_test that invokes Bazel recursively in a temporary workspace. This may indicate a difference in how Bazel is invoked locally and in CI. The recursive invocation of Bazel won't get the flags from the top-level invocation because the test doesn't know what they are. I've specified the flags I know are important through --test_env, especially by setting GO_BAZEL_TEST_BAZELFLAGS, which the test passes to Bazel.

[meteorcloudy]

Hmm, that command doesn't seem to build anything:

google\pcloudy@PCLOUDY2-W C:\workdir\rules_go_2>bazel build --cpu=x64_windows --compiler=mingw-gcc "--action_env=PATH=C:\tools\msys64\usr\bin;C:\tools\msys64\bin;C:\tools\msys64\mingw64\bin;C:\python3\Scripts\;C:\python3;C:\Windows\system32;C:\Windows;
C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\
Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\tools\msys64\usr\bin;c:\openjdk\bin;C:\Program Files (x86)\Windows Kits\8.1\Windows Performance Toolkit\;C:\Program Files\CMake\bin;c:\ninja;c:\bazel;c:\buildkite" 
 --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows_mingw --host_platform=@io_bazel_rules_go//go/toolchain:windows_amd64_cgo --platforms=@io_bazel_rules_go//go/toolchain:windows_amd64_cgo --incompatible_enable_cc_toolchain_resolution --@io_
bazel_rules_go//go/config:race @io_bazel_rules_go//:stdlib
INFO: Invocation ID: 82ccdb6e-8cfc-4577-bd61-fa04b69a8611
INFO: Reading 'startup' options from c:\users\pcloudy\.bazelrc: --output_user_root=C:/src/tmp
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=252
INFO: Options provided by the client:
  'build' options: --python_path=C:/Python36/python.exe
INFO: Reading rc options for 'build' from c:\users\pcloudy\.bazelrc:
  'build' options: --curses=yes --color=yes --verbose_failures --announce_rc --disk_cache=C:/src/tmp/bazel_disk_cache
DEBUG: C:/src/tmp/xrqnmb3g/external/bazel_toolchains/rules/rbe_repo/version_check.bzl:68:14: 
Current running Bazel is ahead of bazel-toolchains repo. Please update your pin to bazel-toolchains repo in your WORKSPACE file.
DEBUG: C:/src/tmp/xrqnmb3g/external/bazel_toolchains/rules/rbe_repo/checked_in.bzl:125:14: buildkite_config not using checked in configs; Bazel version 3.4.1 was picked/selected but no checked in config was found in map {"0.20.0": ["8.0.0"], "0.21.0": 
["8.0.0"], "0.22.0": ["8.0.0", "9.0.0"], "0.23.0": ["8.0.0", "9.0.0"], "0.23.1": ["8.0.0", "9.0.0"], "0.23.2": ["9.0.0"], "0.24.0": ["9.0.0"], "0.24.1": ["9.0.0"], "0.25.0": ["9.0.0"], "0.25.1": ["9.0.0"], "0.25.2": ["9.0.0"], "0.26.0": ["9.0.0"], "0.2
6.1": ["9.0.0"], "0.27.0": ["9.0.0"], "0.27.1": ["9.0.0"], "0.28.0": ["9.0.0"], "0.28.1": ["9.0.0"], "0.29.0": ["9.0.0"], "0.29.1": ["9.0.0", "10.0.0"], "1.0.0": ["9.0.0", "10.0.0"], "1.0.1": ["10.0.0"], "1.1.0": ["10.0.0"], "1.2.0": ["10.0.0"], "1.2.1
": ["10.0.0"], "2.0.0": ["10.0.0"], "2.1.0": ["10.0.0"], "2.1.1": ["10.0.0", "11.0.0"], "2.2.0": ["11.0.0"], "3.0.0": ["11.0.0"], "3.1.0": ["11.0.0"], "3.2.0": ["11.0.0"], "3.3.0": ["11.0.0"], "3.3.1": ["11.0.0"]}
INFO: Analyzed target //:stdlib (24 packages loaded, 6781 targets configured).
INFO: Found 1 target...
Target //:stdlib up-to-date (nothing to build)
INFO: Elapsed time: 1.566s, Critical Path: 0.03s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

[jayconrod]

Ah, I apologize, //:stdlib doesn't return DefaultInfo, so building it directly will have no effect. You'll need to build another target that depends on it. Building //tests/core/go_binary:hello with --//go/config:race should do it though.

[meteorcloudy]

I see, thanks!

So making some progress here. I constructed a minimal reproducible case here: https://github.com/meteorcloudy/my_tests/tree/master/debug_go_issue_2614

It is now failing on my local machine as well, maybe you can also give it a try?
Just clone this folder and do bazel build --features=race //:racy_cmd with Bazel 3.4.1

google\pcloudy@PCLOUDY2-W C:\tools\msys64\home\pcloudy\workspace\my_tests\debug_go_issue_2614>bazel build --features=race //:racy_cmd -s
INFO: Invocation ID: 83921e3e-6269-4627-8cf8-9146f8d79e11
INFO: Reading 'startup' options from c:\users\pcloudy\.bazelrc: --output_user_root=C:/src/tmp
INFO: Options provided by the client:
  Inherited 'common' options: --isatty=1 --terminal_columns=252
INFO: Options provided by the client:
  'build' options: --python_path=C:/Python36/python.exe
INFO: Reading rc options for 'build' from c:\tools\msys64\home\pcloudy\workspace\my_tests\debug_go_issue_2614\.bazelrc:
  'build' options: --cpu=x64_windows --compiler=mingw-gcc --extra_toolchains=@local_config_cc//:cc-toolchain-x64_windows_mingw --host_platform=@io_bazel_rules_go//go/toolchain:windows_amd64_cgo --platforms=@io_bazel_rules_go//go/toolchain:windows_amd64
_cgo --incompatible_enable_cc_toolchain_resolution
INFO: Reading rc options for 'build' from c:\users\pcloudy\.bazelrc:
  'build' options: --curses=yes --color=yes --verbose_failures --announce_rc --disk_cache=C:/src/tmp/bazel_disk_cache
INFO: Analyzed target //:racy_cmd (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
SUBCOMMAND: # //:racy_cmd [action 'GoLink racy_cmd_/racy_cmd.exe', configuration: 3c46d5c4221c3f9513c440c84a070f8048eb6aef59e6601d3b7515e7ae949aea, execution platform: @io_bazel_rules_go//go/toolchain:windows_amd64_cgo]
cd C:/src/tmp/na7y6ljb/execroot/__main__
  SET CGO_ENABLED=1
    SET GOARCH=amd64
    SET GOOS=windows
    SET GOPATH=
    SET GOROOT=bazel-out/x64_windows-fastbuild/bin/external/io_bazel_rules_go/stdlib_
    SET GOROOT_FINAL=GOROOT
    SET PATH=c:/tools/msys64/mingw64/bin
  bazel-out/host/bin/external/go_sdk/builder.exe link -sdk external/go_sdk -installsuffix windows_amd64_race -tags race -package_list bazel-out/host/bin/external/go_sdk/packages.txt -o bazel-out/x64_windows-fastbuild/bin/racy_cmd_/racy_cmd.exe -main ba
zel-out/x64_windows-fastbuild/bin/racy_cmd.a -p example.com/racy -- -extld c:/tools/msys64/mingw64/bin/gcc -race -buildid=redacted
ERROR: C:/tools/msys64/home/pcloudy/workspace/my_tests/debug_go_issue_2614/BUILD:19:10: GoLink racy_cmd_/racy_cmd.exe failed (Exit 1): builder.exe failed: error executing command
  cd C:/src/tmp/na7y6ljb/execroot/__main__
  SET CGO_ENABLED=1
    SET GOARCH=amd64
    SET GOOS=windows
    SET GOPATH=
    SET GOROOT=bazel-out/x64_windows-fastbuild/bin/external/io_bazel_rules_go/stdlib_
    SET GOROOT_FINAL=GOROOT
    SET PATH=c:/tools/msys64/mingw64/bin
  bazel-out/host/bin/external/go_sdk/builder.exe link -sdk external/go_sdk -installsuffix windows_amd64_race -tags race -package_list bazel-out/host/bin/external/go_sdk/packages.txt -o bazel-out/x64_windows-fastbuild/bin/racy_cmd_/racy_cmd.exe -main ba
zel-out/x64_windows-fastbuild/bin/racy_cmd.a -p example.com/racy -- -extld c:/tools/msys64/mingw64/bin/gcc -race -buildid=redacted
Execution platform: @io_bazel_rules_go//go/toolchain:windows_amd64_cgo
c:/tools/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/lib/../lib/libmsvcrt.a(/2203): duplicate symbol reference: _unlock_file in both libgcc(.text) and libgcc(.data)
libgcc(.text): relocation target ___chkstk_ms not defined
libgcc(.text): relocation target atexit not defined
link: error running the following subcommand: exit status 2
CGO_ENABLED=1 \
GOARCH=amd64 \
GOOS=windows \
GOPATH= \
GOROOT=bazel-out/x64_windows-fastbuild/bin/external/io_bazel_rules_go/stdlib_ \
GOROOT_FINAL=GOROOT \
PATH=c:/tools/msys64/mingw64/bin \
SYSTEMDRIVE=C: \
SYSTEMROOT=C:\Windows \
TEMP=C:\src\temp \
TMP=C:\src\temp \
external\go_sdk\pkg\tool\windows_amd64\link.exe -importcfg C:\src\tmp\na7y6ljb\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd_\importcfg261826519 -o C:\src\tmp\na7y6ljb\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd_\rac
y_cmd.exe -extld c:/tools/msys64/mingw64/bin/gcc -race -buildid=redacted C:\src\tmp\na7y6ljb\execroot\__main__\bazel-out\x64_windows-fastbuild\bin\racy_cmd.a
Target //:racy_cmd failed to build
INFO: Elapsed time: 2.957s, Critical Path: 0.67s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

[jayconrod]

Thanks, that reproduces on my machine, too.

So it looks like this is probably not an issue with CI then. I'll investigate whether this is a problem with mingw or the way rules_go uses it. I'll likely end up disabling this test.

[meteorcloudy]

Thanks!

[meteorcloudy]

I can confirm the difference of race.a indeed caused this problem (I replaced this library in one build with another build). But it's hard for me to figure out how this library is built, that's probably where you want to look into.

[jayconrod]

I'm not entirely sure I understand what's going on here, but it seems to be a problem between Go itself and mingw. I was able to reproduce this with go build -race hello.go, without involving Bazel at all. golang/go#23649 seems to be a related issue. CL 197977 was supposed to have fixed that, but it added the dependency on libmsvcrt.a, which is causing this failure at another version.

I think the right resolution here is to force external linking when race mode is enabled on Windows. That seems to resolve the problem.

[meteorcloudy]

Nice, thanks for fixing this!

[jayconrod]

@meteorcloudy Thanks for investigating it and narrowing it down! I figured since //tests/core/cgo:race_test was passing it couldn't be an issue with rules_go or Go. But there was a difference between that one and the other test that triggered external linking (that is, linking with the C linker instead of the Go linker), so that test didn't trigger the bug.