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/go: when building gccgo, GOPKGPATH encoding does not match pkgpath encoding #37272

Closed
paleozogt opened this issue Feb 17, 2020 · 7 comments
Closed

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

paleozogt opened this issue Feb 17, 2020 · 7 comments
Labels
FrozenDueToAge GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@paleozogt
Copy link

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 This label describes issues relating to any tools in the x/tools repository. label Feb 17, 2020
@gopherbot gopherbot modified the milestones: Gccgo, Unreleased Feb 17, 2020
@paleozogt
Copy link
Author

paleozogt commented Feb 17, 2020 •
edited

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 The path to resolution is known, but the work has not been done. and removed Tools This label describes issues relating to any tools in the x/tools repository. labels Feb 18, 2020
@ianlancetaylor ianlancetaylor modified the milestones: Gccgo, Go1.15 Feb 18, 2020
@ianlancetaylor
Copy link
Contributor

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

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
@ianlancetaylor
cmd/go: update -DGOPKGPATH to use current pkgpath encoding
d5d00d3 
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
@ianlancetaylor
cmd/go: update -DGOPKGPATH to use current pkgpath encoding
855b4aa 
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

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

@ianlancetaylor ianlancetaylor self-assigned this Oct 2, 2020
@gopherbot
Copy link

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

@gopherbot
Copy link

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
@ianlancetaylor
cmd/cgo: split gofrontend mangling checks into cmd/internal/pkgpath
72ee5ba 
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
Copy link

Change https://golang.org/cl/270637 mentions this issue: cmd/go, cmd/cgo: update gofrontend mangling checks

gopherbot pushed a commit to golang/gofrontend that referenced this issue Nov 18, 2020
@ianlancetaylor
cmd/go, cmd/cgo: update gofrontend mangling checks
c948c2d 
This is a port of two patches in the master repository.

https://golang.org/cl/259298

    cmd/cgo: split gofrontend mangling checks into cmd/internal/pkgpath

    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.

https://golang.org/cl/259299

    cmd/go: use cmd/internal/pkgpath for gccgo pkgpath symbol

For golang/go#37272
For golang/go#41862

Change-Id: Id276ca1bac7e1d850408a9d222c19e57b26ba001
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/270637
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
@golang golang locked and limited conversation to collaborators Nov 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@paleozogt @ianlancetaylor @gopherbot