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

math/bits: divideError is not a function; //go:linkname divideError runtime.divideError #30771

Closed
apaprocki opened this issue Mar 12, 2019 · 11 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@apaprocki
Copy link

apaprocki commented Mar 12, 2019

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

Compiling 1.12.0 release source code using GCC 8.2 gccgo

Does this issue reproduce with the latest release?

Yes

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

linux amd64

What did you do?

Attempting to build the 1.12 release using GCC 8.2 gccgo fails with:

GOOS=linux GOARCH=amd64 \
GOROOT=/tmp/go-1.12.0 \
GOROOT_FINAL=/opt/go-1.12 \
GOROOT_BOOTSTRAP=/opt/go \
GCCGO=gccgo \
./make.bash
Building Go cmd/dist using /opt/go.
Building Go toolchain1 using /opt/go.
# bootstrap/math/bits
/tmp/go-1.12.0/src/math/bits/bits.go:534:3: error: divideError is not a function; //go:linkname is only supported for functions
//go:linkname divideError runtime.divideError
^
/tmp/go-1.12.0/src/math/bits/bits.go:531:3: error: overflowError is not a function; //go:linkname is only supported for functions
//go:linkname overflowError runtime.overflowError
^
go tool dist: FAILED: /opt/go/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

What did you expect to see?

Successful build

What did you see instead?

Failed build

@mvdan
Copy link
Member

mvdan commented Mar 12, 2019

/cc @ianlancetaylor; I presume this needs some sort of fix to be backported into 1.12.x.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Mar 12, 2019

In general I don't think we want to guarantee that the standard library can be built with gccgo. Certainly the runtime package cannot. While we should avoid introducing gratuitous differences it's hard to have no differences at all.

In this case the gccgo copy of math/bits is patched to avoid the problem.

In general we should probably fix gccgo to permit using go:linkname for a variable. While technically it cannot be fully supported, I think that in practice we can treat it as always creating a reference to an externally defined variable.

@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 12, 2019
@spackard
Copy link

spackard commented Mar 12, 2019

If you can point us to how to patch either gccgo or go, we'd be happy to apply the patch to our local build and report back. I am curious how the prebuilt packages are built and how it's different from how we're trying to build go, also.

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Mar 12, 2019

I'm sorry, I'm not quite sure what you are asking.

gccgo comes with its own copy of the Go standard library. It can be found in the gccgo sources in the libgo subdirectory. That copy of the standard library is built using gccgo during the normal gccgo build and install. There are docs on how to do that at https://golang.org/doc/install/gccgo.

@cherrymui
Copy link
Member

cherrymui commented Mar 13, 2019

I think it could be useful that the gc toolchain can be bootstrapped with gccgo. Since this is in bootstrap code, maybe we want to consider to work around it. Otherwise people who want to bootstrap gc from gccgo would have to bootstrap an older version of gc then bootstrap gc tip from the older gc.

@apaprocki
Copy link
Author

apaprocki commented Mar 13, 2019

@cherrymui summarizes our situation -- we would like to avoid having to drag along older versions of gc just to bootstrap the latest release tarball using gccgo.

@gopherbot
Copy link

gopherbot commented Apr 4, 2019

Change https://golang.org/cl/170737 mentions this issue: math/bits: add gccgo-friendly code for compiler bootstrap

gopherbot pushed a commit that referenced this issue Apr 4, 2019
When building as part of the bootstrap process, avoid
use of "go:linkname" applied to variables, since this
feature is ill-defined/unsupported for gccgo.

Updates #30771.

Change-Id: Id44d01b5c98d292702e5075674117518cb59e2d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170737
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@thanm
Copy link
Contributor

thanm commented Apr 4, 2019

@apaprocki This should be fixed now on tip (bootstrap with gccgo now works for me). Please reopen if you run into other issues.

@thanm thanm closed this as completed Apr 4, 2019
@spackard
Copy link

spackard commented Apr 4, 2019

Thanks!

@greencoloured
Copy link

greencoloured commented Jul 31, 2019

fails on 1.12.7 with gcc-9.1.0 compiled with the gcc-go.

I guess the changes must be after this release?

Building Go toolchain1 using /opt/test/gcc-go.
# bootstrap/math/bits
/home/michael/compile_by_hand/go/src/math/bits/bits.go:534:3: error: divideError is not a function; //go:linkname is only supported for functions
  534 | //go:linkname divideError runtime.divideError
      |   ^
/home/michael/compile_by_hand/go/src/math/bits/bits.go:531:3: error: overflowError is not a function; //go:linkname is only supported for functions
  531 | //go:linkname overflowError runtime.overflowError
      |   ^
go tool dist: FAILED: /opt/test/gcc-go/bin/go install -gcflags=-l -tags=math_big_pure_go compiler_bootstrap bootstrap/cmd/...: exit status 2

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Jul 31, 2019

Correct. The changes to the math/bits package will be in the upcoming 1.13 release.

@golang golang locked and limited conversation to collaborators Jul 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

9 participants