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/compile: internal compiler error: no function definition for [0xc42181a5a0] #23701

Closed
tamird opened this issue Feb 5, 2018 · 24 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@tamird
Copy link
Contributor

tamird commented Feb 5, 2018

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

go version go1.10rc1 linux/amd64

Does this issue reproduce with the latest release?

This affects the release candidate.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/tduberstein/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/tduberstein/local/go"
GORACE=""
GOROOT="/home/tduberstein/local/go1.10"
GOTMPDIR=""
GOTOOLDIR="/home/tduberstein/local/go1.10/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build297022879=/tmp/go-build -gno-record-gcc-switches"

What did you do?

go get -d github.com/cockroachdb/cockroach
cd $(go env GOPATH)/src/github.com/cockroachdb/cockroach
make build PKG=./pkg/sql/pgwire

What did you expect to see?

Tests run.

What did you see instead?

# github.com/cockroachdb/cockroach/pkg/sql/pgwire_test
pkg/sql/pgwire/pgwire_test.go:317:45: internal compiler error: no function definition for [0xc424862960] FUNC-method(*pgwire.Server) func() []context.CancelFunc


Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new
FAIL    github.com/cockroachdb/cockroach/pkg/sql/pgwire [build failed]
make: *** [Makefile:773: test] Error 2
@mvdan
Copy link
Member

mvdan commented Feb 5, 2018

I assume this does not affect 1.9?

@mvdan mvdan added this to the Go1.10 milestone Feb 5, 2018
@mvdan mvdan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Feb 5, 2018
@tamird
Copy link
Contributor Author

tamird commented Feb 5, 2018

Correct.

@mvdan
Copy link
Member

mvdan commented Feb 5, 2018

Have you by any chance been able to come up with a small program/package to reproduce the same crash? That project is large and a bit complex to build, so it would help the compiler developers.

@tamird
Copy link
Contributor Author

tamird commented Feb 5, 2018 via email

@dgryski
Copy link
Contributor

dgryski commented Feb 5, 2018

Note that adding methods in a _test.go file is #6204

@tamird
Copy link
Contributor Author

tamird commented Feb 5, 2018

@dgryski that's incorrect - in this case we're not going across packages, and this pattern works in Go 1.9.

@remyoudompheng
Copy link
Contributor

Same issue on my side with the following minimal example:

$ cat a/a.go
package a

import "b"

type Type struct {}

func (*Type) M() b.T { return 0 }
$ cat b/export_test.go
package b

func (*T) Method() *T { return nil }
$ cat b/b.go
package b

type T int

type I interface { M() T }

$ cat b/b_test.go
package b_test

import (
"testing"
"a"
. "b"
)

func TestBroken(*testing.T) {
x := new(T)
x.Method()
_ = new(a.Type)
}

@tamird
Copy link
Contributor Author

tamird commented Feb 5, 2018

a/a.go

package a

import "b"

type Type struct {}

func (*Type) M() b.T { return 0 }

b/export_test.go

package b

func (*T) Method() *T { return nil }

b/b.go

package b

type T int

type I interface { M() T }

b/b_test.go

package b_test

import (
  "testing"
  "a"
  . "b"
)

func TestBroken(*testing.T) {
  x := new(T)
  x.Method()
  _ = new(a.Type)
}

Ah, I guess a second package is required.

@dgryski
Copy link
Contributor

dgryski commented Feb 5, 2018

Bisecting seems to point at c8e9fd5 , which reverts a revert of f0b3626

@ianlancetaylor
Copy link
Contributor

Issue #6204 says that cmd/go is supposed to reject adding methods in a _test.go file, but that it currently does not. And indeed the code in #6204 does compile today. I don't understand how this issue differs from #6204.

@ianlancetaylor
Copy link
Contributor

Oh, never mind, I do get the same error with the test case in #6204 when I use go test p2.

But I still don't see why this is different than #6204.

@dgryski
Copy link
Contributor

dgryski commented Feb 5, 2018

@ianlancetaylor I think the problem is 1.10 breaks an unsupported feature.

@ianlancetaylor
Copy link
Contributor

CC @mdempsky @randall77

@rsc
Copy link
Contributor

rsc commented Feb 5, 2018

I think it's OK to break this in Go 1.10 but we need a better error message.

@tamird
Copy link
Contributor Author

tamird commented Feb 5, 2018

Going to be incovenient for CockroachDB.

$ find . -name helpers_test.go
./pkg/storage/idalloc/helpers_test.go
./pkg/storage/helpers_test.go
./pkg/sql/jobs/helpers_test.go
./pkg/sql/pgwire/helpers_test.go
./pkg/sql/helpers_test.go

/cc @bdarnell @petermattis @spencerkimball

@rsc
Copy link
Contributor

rsc commented Feb 5, 2018

