cmd/cgo: use --no-gc-sections if available #53028

motiejus · 2022-05-23T08:53:22Z

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:

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

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

gopherbot · 2022-05-23T08:57:44Z

This PR (HEAD: b9ef800) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/407814 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2022-05-23T09:05:16Z

Message from Motiejus Jakštys:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-23T14:05:00Z

Message from Bryan Mills:

Patch Set 1: Run-TryBot+1 Code-Review+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-23T14:12:48Z

Message from Gopher Robot:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-23T14:16:47Z

Message from Gopher Robot:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-23T14:20:40Z

Message from Cherry Mui:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-23T14:36:20Z

Message from Gopher Robot:

Patch Set 1: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-24T00:34:38Z

Message from Ian Lance Taylor:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-24T08:04:22Z

This PR (HEAD: 9e1d5b2) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/407814 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2022-05-24T08:12:07Z

Message from Motiejus Jakštys:

Patch Set 1:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-24T13:09:03Z

Message from Bryan Mills:

Patch Set 2: Run-TryBot+1 Code-Review+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-24T13:09:04Z

Message from Gopher Robot:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-24T13:45:28Z

Message from Gopher Robot:

Patch Set 2: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-24T15:48:55Z

Message from Cherry Mui:

Patch Set 2: Code-Review+2

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

@ianlancetaylor

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: 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 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 golang#52690

gopherbot · 2022-05-25T02:55:40Z

This PR (HEAD: 86f227a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/407814 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2022-05-25T02:59:27Z

Message from Motiejus Jakštys:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-25T23:30:23Z

Message from Ian Lance Taylor:

Patch Set 3: Run-TryBot+1 Auto-Submit+1 Code-Review+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-25T23:34:21Z

Message from Gopher Robot:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-25T23:42:05Z

Message from Gopher Robot:

Patch Set 3:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-05-26T00:15:58Z

Message from Gopher Robot:

Patch Set 3: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/407814.
After addressing review feedback, remember to publish your drafts!

@ianlancetaylor

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: 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 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: 86f227a GitHub-Pull-Request: #53028 Reviewed-on: https://go-review.googlesource.com/c/go/+/407814 Reviewed-by: Cherry Mui <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]> Reviewed-by: Bryan Mills <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]>

gopherbot · 2022-05-26T00:53:13Z

This PR is being closed because golang.org/cl/407814 has been merged.

motiejus force-pushed the no-gc-sections-2 branch from b9ef800 to 9e1d5b2 Compare May 24, 2022 08:02

motiejus force-pushed the no-gc-sections-2 branch from 9e1d5b2 to 86f227a Compare May 25, 2022 02:54

gopherbot closed this May 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/cgo: use --no-gc-sections if available #53028

cmd/cgo: use --no-gc-sections if available #53028

motiejus commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 26, 2022

gopherbot commented May 26, 2022

cmd/cgo: use --no-gc-sections if available #53028

cmd/cgo: use --no-gc-sections if available #53028

Conversation

motiejus commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 23, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 24, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 25, 2022

gopherbot commented May 26, 2022

gopherbot commented May 26, 2022