[go] cmd/cgo: use --no-gc-sections if available
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
cmd/cgo: use --no-gc-sections if available
zig cc passes `--gc-sections` to the underlying linker, which then
causes undefined symbol errors when compiling with cgo but without C
code. Minimal example:
**main.go**
```
package main
import _ "runtime/cgo"
func main() {}
```
Run (works after the patch, doesn't work before):
```
CGO_ENABLED=1 CC="zig cc" go build main.go
```
@ianlancetaylor suggested:
> It would be fine with me if somebody wants to send a cgo patch that
passes -Wl,--no-gc-sections, with a fallback if that option is not
supported.
... and this is what we are doing. Tested with zig
0.10.0-dev.896+e620b692c
Fixes #52690
Change-Id: Ib6d1b2bd59335e9663afefd360ddad7da358a938
GitHub-Last-Rev: 136bf8fb4bf143247d6c3c7683f2cdd071aa2e6c
GitHub-Pull-Request: golang/go#52815
---
M src/cmd/go/internal/work/exec.go
1 file changed, 46 insertions(+), 0 deletions(-)
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 15b9e1e..590a63f 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -2528,6 +2528,13 @@
a = append(a, "-Qunused-arguments")
}
+ // zig cc passes --gc-sections to the underlying linker, which then causes
+ // undefined symbol errors when compiling with cgo but without C code.
+ // https://github.com/golang/go/issues/52690
+ if b.gccSupportsFlag(compiler, "-Wl,--no-gc-sections") {
+ a = append(a, "-Wl,--no-gc-sections")
+ }
+
// disable word wrapping in error messages
a = append(a, "-fmessage-length=0")
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gopher Robot (Gerrit)
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Bryan Mills (Gerrit)
Attention is currently required from: Michael Matloob.
Patch set 1:Run-TryBot +1Code-Review +1
2 comments:
Commit Message:
Patch Set #1, Line 20:
works after the patch, doesn't work beforeThis should have a regression test, even if the builders don't run it yet.
Are there existing `cmd/go` tests that fail when run with
```
CGO_ENABLED=1 CC="zig cc" go test cmd/go
```
?If so, please mention one or more of those tests here. (If not, please add one that fails locally before this change and passes after it.)
Patchset:
TRY=linux-amd64-clang,longtest
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Michael Matloob.
1 comment:
Patchset:
1 of 32 SlowBots failed. […]
#52545
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Michael Matloob.
Bryan Mills removed a vote from this change.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Michael Matloob.
1 comment:
Patchset:
Build is still in progress... Status page: https://farmer.golang.org/try?commit=12336990 […]
#52829
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Michael Matloob.
Bryan Mills removed a vote from this change.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Michael Matloob.
1 comment:
File src/cmd/go/internal/work/exec.go:
Patch Set #1, Line 2534:
if b.gccSupportsFlag(compiler, "-Wl,--no-gc-sections") {Unfortunately, this won't work. The gccSupportsFlag method just does a compilation using -c, so the -Wl option does nothing.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Patchset:
1 of 32 SlowBots failed. […]
Ack
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Michael Matloob.
2 comments:
Commit Message:
Patch Set #1, Line 20:
works after the patch, doesn't work before
This should have a regression test, even if the builders don't run it yet. […]
Good suggestion. src/runtime/testprognet fails to build:
```
motiejus ~/code/go/src/runtime/testdata/testprognet $ CC="zig cc" go build .
# std/runtime/testdata/testprognet
net(.text): relocation target __errno_location not defined
net(.text): relocation target getaddrinfo not defined
net(.text): relocation target freeaddrinfo not defined
net(.text): relocation target gai_strerror not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target fwrite not defined
runtime/cgo(.text): relocation target vfprintf not defined
runtime/cgo(.text): relocation target fputc not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_create not defined
runtime/cgo(.text): relocation target nanosleep not defined
runtime/cgo(.text): relocation target pthread_detach not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target strerror not defined
runtime/cgo(.text): relocation target fprintf not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_mutex_lock not defined
runtime/cgo(.text): relocation target pthread_cond_wait not defined
runtime/cgo(.text): relocation target pthread_mutex_unlock not defined
runtime/cgo(.text): relocation target pthread_cond_broadcast not defined
runtime/cgo(.text): relocation target malloc not defined
/usr/local/go/pkg/tool/linux_amd64/link: too many errors
motiejus ~/code/go/src/runtime/testdata/testprognet $ CC="zig cc -Wl,--no-gc-sections" go build .
motiejus ~/code/go/src/runtime/testdata/testprognet $ ./testprognet
usage: ./testprognet name-of-test
motiejus ~/code/go/src/runtime/testdata/testprognet $
```However, the builders can't run it yet, unless we install zig (should we?).
I guess I'll update the commit message for now. If you decide we should install zig to the builders and run it as a regression test, I can look into it, but as a separate diff.
File src/cmd/go/internal/work/exec.go:
Patch Set #1, Line 2534:
if b.gccSupportsFlag(compiler, "-Wl,--no-gc-sections") {
Unfortunately, this won't work. […]
Ack
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Ian Lance Taylor, Michael Matloob, Motiejus Jakštys.
1 comment:
Commit Message:
Patch Set #1, Line 20:
works after the patch, doesn't work before
Good suggestion. src/runtime/testprognet fails to build: […]
Just updating the commit message seems fine for now.
FWIW, if you want to add a builder that uses `zig cc` you might be able to model it on the `linux-amd64-clang` builder, for which the image-creation script is in https://cs.opensource.google/go/x/build/+/master:env/linux-x86-clang/
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Michael Matloob, Motiejus Jakštys.
Gerrit Bot uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Bryan Mills, TryBot-Result+1 by Gopher Robot
cmd/cgo: use --no-gc-sections if available
cmd/cgo: use -Wl,--no-gc-sections if available
This change adds `--no-gc-sections` to the external linker if the flag
is understood.
zig cc passes `--gc-sections` to the underlying linker, which then
causes undefined symbol errors when compiling with cgo but without C
code. Minimal example:
**main.go**
```
package main
import _ "runtime/cgo"
func main() {}
```
Run (works after the patch, doesn't work before):
```
CC="zig cc" go build main.go
```
Among the existing code, `src/runtime/testdata/testprognet` fails too:
```
src/runtime/testdata/testprognet$ CC="zig cc" go build .
net(.text): relocation target __errno_location not defined
net(.text): relocation target getaddrinfo not defined
net(.text): relocation target freeaddrinfo not defined
net(.text): relocation target gai_strerror not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target fwrite not defined
runtime/cgo(.text): relocation target vfprintf not defined
runtime/cgo(.text): relocation target fputc not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_create not defined
runtime/cgo(.text): relocation target nanosleep not defined
runtime/cgo(.text): relocation target pthread_detach not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target strerror not defined
runtime/cgo(.text): relocation target fprintf not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_mutex_lock not defined
runtime/cgo(.text): relocation target pthread_cond_wait not defined
runtime/cgo(.text): relocation target pthread_mutex_unlock not defined
runtime/cgo(.text): relocation target pthread_cond_broadcast not defined
runtime/cgo(.text): relocation target malloc not defined
```
With the patch both examples build as expected.
@ianlancetaylor suggested:
> It would be fine with me if somebody wants to send a cgo patch that
passes -Wl,--no-gc-sections, with a fallback if that option is not
supported.
... and this is what we are doing. Tested with zig
0.10.0-dev.2252+a4369918b
Fixes #52690
Change-Id: Ib6d1b2bd59335e9663afefd360ddad7da358a938
GitHub-Last-Rev: e42ccc7b975382a11c22281f1d1ffbca08ad6bf2
GitHub-Pull-Request: golang/go#52815
---
M src/cmd/go/internal/work/exec.go
M src/cmd/go/internal/work/gccgo.go
2 files changed, 121 insertions(+), 18 deletions(-)
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Michael Matloob.
2 comments:
Commit Message:
Patch Set #1, Line 20:
works after the patch, doesn't work before
Just updating the commit message seems fine for now.
FWIW, if you want to add a builder that uses `zig cc` you might be able to model it on the `linux-amd64-clang` builder, for which the image-creation script is in https://cs.opensource.google/go/x/build/+/master:env/linux-x86-clang/
Once this is landed, I will file a separate issue to discuss how to make zig-cc part of the CI process. This will help me focus on the `--no-gc-sections` here for now.
Patchset:
Added patchset 2, resolving Ian's concern: we will now have two functions to test compiler/linker flags, respectively.
I made two commits (one renaming `gccSupportsFlag`) and one fixing the original issue, but Gerrit doesn't seem to like it. At least I can't find the first commit that only does the rename.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob, Motiejus Jakštys.
5 comments:
Commit Message:
Patch Set #2, Line 9:
cmd/cgo: use -Wl,--no-gc-sections if availableThis line duplicates the topic line. Please remove (by editing the first comment in the GitHub pull request).
Patch Set #2, Line 11:
This change adds `--no-gc-sections` to the external linker if the flagI think we can remove this sentence also.
Patchset:
Added patchset 2, resolving Ian's concern: we will now have two functions to test compiler/linker fl […]
A stack of commits in a GitHub pull request will be squashed when it is imported into Gerrit. If you want to use a stack of CLs in Gerrit, then I think you have to start in Gerrit. This change is fine as a squashed commit, though.
File src/cmd/go/internal/work/exec.go:
Patch Set #2, Line 2584:
panic(fmt.Sprintf("use gccSupportsLinkerFlag for %q", flag))base.Fatalf would be better than panic here. Similarly below.
Patch Set #2, Line 2631:
cmdArgs = append(cmdArgs, "-c")I'm not following this. We already pass -c. Passing -c is not going to cause a link.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob, Motiejus Jakštys.
Gerrit Bot uploaded patch set #3 to this change.
cmd/cgo: use --no-gc-sections if available
2 files changed, 116 insertions(+), 18 deletions(-)
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob, Motiejus Jakštys.
Gerrit Bot uploaded patch set #4 to this change.
cmd/cgo: use --no-gc-sections if available
zig cc passes `--gc-sections` to the underlying linker, which then
causes undefined symbol errors when compiling with cgo but without C
code. Add `-Wl,--no-gc-sections` to make it work with zig cc. Minimal
example:
**main.go**
package main
import _ "runtime/cgo"
func main() {}
Run (works after the patch, doesn't work before):
CC="zig cc" go build main.go
Among the existing code, `src/runtime/testdata/testprognet` fails to
build:
GitHub-Last-Rev: 856a2e0f356609cf331a88694e0ac311cae89a6f
GitHub-Pull-Request: golang/go#52815
---
M src/cmd/go/internal/work/exec.go
M src/cmd/go/internal/work/gccgo.go
2 files changed, 114 insertions(+), 18 deletions(-)
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob, Motiejus Jakštys.
Gerrit Bot uploaded patch set #5 to this change.
GitHub-Last-Rev: 55004bc2ce52ed888e8cfa45aa2f494f4718dc0a
GitHub-Pull-Request: golang/go#52815
---
M src/cmd/go/internal/work/exec.go
M src/cmd/go/internal/work/gccgo.go
2 files changed, 114 insertions(+), 18 deletions(-)
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Michael Matloob.
6 comments:
Commit Message:
Patch Set #2, Line 9:
cmd/cgo: use -Wl,--no-gc-sections if available
This line duplicates the topic line. […]
Done
Patch Set #2, Line 11:
This change adds `--no-gc-sections` to the external linker if the flagI think we can remove this sentence also.
Done
Patchset:
A stack of commits in a GitHub pull request will be squashed when it is imported into Gerrit. […]
Thanks, will squash.
Patchset:
Thank you for your findings.
- Squashed
- Updated the description in both github and commit.
- Fixed the `link` param to `gccSupportsFlag`.
File src/cmd/go/internal/work/exec.go:
Patch Set #2, Line 2584:
panic(fmt.Sprintf("use gccSupportsLinkerFlag for %q", flag))base.Fatalf would be better than panic here. Similarly below.
Done
Patch Set #2, Line 2631:
cmdArgs = append(cmdArgs, "-c")I'm not following this. We already pass -c. Passing -c is not going to cause a link.
No, I am the one not following. Thanks.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob.
1 comment:
File src/cmd/go/internal/work/exec.go:
Patch Set #5, Line 2592:
if !strings.HasPrefix(flag, "-Wl,") {Sorry for just noticing this, but with this implementation any option that starts with -Wl must use supportsCompilerFlag and any option that doesn't start with -Wl must use supportsLinkerFlag. It will be simpler to just let gccSupportsFlag decide whether to invoke the linker by examining flag, rather than adding two new functions.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob, Motiejus Jakštys.
1 comment:
Patchset:
Thank you for your findings. […]
Ack
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Michael Matloob.
2 comments:
Patchset:
Good call, updated. Simplifies the diff too.
File src/cmd/go/internal/work/exec.go:
Patch Set #5, Line 2592:
if !strings.HasPrefix(flag, "-Wl,") {
Sorry for just noticing this, but with this implementation any option that starts with -Wl must use […]
Done
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Michael Matloob.
Gerrit Bot uploaded patch set #6 to this change.
GitHub-Last-Rev: 58406b36cabec694003b2c50533220410853e295
GitHub-Pull-Request: golang/go#52815
---
M src/cmd/go/internal/work/exec.go
1 file changed, 87 insertions(+), 7 deletions(-)
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob.
Patch set 6:Run-TryBot +1
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Michael Matloob.
Patch set 6:Run-TryBot +1Auto-Submit +1Code-Review +2
1 comment:
Patchset:
Thanks.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gopher Robot (Gerrit)
Gopher Robot submitted this change.
Reviewed-on: https://go-review.googlesource.com/c/go/+/405414
Reviewed-by: Ian Lance Taylor <ia...@google.com>
Auto-Submit: Ian Lance Taylor <ia...@google.com>
Run-TryBot: Ian Lance Taylor <ia...@golang.org>
Run-TryBot: Ian Lance Taylor <ia...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/cmd/go/internal/work/exec.go
1 file changed, 94 insertions(+), 7 deletions(-)
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 15b9e1e..7a45a25 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -2528,6 +2528,13 @@
a = append(a, "-Qunused-arguments")
}
+ // zig cc passes --gc-sections to the underlying linker, which then causes
+ // undefined symbol errors when compiling with cgo but without C code.
+ // https://github.com/golang/go/issues/52690
+ if b.gccSupportsFlag(compiler, "-Wl,--no-gc-sections") {
+ a = append(a, "-Wl,--no-gc-sections")
+ }
+
// disable word wrapping in error messages
a = append(a, "-fmessage-length=0")
@@ -2594,13 +2601,21 @@
defer os.Remove(tmp)
}
- // We used to write an empty C file, but that gets complicated with
- // go build -n. We tried using a file that does not exist, but that
- // fails on systems with GCC version 4.2.1; that is the last GPLv2
- // version of GCC, so some systems have frozen on it.
- // Now we pass an empty file on stdin, which should work at least for
- // GCC and clang.
- cmdArgs := str.StringList(compiler, flag, "-c", "-x", "c", "-", "-o", tmp)
+ // We used to write an empty C file, but that gets complicated with go
+ // build -n. We tried using a file that does not exist, but that fails on
+ // systems with GCC version 4.2.1; that is the last GPLv2 version of GCC,
+ // so some systems have frozen on it. Now we pass an empty file on stdin,
+ // which should work at least for GCC and clang.
+ //
+ // If the argument is "-Wl,", then it's testing the linker. In that case,
+ // skip "-c". If it's not "-Wl,", then we are testing the compiler and
+ // can emit the linking step with "-c".
+ cmdArgs := str.StringList(compiler, flag)
+ if !strings.HasPrefix(flag, "-Wl,") /* linker flag */ {
+ cmdArgs = append(cmdArgs, "-c")
+ }
+ cmdArgs = append(cmdArgs, "-x", "c", "-", "-o", tmp)
+
if cfg.BuildN || cfg.BuildX {
b.Showcmd(b.WorkDir, "%s || true", joinUnambiguously(cmdArgs))
if cfg.BuildN {
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Cherry Mui (Gerrit)
2 comments:
Patchset:
This CL breaks the AIX builder and the iOS builder.
File src/cmd/go/internal/work/exec.go:
Patch Set #7, Line 2636:
!bytes.Contains(out, []byte("is not supported"))Maybe we also need to match "not recognized" and "unknown option".
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Ian Lance Taylor has created a revert of this change.
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
This is a continuation of CL 405414: the original one broke AIX and iOS
builds. To fix that, added `unknown option` to the list of strings
under lookup.
Fixes #52690
Change-Id: Id6743e1e759a02627b0fc6d2ac89bb69b706d04c
GitHub-Last-Rev: b9ef8009dcb4790e541e4a752632d2bd992f8f42
GitHub-Pull-Request: golang/go#53028
---
M src/cmd/go/internal/work/exec.go
1 file changed, 96 insertions(+), 8 deletions(-)
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 15b9e1e..1401663 100644
@@ -2613,12 +2628,16 @@
out, _ := cmd.CombinedOutput()
// GCC says "unrecognized command line option".
// clang says "unknown argument".
+ // tcc says "unsupported"
+ // AIX and/or iOS say "not recognized"
// Older versions of GCC say "unrecognised debug output level".
// For -fsplit-stack GCC says "'-fsplit-stack' is not supported".
supported := !bytes.Contains(out, []byte("unrecognized")) &&
!bytes.Contains(out, []byte("unknown")) &&
!bytes.Contains(out, []byte("unrecognised")) &&
- !bytes.Contains(out, []byte("is not supported"))
+ !bytes.Contains(out, []byte("is not supported")) &&
+ !bytes.Contains(out, []byte("not recognized")) &&
+ !bytes.Contains(out, []byte("unsupported"))
b.flagCache[key] = supported
return supported
}
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
2 comments:
Patchset:
See CL 407814
File src/cmd/go/internal/work/exec.go:
Patch Set #7, Line 2636:
!bytes.Contains(out, []byte("is not supported"))Maybe we also need to match "not recognized" and "unknown option".
Done. Opened https://github.com/golang/go/pull/53028
To view, visit change 405414. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Bryan Mills, Cherry Mui, Ian Lance Taylor.
1 comment:
Patchset:
Adding original reviewers from CL 405414.
Cherry Mui, I would appreciate if you could double-check this with iOS and AIX.
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Cherry Mui, Ian Lance Taylor.
Patch set 1:Run-TryBot +1Code-Review +1
1 comment:
Patchset:
TRY=aix,ios
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Cherry Mui (Gerrit)
Attention is currently required from: Ian Lance Taylor.
1 comment:
Patchset:
Build is still in progress... Status page: https://farmer.golang.org/try?commit=33bad808 […]
Apparently it is still failing on iOS. The error messages is "ld: unknown option". It seems the code already checks for "unknown". Not sure what is still missing...
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Cherry Mui, Ian Lance Taylor.
1 comment:
Patchset:
1 of 32 SlowBots failed. […]
On the iOS builder the command "$CC -Wl,--no-gc-sections -x c - -o /dev/null < /dev/null" is failing with
Unable to remove existing file: Invalid argument
clang-5.0: error: linker command failed with exit code 255 (use -v to see invocation)Perhaps it will work to add "ios" to the "windows" case in gccSupportsFlag to use -o with a temporary file.
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Cherry Mui, Ian Lance Taylor.
Gerrit Bot uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Bryan Mills, TryBot-Result-1 by Gopher Robot
GitHub-Last-Rev: 9e1d5b296992a9ca435403565cf4f7f6511e2b9f
GitHub-Pull-Request: golang/go#53028
---
M src/cmd/go/internal/work/exec.go
1 file changed, 102 insertions(+), 9 deletions(-)
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Bryan Mills, Cherry Mui, Ian Lance Taylor, Ian Lance Taylor.
2 comments:
Patchset:
On the iOS builder the command "$CC -Wl,--no-gc-sections -x c - -o /dev/null < /dev/null" is failing […]
Done
Thanks for the suggestion, applied.
Unfortunately, I don't have access to an iOS build(device|farm), thus flying blind from here.
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Cherry Mui, Ian Lance Taylor, Ian Lance Taylor.
Patch set 2:Run-TryBot +1Code-Review +1
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Cherry Mui (Gerrit)
Attention is currently required from: Ian Lance Taylor, Ian Lance Taylor.
Patch set 2:Code-Review +2
1 comment:
File src/cmd/go/internal/work/exec.go:
iOS says "unknown option" when the output file is specified. I think we can update this comment (just removing iOS is fine).
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Motiejus Jakštys (Gerrit)
Attention is currently required from: Ian Lance Taylor, Ian Lance Taylor.
1 comment:
File src/cmd/go/internal/work/exec.go:
iOS says "unknown option" when the output file is specified. […]
Done
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor, Ian Lance Taylor.
Gerrit Bot uploaded patch set #3 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Bryan Mills, TryBot-Result+1 by Gopher Robot
GitHub-Last-Rev: 86f227a14e9f326f1b461b641e4865bc4dc70780
GitHub-Pull-Request: golang/go#53028
---
M src/cmd/go/internal/work/exec.go
1 file changed, 102 insertions(+), 9 deletions(-)
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor.
Patch set 3:Run-TryBot +1Auto-Submit +1Code-Review +1
1 comment:
Patchset:
Thanks.
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Attention is currently required from: Bryan Mills, Ian Lance Taylor.
1 comment:
Patchset:
1 of 33 SlowBots failed. […]
Trybot failure is a known problem unrelated to this CL.
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Ian Lance Taylor submitted this change.
2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/cmd/go/internal/work/exec.go
Insertions: 1, Deletions: 1.
@@ -2634,7 +2634,7 @@
// GCC says "unrecognized command line option".
// clang says "unknown argument".
// tcc says "unsupported"
- // AIX and/or iOS say "not recognized"
+ // AIX says "not recognized"
// Older versions of GCC say "unrecognised debug output level".
// For -fsplit-stack GCC says "'-fsplit-stack' is not supported".
supported := !bytes.Contains(out, []byte("unrecognized")) &&
```
Reviewed-on: https://go-review.googlesource.com/c/go/+/407814
Reviewed-by: Cherry Mui <cher...@google.com>
Auto-Submit: Ian Lance Taylor <ia...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
Reviewed-by: Ian Lance Taylor <ia...@google.com>
Run-TryBot: Ian Lance Taylor <ia...@google.com>
---
M src/cmd/go/internal/work/exec.go
1 file changed, 108 insertions(+), 9 deletions(-)
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index 15b9e1e..2becc6d 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -2528,6 +2528,13 @@
a = append(a, "-Qunused-arguments")
}
+ // zig cc passes --gc-sections to the underlying linker, which then causes
+ // undefined symbol errors when compiling with cgo but without C code.
+ // https://github.com/golang/go/issues/52690
+ if b.gccSupportsFlag(compiler, "-Wl,--no-gc-sections") {
+ a = append(a, "-Wl,--no-gc-sections")
+ }
+
// disable word wrapping in error messages
a = append(a, "-fmessage-length=0")
@@ -2584,7 +2591,12 @@
}
tmp := os.DevNull
- if runtime.GOOS == "windows" {
+
+ // On the iOS builder the command
+ // $CC -Wl,--no-gc-sections -x c - -o /dev/null < /dev/null
+ // is failing with:
+ // Unable to remove existing file: Invalid argument
+ if runtime.GOOS == "windows" || runtime.GOOS == "ios" {
f, err := os.CreateTemp(b.WorkDir, "")
if err != nil {
return false
@@ -2594,13 +2606,21 @@
@@ -2613,12 +2633,16 @@
out, _ := cmd.CombinedOutput()
// GCC says "unrecognized command line option".
// clang says "unknown argument".
+ // tcc says "unsupported"
+ // AIX says "not recognized"
// Older versions of GCC say "unrecognised debug output level".
// For -fsplit-stack GCC says "'-fsplit-stack' is not supported".
supported := !bytes.Contains(out, []byte("unrecognized")) &&
!bytes.Contains(out, []byte("unknown")) &&
!bytes.Contains(out, []byte("unrecognised")) &&
- !bytes.Contains(out, []byte("is not supported"))
+ !bytes.Contains(out, []byte("is not supported")) &&
+ !bytes.Contains(out, []byte("not recognized")) &&
+ !bytes.Contains(out, []byte("unsupported"))
b.flagCache[key] = supported
return supported
}
To view, visit change 407814. To unsubscribe, or for help writing mail filters, visit settings.