I still think that it's OK to reject methods in tests if we end up needing to do that, but that doesn't look fundamental here. This looks to me like the go command is not rebuilding dependencies of test packages correctly. There was a related bug in this code a few weeks ago, and this report looks like a variant of that. It should be possible to fix this new bug and leave the disposition of #6204 for a less hurried time. I'll take a look at this for 1.10rc2, which we are thinking about doing on Wednesday.

@gopherbot
Copy link

Change https://golang.org/cl/92215 mentions this issue: cmd/go: rebuild as needed for tests of packages that add methods

@rsc
Copy link
Contributor

rsc commented Feb 6, 2018

This was easy to make work after the simplifications done for the build cache: basically a 1/2-line change to cmd/go, simplifying an if condition. Given that we can now rebuild perfectly, I see no reason to go out of our way to disallow adding methods in test files, even though I might wish I had done that 5+ years ago.

@dgryski
Copy link
Contributor

dgryski commented Feb 6, 2018

Should the release notes be updated to mention adding methods in a test file is now "officially" supported?

@ianlancetaylor
Copy link
Contributor

Before we put it in the release notes we need to put it in the docs. Actually I'm not sure what the release notes would say. "This thing that used to work still works." Personally I think this is doc-worthy but not release-notes-worthy.

bboozzoo added a commit to bboozzoo/snapd that referenced this issue Mar 14, 2018
Workaround go vet 'internal compiler error' when methods are defined in test
files and cross package imports are used. The original ticket is:
  golang/go#23701

Although it is supposed to be fixed in 1.10 it does not appear to be so.

Error reported by go vet:

  ./ifacestate_test.go:120:29: internal compiler error: no function definition for [0xc420a2fc80] FUNC-method(*ifacestate.InterfaceManager) func()

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
bboozzoo added a commit to bboozzoo/snapd that referenced this issue Mar 14, 2018
Workaround go vet 'internal compiler error' when methods are defined in test
files and cross package imports are used. The original ticket is:
  golang/go#23701

Although it is supposed to be fixed in 1.10 it does not appear to be so.

Error reported by go vet:

./devicestate_test.go:403:38: internal compiler error: no function definition for [0xc4212363c0] FUNC-method(*devicestate.DeviceManager) func() asserts.KeypairManager

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
@bboozzoo
Copy link

The test case provided in #23701 (comment) can still be reproduced with go1.10 linux/amd64.

With master (go version devel +d32018a500 Wed Mar 14 08:36:15 2018 +0000 linux/amd64) I get this:

# b_test
<unknown line number>: internal compiler error: no function definition for [0xc0003706c0] FUNC-method(*b.T) func() *b.T


goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
        /home/maciek/code/go/go-git/src/runtime/debug/stack.go:24 +0xa7
cmd/compile/internal/gc.Fatalf(0xc50b01, 0x24, 0xc0003bb498, 0x2, 0x2)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/subr.go:182 +0x1f7
cmd/compile/internal/gc.(*hairyVisitor).visit(0xc0003bb650, 0xc000412500, 0xc000074000)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/inl.go:291 +0x7b1
cmd/compile/internal/gc.(*hairyVisitor).visitList(0xc0003bb650, 0xc0004013c0, 0xc000407860)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/inl.go:238 +0x74
cmd/compile/internal/gc.caninl(0xc000001200)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/inl.go:160 +0x186
cmd/compile/internal/gc.Main.func6(0xc00000d7c8, 0x1, 0x1, 0xc000324100)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/main.go:557 +0x164
cmd/compile/internal/gc.(*bottomUpVisitor).visit(0xc0003bb810, 0xc000001200, 0x1)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/esc.go:107 +0x230
cmd/compile/internal/gc.visitBottomUp(0xc00000d7a8, 0x1, 0x1, 0xc5bcb8)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/esc.go:63 +0x95
cmd/compile/internal/gc.Main(0xc5bb98)
        /home/maciek/code/go/go-git/src/cmd/compile/internal/gc/main.go:554 +0x2e05
main.main()
        /home/maciek/code/go/go-git/src/cmd/compile/main.go:49 +0x96

mvo5 pushed a commit to mvo5/snappy that referenced this issue Mar 15, 2018
Workaround go vet 'internal compiler error' when methods are defined in test
files and cross package imports are used. The original ticket is:
  golang/go#23701

Although it is supposed to be fixed in 1.10 it does not appear to be so.

Error reported by go vet:

  ./ifacestate_test.go:120:29: internal compiler error: no function definition for [0xc420a2fc80] FUNC-method(*ifacestate.InterfaceManager) func()

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
mvo5 pushed a commit to mvo5/snappy that referenced this issue Mar 15, 2018
Workaround go vet 'internal compiler error' when methods are defined in test
files and cross package imports are used. The original ticket is:
  golang/go#23701

Although it is supposed to be fixed in 1.10 it does not appear to be so.

Error reported by go vet:

./devicestate_test.go:403:38: internal compiler error: no function definition for [0xc4212363c0] FUNC-method(*devicestate.DeviceManager) func() asserts.KeypairManager

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
@gopherbot
Copy link

