Closed dmitshur closed 3 years ago
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: runtime: skip tests that build testprogcgo on windows/arm64
That said that CL is in the 1.15 and 1.16 releases and I don't recall seeing any complaints.
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?
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 dllexport
and the other without. Clearly, GCC does not complain about that, although I haven't found any reference on the web explaining why it doesn't.
A solution would be to prepend dllexport
in the C preamble function if it doesn't contain it and is exported in Go. I can take a stab at this issue, as it can be tested using mingw toolchain.
Digging a little bit deeper, clang emit a warning and not an error because redeclaring a function with __declspec(dllexport)
or __declspec(dllimport)
is "technically" fine as long as the function is not used between both declarations. See https://reviews.llvm.org/D5087 for more info.
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 -Wno-dll-attribute-on-redeclaration
, as it is already happening with other warnings:
Doing this would simplify the cgo
code that generates the C export header, as implementing the solution I was proposing in the previous comment is not straight forward.
it would be safe to ignore this warning using
-Wno-dll-attribute-on-redeclaration
, as it is already happening with other warnings:
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: cmd/internal/sys: mark windows/arm64 as c-shared-capable
In my local testing on a Surface Pro X device, this issue reproduces only when GO_BUILDER_NAME
is set to a non-empty string (which is the case on builders). It requires clearing out the build cache (go clean -cache
) after successful invocations to cause the testprogcgo
program to be rebuilt.
So, this passes:
$ GO_BUILDER_NAME='' go test -count=1 -v -run='TestCgoDLLImports' runtime
And this reproduces the test failure:
$ go clean -cache # so the testprogcgo program is built from source again
$ GO_BUILDER_NAME='foo' go test -count=1 -v -run='TestCgoDLLImports' runtime
For some reason, compilation warnings (which are upgraded to errors on builders) aren't printed even with -v
flag.
It's possible Russ did not run into this since setting a non-empty GO_BUILDER_NAME
during local development is rarely done, and there's no indication of a problem otherwise (that seems like a bug in itself).
After creating a fresh builder image with llvm-mingw-20201020, I can also confirm that the tests fail similarly, reinforcing @dmitshur's work.
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: cmd/go: pass -Wno-dll-attribute-on-redeclaration to clang
Change https://golang.org/cl/327211 mentions this issue: cmd/go, misc/cgo: account for LLD differences on Windows
Change https://golang.org/cl/327309 mentions this issue: runtime: testprogcgo: don't call exported Go functions directly from Go
Change https://golang.org/cl/331670 mentions this issue: dashboard,cmd/release: replace windows-arm64-aws with windows-arm64-10
Change https://golang.org/cl/345129 mentions this issue: cmd/release: start running release tests for windows-arm64
More Failing Tests in
runtime
Here are the other failing test in `runtime`, all with the same problem: ``` --- FAIL: TestCgoPanicDeadlock (17.25s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestCgoTracebackContext (17.25s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestCgoCCodeSIGPROF (17.24s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestCgoCheckBytes (0.00s) crash_cgo_test.go:188: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestBigStackCallbackCgo (0.00s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestCgoExternalThreadPanic (0.00s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestCgoTraceback (0.00s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders --- FAIL: TestCgoCrashHandler (0.00s) crash_test.go:54: building testprogcgo []: exit status 1 # runtime/testdata/testprogcgo In file included from _cgo_export.c:4: cgo-gcc-export-header-prolog:43:35: warning: redeclaration of 'GoNop' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] aprof.go:13:14: note: previous declaration is here cgo-gcc-export-header-prolog:44:35: warning: redeclaration of 'goBigStack1' should not add 'dllexport' attribute [-Wdll-attribute-on-redeclaration] bigstack_windows.go:9:13: note: previous declaration is here go build runtime/testdata/testprogcgo: C compiler warning promoted to error on Go builders ```
Source: https://storage.googleapis.com/go-build-log/986587f4/windows-arm64-aws_fcda3dac.log from CL 323990.
There's also a test in
cmd/link
that 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.