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

cmd/go: when building gccgo, GOPKGPATH encoding does not match pkgpath encoding #37272

Closed
paleozogt opened this issue Feb 17, 2020 · 6 comments
Closed
Assignees
Milestone

Comments

@paleozogt
Copy link

@paleozogt paleozogt commented Feb 17, 2020

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

$ go version
go version go1.12.2 gccgo (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 linux/amd64

Does this issue reproduce with the latest release?

I'm using gccgo-9, which I think is the latest release.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/apollo/.gocache"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/apollo/Development/weird-gopath"
GOPROXY=""
GORACE=""
GOROOT="/usr"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/gcc/x86_64-linux-gnu/9"
GCCGO="/usr/bin/x86_64-linux-gnu-gccgo-9"
CC="x86_64-linux-gnu-gcc-9"
CXX="x86_64-linux-gnu-g++-9"
CGO_ENABLED="1"
GOMOD=""
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-build492198966=/tmp/go-build -gno-record-gcc-switches -funwind-tables"

What did you do?

Code: https://play.golang.org/p/CwSEukNuiAg

// main.go

package main

import "fmt"
import "golang.org/x/tools/container/intsets"

func main() {
	var foo intsets.Sparse
	fmt.Println(foo)
}

Building with regular go works:

> go version
go version go1.12.2 linux/amd64
> CGO_ENABLED=1 go build -v -i -o foo
> ./foo
{{0 [0 0 0 0] <nil> <nil>}}

while building with gccgo fails:

> go version
go version go1.12.2 gccgo (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 linux/amd64
> CGO_ENABLED=1 go build -v -i -o foo
github.com/paleozogt/golangtoolstest
# github.com/paleozogt/golangtoolstest
/usr/bin/ld: /home/apollo/Development/weird-gopath/pkg/gccgo_linux_amd64/golang.org/x/tools/container/libintsets.a(_go_.o): in function `golang.org..z2fx..z2ftools..z2fcontainer..z2fintsets.block.len':
/home/apollo/Development/weird-gopath/src/golang.org/x/tools/container/intsets/sparse.go:137: undefined reference to `golang.org..z2fx..z2ftools..z2fcontainer..z2fintsets.popcount'
/usr/bin/ld: /home/apollo/Development/weird-gopath/pkg/gccgo_linux_amd64/golang.org/x/tools/container/libintsets.a(_go_.o): in function `intsets.nlz':
/home/apollo/Development/weird-gopath/src/golang.org/x/tools/container/intsets/util.go:51: undefined reference to `golang.org..z2fx..z2ftools..z2fcontainer..z2fintsets.popcount'
collect2: error: ld returned 1 exit status

What did you expect to see?

Build success.

What did you see instead?

Build failure.

@gopherbot gopherbot modified the milestones: Unreleased, Gccgo Feb 17, 2020
@gopherbot gopherbot added the Tools label Feb 17, 2020
@gopherbot gopherbot modified the milestones: Gccgo, Unreleased Feb 17, 2020
@paleozogt
Copy link
Author

@paleozogt paleozogt commented Feb 17, 2020

What's odd is that there's gccgo code in golang.org/x/tools/container/intsets, but it doesn't seem to be getting hooked up.

Running nm shows this

> nm pkg/gccgo_linux_amd64/golang.org/x/tools/container/libintsets.a | grep popcount
                 U golang.org..z2fx..z2ftools..z2fcontainer..z2fintsets.popcount
0000000000000000 T golang_org_x_tools_container_intsets.popcount
                 U __popcountdi2

so the function definition is there.

That naming is coming from an ASM label in popcnt_gccgo_c.c:

extern intptr_t popcount(uintptr_t x) __asm__(GOSYM_PREFIX GOPKGPATH ".popcount");

It appears that gccgo isn't following this naming convention? I'm not sure where this sort of thing is documented.

At any rate, the problem can be fixed by commenting out the ASM labelling and switching to //extern in popcnt_gccgo.go:

//extern popcount
func popcount(x word) int
@ianlancetaylor ianlancetaylor changed the title x/tools/container/intsets doesn't build with gccgo cmd/go: when building gccgo, GOPKGPATH encoding does not match pkgpath encoding Feb 18, 2020
@ianlancetaylor ianlancetaylor added NeedsFix and removed Tools labels Feb 18, 2020
@ianlancetaylor ianlancetaylor modified the milestones: Gccgo, Go1.15 Feb 18, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 18, 2020

It's a bug in the go tool. The GOPKGPATH macro is supposed to be defined to the encoding of the pkgpath. But when we changed the pkgpath encoding in https://golang.org/cl/135455 and https://golang.org/cl/200838 we didn't update the go tool accordingly.

CC @thanm

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 18, 2020

Change https://golang.org/cl/219817 mentions this issue: cmd/go: update -DGOPKGPATH to use current pkgpath encoding

gopherbot pushed a commit to golang/gofrontend that referenced this issue Feb 18, 2020
This will need to be done in the gc version too, probably more cleverly.
This version will ensure that the next GCC release works correctly
when using the GCC version of the go tool.

Updates golang/go#37272

Change-Id: I5636bec7268bcd41994c2e2262f8293cb95c0cd4
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/219817
Reviewed-by: Than McIntosh <thanm@google.com>
kraj pushed a commit to kraj/gcc that referenced this issue Feb 18, 2020
This will need to be done in the gc version too, probably more cleverly.
This version will ensure that the next GCC release works correctly
when using the GCC version of the go tool.

Updates golang/go#37272

Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/219817
@ianlancetaylor ianlancetaylor modified the milestones: Go1.15, Backlog Jun 16, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 16, 2020

Moving to backlog milestone. This should still be fixed in the gc version of cmd/go.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 2, 2020

Change https://golang.org/cl/259298 mentions this issue: cmd/cgo: split gofrontend mangling checks into cmd/internal/pkgpath

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 2, 2020

Change https://golang.org/cl/259299 mentions this issue: cmd/go: use cmd/internal/pkgpath for gccgo pkgpath symbol

gopherbot pushed a commit that referenced this issue Oct 5, 2020
This is a step toward porting https://golang.org/cl/219817 from the
gofrontend repo to the main repo.

Note that this also corrects the implementation of the v2 mangling
scheme to use ..u and ..U where appropriate.

For #37272

Change-Id: I64a1e7ca1c84348efcbf1cf62049eeb05c830ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/259298
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
@gopherbot gopherbot closed this in a65bc04 Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.