Change https://golang.org/cl/104315 mentions this issue: cmd/go: rebuild as needed when vetting test packages

benesch added a commit to benesch/cockroach that referenced this issue Apr 3, 2018
'go tool vet' uses out-of-date/incorrect type information, so, since Go
1.10, the Go team recommends using 'go vet' instead. Take them up on
their recommendation.

This unfortunately trips across an ICE (golang/go#23701), but I opened a
CL to fix that: https://go-review.googlesource.com/c/go/+/104315. For
now, just ignore the ICE.

Release note: None
@rsc rsc modified the milestones: Go1.10, Go1.10.2 Apr 4, 2018
@rsc rsc reopened this Apr 4, 2018
craig bot pushed a commit to cockroachdb/cockroach that referenced this issue Apr 10, 2018
24454: lint: various fixes r=nvanbenschoten a=benesch

Fill in a deep, deep linter rabbit hole. In order for #24104 to pass the linter, we need to run `go vet ./pkg/...` instead of `cd pkg && go tool vet .` so that intentionally-malformatted Go files in a testdata directory are ignored. That required reorganizing the lint rules. Then it required fixing a `go vet` violation that was previously missed, probably because `go tool vet` was using incorrect type information. It also required a workaround for an internal compiler error (golang/go#23701).

@nvanbenschoten, any chance I can toss this your way?

24573: distsql: Refactor aggregator into a state machine r=couchand a=abhimadan

This lays the groundwork for extending the aggregator to stream results
if some of the group columns have an ordering.

Release note: None

----

This is the first commit from #24113, extracted into its own PR.

24577: distsql: improve hashjoin performance r=jordanlewis a=jordanlewis

- add a benchmark for the spilling hashjoiner
- reset the disk iterator rather than creating a new one every probeRow
- fix small flushing bug that caused a flush on an empty batch

```
name                                old time/op    new time/op    delta
HashJoiner/spill=true/rows=256-8      11.7ms ± 3%     2.2ms ± 3%   -81.43%  (p=0.000 n=10+8)
HashJoiner/spill=true/rows=4096-8     89.9ms ± 2%    18.4ms ± 1%   -79.58%  (p=0.000 n=10+9)
HashJoiner/spill=true/rows=65536-8     1.02s ± 2%     0.28s ± 2%   -72.16%  (p=0.000 n=9+10)

name                                old speed      new speed      delta
HashJoiner/spill=true/rows=256-8     175kB/s ± 3%   941kB/s ± 3%  +437.86%  (p=0.000 n=10+8)
HashJoiner/spill=true/rows=4096-8    364kB/s ± 2%  1780kB/s ± 2%  +389.01%  (p=0.000 n=10+10)
HashJoiner/spill=true/rows=65536-8   517kB/s ± 1%  1850kB/s ± 2%  +258.06%  (p=0.000 n=9+10)

name                                old alloc/op   new alloc/op   delta
HashJoiner/spill=true/rows=256-8       500kB ± 0%      46kB ± 0%   -90.82%  (p=0.000 n=9+9)
HashJoiner/spill=true/rows=4096-8     8.04MB ± 0%    0.79MB ± 0%   -90.16%  (p=0.000 n=9+9)
HashJoiner/spill=true/rows=65536-8     129MB ± 0%      13MB ± 0%   -90.09%  (p=0.000 n=10+9)

name                                old allocs/op  new allocs/op  delta
HashJoiner/spill=true/rows=256-8       2.88k ± 0%     0.83k ± 0%   -71.10%  (p=0.000 n=10+10)
HashJoiner/spill=true/rows=4096-8      45.4k ± 0%     12.7k ± 0%   -72.13%  (p=0.000 n=10+10)
HashJoiner/spill=true/rows=65536-8      726k ± 0%      202k ± 0%   -72.19%  (p=0.000 n=10+10)
```


24605: stats: invalidate table statistic caches through gossip r=RaduBerinde a=RaduBerinde

This change makes the TableStatisticsCache stay up-to-date
automatically: whenever we compute a stat, we gossip a "table stat
added" key for that table, which triggers an invalidation in the
cache.

In the current form, this will add a key per table when we enable
statistics automatically. A possible improvement would be to set a TTL
on these keys and change the cache to always invalidate very old
entries automatically.

Fixes #24585.

Release note: None

24619: storage: remove MVCCStats SpanSet declaration and assertion r=nvanbenschoten a=nvanbenschoten

See #22317 (comment).

Release note: None

Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
Co-authored-by: Abhishek Madan <abhishek@cockroachlabs.com>
Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
@FiloSottile FiloSottile modified the milestones: Go1.10.2, Go1.11 Apr 24, 2018
@FiloSottile
Copy link
Contributor

@gopherbot please open a 1.10 backport tracking issue. We'll need to cherry-pick both CL 92215 by @rsc and CL 104315 by @benesch.

@gopherbot
Copy link

Backport issue(s) opened: #25039 (for 1.10).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

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. release-blocker
Projects
None yet
Development

No branches or pull requests

10 participants