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.

Sign up for GitHub

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

gccgo: etcd reportedly doesn't compile on gccgo #28601

Closed
bradfitz opened this issue Nov 5, 2018 · 11 comments
Closed

gccgo: etcd reportedly doesn't compile on gccgo #28601

bradfitz opened this issue Nov 5, 2018 · 11 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Gccgo

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Nov 5, 2018

Pulling this into its own issue from #19074 (comment) where @glaubitz says:

gccgo doesn't build all packages that golang builds. I know it's supposed to be compatible, but it isn't. I can provide a list of Debian packages which don't build.

See, for example, etcd: https://buildd.debian.org/status/fetch.php?pkg=etcd&arch=ppc64&ver=3.2.18%2Bdfsg-1&stamp=1539989343&raw=0

Fails with:

# github.com/coreos/etcd/pkg/cpuutil
src/github.com/coreos/etcd/pkg/cpuutil/endian.go:31:13: error: array bound is not constant
  if v := (*[intWidth]byte)(unsafe.Pointer(&i)); v[0] == 0 {
             ^
src/github.com/coreos/etcd/pkg/cpuutil/endian.go:31:51: error: array index out of bounds
  if v := (*[intWidth]byte)(unsafe.Pointer(&i)); v[0] == 0 {
                                                   ^
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 5, 2018
@gopherbot gopherbot added this to the Gccgo milestone Nov 5, 2018
@bradfitz bradfitz mentioned this issue Nov 5, 2018
Closed
@gopherbot
Copy link

Change https://golang.org/cl/147537 mentions this issue: test: add test that gccgo failed to compile

@gopherbot
Copy link

Change https://golang.org/cl/147442 mentions this issue: compiler: handle abstract type in builtin numeric const value

gopherbot pushed a commit that referenced this issue Nov 5, 2018
@ianlancetaylor
test: add test that gccgo failed to compile
a540aa3 
Updates #28601

Change-Id: I734fc5ded153126d384f0df912ecd4d208005e49
Reviewed-on: https://go-review.googlesource.com/c/147537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
@ianlancetaylor
Copy link
Contributor

This is now fixed on GCC 7 branch, GCC 8 branch, and tip. Sorry for the trouble.

@glaubitz: Please do report cases where the gc toolchain works and gccgo does not. That's the only way we will know to fix the problems. Thanks.

@glaubitz
Copy link

glaubitz commented Nov 5, 2018

This is now fixed on GCC 7 branch, GCC 8 branch, and tip. Sorry for the trouble.

Thanks. That was a very quick fix.

Please do report cases where the gc toolchain works and gccgo does not. That's the only way we will know to fix the problems. Thanks.

This has been on my TODO list forever. I will try to take care of this this week. There are a couple of packages left that have issues.

@glaubitz
Copy link

glaubitz commented Nov 5, 2018 •
edited

@ianlancetaylor There are a couple of packages which actually build but where the testsuite fails:

https://buildd.debian.org/status/fetch.php?pkg=golang-github-ugorji-go-codec&arch=ppc64&ver=1.1.1-1&stamp=1539998621&raw=0
https://buildd.debian.org/status/fetch.php?pkg=golang-github-spf13-cobra&arch=ppc64&ver=0.0.3-1&stamp=1539989839&raw=0
https://buildd.debian.org/status/fetch.php?pkg=golang-golang-x-tools&arch=ppc64&ver=1%3A0.0%7Egit20180501.d3e4ceb5%2Bds-1&stamp=1539989961&raw=0
https://buildd.debian.org/status/fetch.php?pkg=golint&arch=ppc64&ver=0.0%2Bgit20161013.3390df4-2&stamp=1539990009&raw=0
https://buildd.debian.org/status/fetch.php?pkg=govendor&arch=ppc64&ver=1.0.9%2Bds1-1&stamp=1539990039&raw=0
https://buildd.debian.org/status/fetch.php?pkg=hugo&arch=ppc64&ver=0.47.1-2&stamp=1539990488&raw=0

How do we handle these?

kraj pushed a commit to kraj/gcc that referenced this issue Nov 6, 2018
compiler: handle abstract type in builtin numeric const value
ab12ff1 
    
    Builtin_call_expression::do_numeric_constant_value can be called by
    Array_type::verify_length before the determine types pass, so accept
    an abstract type.
    
    Test case is https://golang.org/cl/147537.
    
    Fixes golang/go#28601
    
    Reviewed-on: https://go-review.googlesource.com/c/147442


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-7-branch@265818 138bc75d-0d04-0410-961f-82ee72b054a4
kraj pushed a commit to kraj/gcc that referenced this issue Nov 6, 2018
compiler: handle abstract type in builtin numeric const value
3b986be 
    
    Builtin_call_expression::do_numeric_constant_value can be called by
    Array_type::verify_length before the determine types pass, so accept
    an abstract type.
    
    Test case is https://golang.org/cl/147537.
    
    Fixes golang/go#28601
    
    Reviewed-on: https://go-review.googlesource.com/c/147442


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gcc-8-branch@265819 138bc75d-0d04-0410-961f-82ee72b054a4
kraj pushed a commit to kraj/gcc that referenced this issue Nov 6, 2018
compiler: handle abstract type in builtin numeric const value
7200ac0 
    
    Builtin_call_expression::do_numeric_constant_value can be called by
    Array_type::verify_length before the determine types pass, so accept
    an abstract type.
    
    Test case is https://golang.org/cl/147537.
    
    Fixes golang/go#28601
    
    Reviewed-on: https://go-review.googlesource.com/c/147442


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@265820 138bc75d-0d04-0410-961f-82ee72b054a4
@ianlancetaylor
Copy link
Contributor

@glaubitz Do you know if they fail with gccgo on amd64 also, or also on PPC64? Is there a way that I could find out?

There are of course various possible reasons for this to happen, ranging from a gccgo bug to a non-portable test. I guess we'll just have to examine each one to see what is going on.

@glaubitz
Copy link

glaubitz commented Nov 6, 2018 •
edited

I'll test those packages on amd64. It's actually not that difficult to switch the package over from gc to gccgo.

@glaubitz
Copy link

glaubitz commented Nov 6, 2018

@ianlancetaylor Tested all these packages on amd64 with gccgo-8 instead of gc. They all fail the testsuite with gccgo but pass it with gc.

@bradfitz
Copy link
Contributor Author

bradfitz commented Nov 6, 2018

@glaubitz, feel free to file as many new bugs as you'd like. But reusing this one risks it being ignored, because our dashboard don't track closed issues.

@glaubitz
Copy link

glaubitz commented Nov 6, 2018 via email

@bradfitz
Copy link
Contributor Author

bradfitz commented Nov 6, 2018

I expect around 50-60 packages which won’t build with gccgo out that. We‘ll see.

We look forward to the reports.

When you do report, though, be sure to note exactly what failed: actually building (e.g. compilation failures) vs test failures. The latter is often due to things like tests assuming certain gc-specific behaviors, like naming of symbols in stack frames. In any case, I assume you'll post links to failure logs, so it should be obvious.

@golang golang locked and limited conversation to collaborators Nov 6, 2019
kraj pushed a commit to kraj/gcc that referenced this issue Jan 11, 2020
compiler: handle abstract type in builtin numeric const value
c9559a6 
    
    Builtin_call_expression::do_numeric_constant_value can be called by
    Array_type::verify_length before the determine types pass, so accept
    an abstract type.
    
    Test case is https://golang.org/cl/147537.
    
    Fixes golang/go#28601
    
    Reviewed-on: https://go-review.googlesource.com/c/147442

From-SVN: r265819
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Gccgo
Development

No branches or pull requests
4 participants
@bradfitz @glaubitz @ianlancetaylor @gopherbot