runtime: several tests are failing on windows-arm64-aws builder due to redeclaration warnings (upgraded to errors in testing) #46502
Labels
arch-arm64
FrozenDueToAge
NeedsFix
The path to resolution is known, but the work has not been done.
OS-Windows
Milestone
Comments
dmitshur
added
OS-Windows
NeedsInvestigation
|
Likely related to https://golang.org/cl/262797 for #30674. CC @qmuntal . That said that CL is in the 1.15 and 1.16 releases and I don't recall seeing any complaints. |
|
Change https://golang.org/cl/323991 mentions this issue: |
Maybe only Windows/ARM64 uses clang-based C toolchain which warns that, whereas previously Windows/AMD64 and 386 use GCC which doesn't warn that? |
I checked that hypothesis and it is right, for the case of a function declared in the C preamble and also exported in Go we are currently adding two declarations in the C export header, one with A solution would be to prepend |
|
Digging a little bit deeper, clang emit a warning and not an error because redeclaring a function with My understanding is that the C export header cannot contain any function call, only function declarations, so it would be safe to ignore this warning using Lines 1615 to 1623 in 0214440 Doing this would simplify the |
SGTM. But I am not an expert here so defer to Go Team. Alex |
|
I rewrote a bunch of tests not to depend on this kind of redeclaration. Are these new? |
|
The test doesn't seem new to me. |
|
Change https://golang.org/cl/326310 mentions this issue: |
|
In my local testing on a Surface Pro X device, this issue reproduces only when So, this passes: And this reproduces the test failure: For some reason, compilation warnings (which are upgraded to errors on builders) aren't printed even with It's possible Russ did not run into this since setting a non-empty |
|
After creating a fresh builder image with llvm-mingw-20201020, I can also confirm that the tests fail similarly, reinforcing @dmitshur's work. Test output |
|
I've got a fix for that in https://go-review.googlesource.com/c/go/+/326310/ But I'm now battling the next clang-related issue, where apparently it wants a microsoft-style import library, rather than a straight up .dll pretending to be a .a like gcc takes. |
|
Change https://golang.org/cl/327209 mentions this issue: |
|
Change https://golang.org/cl/327211 mentions this issue: |
|
Change https://golang.org/cl/327309 mentions this issue: |
dmitshur
added
NeedsFix
gopherbot
pushed a commit
that referenced
this issue
Jun 13, 2021
LLD won't import a .dll directly and instead requires an import library. So generate these using -out-implib, the same way as was done in CL 312046, where it makes sense, and elsewhere build the import library using a def file. We can't use -out-implib all the time, because the output file gets overwritten each time the linker is called, rather than merged. Updates #46502. Change-Id: Iefe54cb6c576004b83b1039ba673881b8640423d Reviewed-on: https://go-review.googlesource.com/c/go/+/327211 Trust: Jason A. Donenfeld <Jason@zx2c4.com> Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
gopherbot
pushed a commit
that referenced
this issue
Jun 14, 2021
The platform supports c-shared now, so flip this on. I've given this a small smoke test using [1], and it was able to pass packets and generally function well. Since [1] uses quite a bit of Go functionality under the hood, I think it's a decent test that a lot of things that should be working are working. So this commit enables it. [1] https://git.zx2c4.com/wireguard-windows/about/embeddable-dll-service/README.md Updates #46502. Change-Id: I5c771d033bd20e5ce472c315d7c207bbc1020b4a Reviewed-on: https://go-review.googlesource.com/c/go/+/326310 Trust: Jason A. Donenfeld <Jason@zx2c4.com> Trust: Alex Brainman <alex.brainman@gmail.com> Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
|
Change https://golang.org/cl/331670 mentions this issue: |
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Jun 30, 2021
This change removes the windows-arm64-aws buildlet and replaces all usages with the windows-arm64-10 buildlet. - Remove the known issue from the windows-arm64-10 buildlet, which has been passing reliably. - Enable tests during releases, and add a slowbot alias, as it's a much faster buildlet. Updates golang/go#46502 For golang/go#42604 Change-Id: Ib9f7c3d5391b01e303b43bbdad030b3f94147c5d Reviewed-on: https://go-review.googlesource.com/c/build/+/331670 Trust: Alexander Rakoczy <alex@golang.org> Run-TryBot: Alexander Rakoczy <alex@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org>
|
Change https://golang.org/cl/345129 mentions this issue: |
gopherbot
pushed a commit
to golang/build
that referenced
this issue
Aug 25, 2021
By now, all outstanding issues preventing release tests for windows-arm64 from being turned on by default are resolved: - Issues golang/go#46406 and golang/go#46502 are fixed. - The other remaining blocker golang/go#47017 is determined to be a builder-specific issue (not reproducible on physical Windows ARM64 hardware) that by now happens infrequently enough that automated retries should be able to cover for future occurrences. Issue golang/go#47965 is opened to track progress on this. - The builder performance is good and allows release tests to complete quickly, not adding any bottlenecks to the release process. If something changes, we can revisit this, but information available so far suggests it's a good time to start running release tests for windows/arm64 by default. Fixes golang/go#47017. Updates golang/go#47965. Change-Id: Ie96164821a2f8e795a22ac2d36b7292587a3e117 Reviewed-on: https://go-review.googlesource.com/c/build/+/345129 Reviewed-by: Alexander Rakoczy <alex@golang.org> Trust: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
More Failing Tests in
runtimeHere are the other failing test in
runtime, all with the same problem:Source: https://storage.googleapis.com/go-build-log/986587f4/windows-arm64-aws_fcda3dac.log from CL 323990.
There's also a test in
cmd/linkthat builds testprogcgo with the same problem:Source: https://storage.googleapis.com/go-build-log/0f330f99/windows-arm64-aws_80fa3b05.log from CL 323993.
This is possibly connected to the builder using a newer version of the C toolchain, as discussed in #46406.
Note these are warnings if run locally, but the build system upgrade warnings during test execution into to failures.
CC @rsc, @golang/release, @zx2c4.
The text was updated successfully, but these errors were encountered: