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

gollvm: #cgo LDFLAGS repeated thrice #60287

Closed
andreybokhanko opened this issue May 18, 2023 · 10 comments
Closed

gollvm: #cgo LDFLAGS repeated thrice #60287

andreybokhanko opened this issue May 18, 2023 · 10 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@andreybokhanko
Copy link
Contributor

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

$ go version
go version go1.18 gollvm LLVM 15.0.0git 20230518 (experimental) linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/abokhanko/.cache/go-build"
GOENV="/home/abokhanko/.config/go/env"
GOEXE=""
GOEXPERIMENT="fieldtrack,regabiwrappers"
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/abokhanko/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/abokhanko/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/abokhanko/gotrunk"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/abokhanko/gollvm/tools"
GOVCS=""
GOVERSION="go1.18 gollvm LLVM 15.0.0git 20230518 (experimental)"
GCCGO="/home/abokhanko/gollvm/bin/llvm-goc"
GOAMD64="v1"
AR="ar"
CC="/usr/bin/cc"
CXX="/usr/bin/c++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3136632811=/tmp/go-build -gno-record-gcc-switches -funwind-tables"

What did you do?

$ cat cpart.c
#include 
#include 
extern void HelloFromC() {
        printf("Hello from C!\n");
}

$ cat goapp.go
package main

// #cgo LDFLAGS: ${SRCDIR}/cpart.o
import "C"

//go:linkname HelloFromC HelloFromC
func HelloFromC()

func main() {
        HelloFromC()
}

$ gcc -c cpart.c

$ go build goapp.go
# command-line-arguments
/usr/bin/ld.gold: error: /home/abokhanko/test/issue604/cpart.o: multiple definition of 'HelloFromC'
/usr/bin/ld.gold: /home/abokhanko/test/issue604/cpart.o: previous definition here
/usr/bin/ld.gold: error: /home/abokhanko/test/issue604/cpart.o: multiple definition of 'HelloFromC'
/usr/bin/ld.gold: /home/abokhanko/test/issue604/cpart.o: previous definition here
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 0
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 96
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 106
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 1b5
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 23e

What did you expect to see?

goapp.go building successfully.

What did you see instead?

Errors caused by #cgo LDFLAGS (/home/abokhanko/test/issue604/cpart.o) being repeated thrice in the linker's command line:

$ go build -x goapp.go
WORK=/tmp/go-build2518111976
mkdir -p $WORK/b001/
cat >$WORK/b001/importcfg.link << 'EOF' # internal
packagefile command-line-arguments=/home/abokhanko/.cache/go-build/b0/b04da2f53bf1f5c55adb13031ca2219feee680950bb17d7b87546d3d22b9a7d6-d
modinfo "0w\xaf\f\x92t\b\x02A\xe1\xc1\a\xe6\xd6\x18\xe6path\tcommand-line-arguments\nbuild\t-compiler=gccgo\nbuild\tCGO_ENABLED=1\nbuild\tCGO_CFLAGS=\nbuild\tCGO_CPPFLAGS=\nbuild\tCGO_CXXFLAGS=\nbuild\tCGO_LDFLAGS=\nbuild\tGOARCH=amd64\nbuild\tGOEXPERIMENT=fieldtrack\nbuild\tGOOS=linux\nbuild\tGOAMD64=v1\n\xf92C1\x86\x18 r\x00\x82B\x10A\x16\xd8\xf2"
EOF
mkdir -p $WORK/b001/exe/
cp /home/abokhanko/.cache/go-build/b0/b04da2f53bf1f5c55adb13031ca2219feee680950bb17d7b87546d3d22b9a7d6-d $WORK/b001/_pkg1_.a
cd $WORK/b001/
ar x ./_pkg1_.a _cgo_flags
cd .
ar d $WORK/b001/_pkg1_.a _cgo_flags
/home/abokhanko/gollvm/bin/llvm-goc -o $WORK/b001/exe/a.out "-Wl,-(" -m64 -Wl,--whole-archive $WORK/b001/_pkg1_.a -Wl,--no-whole-archive /home/abokhanko/test/issue604/cpart.o /home/abokhanko/test/issue604/cpart.o /home/abokhanko/test/issue604/cpart.o "-Wl,-)" -Wl,--build-id=0x516d37472d54316a7067466575666b743157326c2f4937504e7a6f6558467645376a4e712d2d4e526c2f643266584a4f734d4a3963464462653866594f572f516d37472d54316a7067466575666b743157326c -Wl,-E
# command-line-arguments
/usr/bin/ld.gold: error: /home/abokhanko/test/issue604/cpart.o: multiple definition of 'HelloFromC'
/usr/bin/ld.gold: /home/abokhanko/test/issue604/cpart.o: previous definition here
/usr/bin/ld.gold: error: /home/abokhanko/test/issue604/cpart.o: multiple definition of 'HelloFromC'
/usr/bin/ld.gold: /home/abokhanko/test/issue604/cpart.o: previous definition here
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 0
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 96
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 106
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 1b5
/usr/bin/ld.gold: error: $WORK/b001/_pkg1_.a(_cgo_defun.o): failed to match split-stack sequence at section 1 offset 23e
@andreybokhanko andreybokhanko changed the title gollvm: #cgp LDFLAGS repeated thrice gollvm: #cgo LDFLAGS repeated thrice May 18, 2023
@gopherbot
Copy link

Change https://go.dev/cl/496195 mentions this issue: cgo: Fix handling of #cgo LDFLAGS

@thanm thanm self-assigned this May 19, 2023
@seankhliao seankhliao added this to the gollvm milestone May 19, 2023
@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label May 19, 2023
@ianlancetaylor
Copy link
Contributor

When you put a .c file in the same directory as the .go files in your package, the go tool will automatically compile the .c file and include it in the link. So there doesn't seem to be any need for the #cgo LDFLAGS line in this example.

That said, if there is only a cpart.o file, and no cpart.c file, then I agree that when using gccgo the LDFLAGS are repeated. But I don't think the fix is to discard duplicates. I think the fix is to only include them once. I think the problem is that go tool both includes LDFLAGS from the Package struct and from cmd/cgo.

I think that something along the lines of https://go.dev/cl/497117 would be a better approach.

@gopherbot
Copy link

Change https://go.dev/cl/497117 mentions this issue: cmd/go: don't collect package CGOLDFLAGS when using gccgo

@andreybokhanko
Copy link
Contributor Author

@ianlancetaylor Giving that you're already working on a more proper fix, do you suggest me to abandon https://go.dev/cl/496195 ?

@ianlancetaylor
Copy link
Contributor

Yes, I don't think we want the approach in 496195. Thanks. If you want to test 497117 that would be helpful. Thanks.

@andreybokhanko
Copy link
Contributor Author

andreybokhanko commented May 24, 2023

Yes, I don't think we want the approach in 496195. Thanks. If you want to test 497117 that would be helpful. Thanks.

Yep, I tested it on my case, and as expected, it works flawlessly.

(sorry, closed by mistake; then reopened)

@andreybokhanko
Copy link
Contributor Author

@ianlancetaylor it seems https://go-review.googlesource.com/c/go/+/497117 is in review limbo... Not sure I can help in any way to push it through?

@ianlancetaylor
Copy link
Contributor

@andreybokhanko I think it mainly needs a test. It hasn't seemed too urgent since there isn't going to be another GCC release for months.

@andreybokhanko
Copy link
Contributor Author

@andreybokhanko I think it mainly needs a test. It hasn't seemed too urgent since there isn't going to be another GCC release for months.

BTW, it's easy enough to write a test using LLVM's lit testing machinery:

// RUN: GOCACHE=%T not go run -x -a %s 2>&1 | FileCheck %s

package main

// #cgo LDFLAGS: ${SRCDIR}/cpart.o
import "C"

//go:linkname HelloFromC HelloFromC
func HelloFromC()

func main() {
HelloFromC()
}

// CHECK: llvm-goc{{.*}}cpart.o
// CHECK-NOT: llvm-goc{{.*}}cpart.o.*cpart.o

Though I guess due to presence of gccgo, this is not really suitable, right?

@gopherbot
Copy link

Change https://go.dev/cl/511675 mentions this issue: cmd/go: don't collect package CGOLDFLAGS when using gccgo

gopherbot pushed a commit to golang/gofrontend that referenced this issue Jul 20, 2023
They are already collected via cmd/cgo.

The gccgo_link_c test is tweaked to do real linking as with this
change the cgo ldflags are not fully reflected in go build -n output,
since they now only come from the built archive.

This is a backport of https://go.dev/cl/497117 from the main repo.

For golang/go#60287

Change-Id: Ie0bf76b588724265d75dd9a96984dd6cc207f84e
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/511675
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
nstester pushed a commit to nstester/gcc that referenced this issue Jul 20, 2023
They are already collected via cmd/cgo.

The gccgo_link_c test is tweaked to do real linking as with this
change the cgo ldflags are not fully reflected in go build -n output,
since they now only come from the built archive.

This is a backport of https://go.dev/cl/497117 from the main repo.

For golang/go#60287

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/511675
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants