Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/cgo: internal linker's implicit dependency on gcc_s #59735

Open
motiejus opened this issue Apr 20, 2023 · 7 comments
Open

cmd/cgo: internal linker's implicit dependency on gcc_s #59735

motiejus opened this issue Apr 20, 2023 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@motiejus
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version devel go1.21-9d35ebba06 Thu Apr 20 01:07:29 2023 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/motiejus/.cache/go-build"
GOENV="/home/motiejus/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/motiejus/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/motiejus/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1290842097=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Download zig nightly from ziglang.org/download
  2. Run a unit test in Go:
$ cd misc/cgo/test
$ CC="zig cc" ../../../bin/go test -buildonly -tags=internal -ldflags=-linkmode=internal -count=1 -short .
# misc/cgo/test.test
misc/cgo/test(.text): relocation target memcpy not defined
misc/cgo/test(.text): relocation target memset not defined
misc/cgo/test(.text): relocation target memcmp not defined
misc/cgo/test(.text): relocation target memmove not defined
FAIL    misc/cgo/test [build failed]

Analysis

The resulting linker line when executed with zig cc is this:

    ld.lld-16 --error-limit=0 -O3 -z stack-size=16777216 --eh-frame-hdr -znow -m elf_x86_64 -o /tmp/go-build3231974750/b087/_cgo_.o /home/motiejus/.cache/zig/o/a4418d9504bdd855afcbea1bc94ebae8/Scrt1.o /home/motiejus/.cache/zig/o/e065a1b82ed8b24fe59592b3eb8fc65d/crti.o -dynamic-linker /lib64/ld-linux-x86-64.so.2 /tmp/go-build3231974750/b087/_cgo_main.o /tmp/go-build3231974750/b087/_x001.o /tmp/go-build3231974750/b087/_x002.o /tmp/go-build3231974750/b087/_x003.o /tmp/go-build3231974750/b087/_x004.o /tmp/go-build3231974750/b087/_x005.o /tmp/go-build3231974750/b087/_x006.o /tmp/go-build3231974750/b087/_x007.o /tmp/go-build3231974750/b087/_x008.o /tmp/go-build3231974750/b087/_x009.o /tmp/go-build3231974750/b087/_x010.o /tmp/go-build3231974750/b087/_x011.o /tmp/go-build3231974750/b087/_x012.o /tmp/go-build3231974750/b087/_x013.o /tmp/go-build3231974750/b087/_x014.o /tmp/go-build3231974750/b087/_x015.o /tmp/go-build3231974750/b087/_x016.o /tmp/go-build3231974750/b087/_x017.o /tmp/go-build3231974750/b087/_x018.o /tmp/go-build3231974750/b087/_x019.o /tmp/go-build3231974750/b087/_x020.o /tmp/go-build3231974750/b087/_x021.o /tmp/go-build3231974750/b087/_x022.o /tmp/go-build3231974750/b087/_x023.o /tmp/go-build3231974750/b087/_x024.o /tmp/go-build3231974750/b087/_x025.o /tmp/go-build3231974750/b087/_x026.o /tmp/go-build3231974750/b087/_x027.o /tmp/go-build3231974750/b087/_x028.o /tmp/go-build3231974750/b087/_x029.o /tmp/go-build3231974750/b087/_x030.o /tmp/go-build3231974750/b087/_x031.o /tmp/go-build3231974750/b087/_x032.o /tmp/go-build3231974750/b087/_x033.o /tmp/go-build3231974750/b087/_x034.o /tmp/go-build3231974750/b087/_x035.o /tmp/go-build3231974750/b087/_x036.o /tmp/go-build3231974750/b087/_x037.o /tmp/go-build3231974750/b087/_x038.o --as-needed /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libm.so.6 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libpthread.so.0 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libc.so.6 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libdl.so.2 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/librt.so.1 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libld.so.2 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libutil.so.1 /home/motiejus/.cache/zig/o/049461b7b599a65b7c71125d31219c66/libresolv.so.2 /home/motiejus/.cache/zig/o/ba89224c6a25253f56c3c4cf069c9b95/libc_nonshared.a /home/motiejus/.cache/zig/o/48d2e0a1bf92f705efe7e7b583e98d69/libcompiler_rt.a /home/motiejus/.cache/zig/o/9984432c78523964c83d28b8c5dbd85a/crtn.o --allow-shlib-undefined

Looking at _cgo_.o:

$ readelf -Ws /tmp/go-build3231974750/b087/_cgo_.o | grep -w memset
    52: 0000000000224780   206 FUNC    WEAK   DEFAULT   14 memset
   366: 0000000000224780   206 FUNC    WEAK   DEFAULT   14 memset

Comparing to _cgo_.o produced by CC=clang-16 (omitting the linker line for brevity):

$ readelf -Ws /tmp/go-build1532797969/b087/_cgo_.o | grep -w memset
    19: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND memset@GLIBC_2.2.5 (2)
   263: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND memset@GLIBC_2.2.5

As we can see, the Zig internal linker has a problem linking this symbol when a specific DSO version is not requested.

Why does it work with clang/gcc?

In both cases (plain clang-16 and zig cc) nothing in the Go compiler line necessitates a versioned memset. The only symbol that is requesting memset is _x017.o:

$ readelf -Ws /tmp/go-build1532797969/b087/_x017.o | grep -w memset
   200: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND memset

However, in the working case, the -lgcc_s is also in the linker line. Which is asking for a versioned memset:

$ readelf -Ws /usr/lib/x86_64-linux-gnu/libgcc_s.so.1 | grep -w memset
     7: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND memset@GLIBC_2.2.5 (15)

... which results _cgo_.o to have a versioned relocation (is that the correct term?), which then Go internal linker is able to process.

Summary

  1. CGo application uses memset.
  2. Go linker gets a memset@GLIBC_2.2.5 because of a transitive dependency to -lgcc_s.
  3. If _cgo_.o does not request a versioned memset, it is not able to link the file, resulting in relocation target memset not defined.

Questions:

  1. Can we make Go internal linker understand the non-versioned stubs that are in glibc?
  2. Is it a good idea to use an internal linker when CGo is involved at all?
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 20, 2023
@prattmic
Copy link
Member

cc @golang/compiler

@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 21, 2023
@prattmic prattmic added this to the Backlog milestone Apr 21, 2023
@cherrymui
Copy link
Member

Can we make Go internal linker understand the non-versioned stubs that are in glibc?

I think the internal linker knows about libgcc but it doesn't know about the existence of glibc (for one, one may use other libc implementations). It only sees glibc if there is a dynamic reference somewhere. Otherwise I think it doesn't even know to link against glibc, so the symbol remains undefined.

Is it a good idea to use an internal linker when CGo is involved at all?

The short answer is no. The main purpose of internal linking is to support linking standard library packages that uses cgo (e.g. the net package, the race detector). It is already not the default link mode if any other cgo code is involved. We keep the support of the internal linking mode (probably a little beyond the standard library packages) but we don't put a lot effort in supporting C linking in general in the Go linker. I'd recommend using external linking mode when cgo is involved.

Thanks.

@cherrymui cherrymui modified the milestones: Backlog, Unplanned Apr 21, 2023
@ianlancetaylor
Copy link
Contributor

Is there a particular use case for internal linking that ought to be supported?

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 21, 2023
@motiejus
Copy link
Contributor Author

Is there a particular use case for internal linking that ought to be supported?

I am working on #59666: this is one of the two known unit test failures when running Go unit tests with CC="zig cc". Here is a higher level command to repro this:

$ cd src; CC="zig cc" ../bin/go tool dist test   -v -run=^cgo:test-internal$
##### Test execution environment.
# GOARCH: amd64
# CPU: Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz
# GOOS: linux
# OS Version: Linux 5.19.0-35-generic #36~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Feb 17 15:17:25 UTC 2 x86_64

##### ../misc/cgo/test
# go tool dist test -run=^cgo:test-internal$
# misc/cgo/test.test
misc/cgo/test(.text): relocation target memcpy not defined
misc/cgo/test(.text): relocation target memset not defined
misc/cgo/test(.text): relocation target memcmp not defined
misc/cgo/test(.text): relocation target memmove not defined
FAIL    misc/cgo/test [build failed]
FAIL
2023/04/22 00:13:39 Failed: exit status 1
go tool dist: FAILED

The short answer is no. The main purpose of internal linking is to support linking standard library packages that uses cgo (e.g. the net package, the race detector). It is already not the default link mode if any other cgo code is involved. We keep the support of the internal linking mode (probably a little beyond the standard library packages) but we don't put a lot effort in supporting C linking in general in the Go linker. I'd recommend using external linking mode when cgo is involved.

The unit test compiles the following:

https://github.com/golang/go/blob/go1.20.3/misc/cgo/test/test.go#L1506-L1528

with -linkmode=internal. Is It OK to remove this specific test which compiles CGo with internal linkmode?

My goal, again, is to make all tests with CC="zig cc" pass, with fixes to Go and/or Zig.

@ianlancetaylor
Copy link
Contributor

It's fine to skip testing misc/cgo/test with -linkmode=internal in cases where it doesn't currently work. Offhand I'm not sure of the best way to detect the use of zig cc in cmd/dist, but hopefully there is a good way.

@ianlancetaylor ianlancetaylor removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 21, 2023
@motiejus
Copy link
Contributor Author

Thanks for your response, I will see if I can remove/disable the unit test for test6506 with -linkmode=internal.

@motiejus
Copy link
Contributor Author

motiejus commented Apr 26, 2023 •
edited

It's fine to skip testing misc/cgo/test with -linkmode=internal in cases where it doesn't currently work. Offhand I'm not sure of the best way to detect the use of zig cc in cmd/dist, but hopefully there is a good way.

There is a precedent with Alpine:

go/src/cmd/dist/test.go

Lines 826 to 828 in 7c47a6b

// Stub out following test on alpine until 54354 resolved.
builderName := os.Getenv("GO_BUILDER_NAME")
disablePIE := strings.HasSuffix(builderName, "-alpine")

I can do the same with zig-cc if #59666 gets accepted.

@motiejus motiejus mentioned this issue Jun 20, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@motiejus @prattmic @ianlancetaylor @gopherbot @cherrymui