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

go/internal/gcimporter: untyped interface type parameter causes "panic: nil underlying" during testing #57015

Closed
csgura opened this issue Dec 1, 2022 · 15 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@csgura
Copy link

csgura commented Dec 1, 2022

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

$ go version
go version devel go1.20-c85848a4a6 Wed Nov 30 23:25:43 2022 +0000 darwin/arm64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/gura/Library/Caches/go-build"
GOENV="/Users/gura/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/gura/go/pkg/mod"
GONOPROXY="*.uangel.com"
GONOSUMDB="*.uangel.com"
GOOS="darwin"
GOPATH="/Users/gura/go"
GOPRIVATE="*.uangel.com"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/gura/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/gura/sdk/gotip/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="devel go1.20-c85848a4a6 Wed Nov 30 23:25:43 2022 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/mr/dvhjg7zd4l125pln73bqyr400000gp/T/go-build2626569659=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://go.dev/play/p/ZQZ-dOAbTbo?v=gotip

What did you expect to see?

No Error

What did you see instead?

$ gotip test ./...
# gotiperr
panic: nil underlying [recovered]
        panic: nil underlying

goroutine 1 [running]:
go/types.(*Checker).handleBailout(0x140000f81e0, 0x14000117a88)
        /Users/gura/sdk/gotip/src/go/types/check.go:299 +0x98
panic({0x1031cab00, 0x10321ebf8})
        /Users/gura/sdk/gotip/src/runtime/panic.go:884 +0x1f4
go/types.(*Named).under(0x1400010eaf0)
        /Users/gura/sdk/gotip/src/go/types/named.go:494 +0x34c
go/types.under({0x1032211f0?, 0x1400010eaf0?})
        /Users/gura/sdk/gotip/src/go/types/type.go:23 +0x4c
go/types.computeInterfaceTypeSet(0x0, 0x0?, 0x14000138140)
        /Users/gura/sdk/gotip/src/go/types/typeset.go:273 +0x2c8
go/types.(*Interface).typeSet(0x1031d6e60?)
        /Users/gura/sdk/gotip/src/go/types/interface.go:29 +0x2c
go/types.(*typeWriter).typ(0x1400013a0c0, {0x103221178, 0x14000138140})
        /Users/gura/sdk/gotip/src/go/types/typestring.go:239 +0x4c0
go/types.(*typeWriter).typeList(0x1400013a0c0?, {0x14000097970, 0x1, 0x0?})
        /Users/gura/sdk/gotip/src/go/types/typestring.go:372 +0x44
go/types.(*Context).instanceHash(0x140000c0ce0, {0x1032211f0, 0x1400010ea80}, {0x14000097970, 0x1, 0x1})
        /Users/gura/sdk/gotip/src/go/types/context.go:79 +0xd4
go/types.(*Checker).instance(0xffffffffffffffff?, 0x400000005?, {0x1032211f0?, 0x1400010ea80?}, {0x14000097970, 0x1, 0x1}, 0x0, 0x140000c0ce0)
        /Users/gura/sdk/gotip/src/go/types/instantiate.go:97 +0x1e8
go/types.Instantiate(0x140001165a0?, {0x1032211f0?, 0x1400010ea80}, {0x14000097970, 0x1, 0x1}, 0x0?)
        /Users/gura/sdk/gotip/src/go/types/instantiate.go:63 +0x2f0
go/internal/gcimporter.(*reader).doTyp(0x140001165a0)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:334 +0xd0
go/internal/gcimporter.(*pkgReader).typIdx(0x140000c90e0, {0x29?, 0x0?}, 0x140000b1380)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:308 +0x154
go/internal/gcimporter.(*reader).typ(0x140000b13e0)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:278 +0x4c
go/internal/gcimporter.(*reader).param(0x140000b13e0)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:453 +0x58
go/internal/gcimporter.(*reader).params(0x140000b13e0)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:442 +0x78
go/internal/gcimporter.(*reader).signature(0x140000b13e0, 0x140001343f8?, {0x0, 0x0, 0x0}, {0x0, 0x0, 0x0})
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:431 +0x58
go/internal/gcimporter.(*pkgReader).objIdx(0x140000c90e0, 0x6?)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:529 +0x7f4
go/internal/gcimporter.readUnifiedPackage(0x1400011e9c0, 0x0, 0x14000099c50, {0x1, 0x0, {0x140000b4f10, 0xf}, {0x140001341e0, 0x4b3}, {0x140000f21c0, ...}, ...})
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:83 +0x2d4
go/internal/gcimporter.Import(0x1?, 0x0?, {0x140000b4f10, 0xf}, {0x0?, 0x1031ddfe0?}, 0x140000976f0?)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/gcimporter.go:202 +0x60c
go/importer.(*gcimports).ImportFrom(0x14000117058?, {0x140000b4f10?, 0x103101150?}, {0x0?, 0x1031de3e0?}, 0x140000c8d80?)
        /Users/gura/sdk/gotip/src/go/importer/importer.go:102 +0x4c
go/importer.(*gcimports).Import(0x1031d83c0?, {0x140000b4f10?, 0x140000be241?})
        /Users/gura/sdk/gotip/src/go/importer/importer.go:95 +0x2c
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func2({0x140000be241?, 0x140000c0b20?})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:222 +0x6c
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.importerFunc.Import(0x140001171e8?, {0x140000be241?, 0x140000c0b20?})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:401 +0x30
go/types.(*Checker).importPackage(0x140000f81e0, {0x10321f920, 0x140000c0a20}, {0x140000be241, 0xf}, {0x140000ba640, 0x18})
        /Users/gura/sdk/gotip/src/go/types/resolver.go:163 +0x1c0
go/types.(*Checker).collectObjects.func1({0x103220300?, 0x14000099ad0?})
        /Users/gura/sdk/gotip/src/go/types/resolver.go:267 +0xc0
go/types.(*Checker).walkDecl(0x140000ba640?, {0x103221a38?, 0x1400011ea00?}, 0x14000117950)
        /Users/gura/sdk/gotip/src/go/types/decl.go:404 +0x1a4
go/types.(*Checker).walkDecls(...)
        /Users/gura/sdk/gotip/src/go/types/decl.go:391
go/types.(*Checker).collectObjects(0x140000f81e0)
        /Users/gura/sdk/gotip/src/go/types/resolver.go:254 +0x990
go/types.(*Checker).checkFiles(0x140000f81e0, {0x140000ae5e8?, 0x140000c3950?, 0x0?})
        /Users/gura/sdk/gotip/src/go/types/check.go:326 +0xa4
go/types.(*Checker).Files(...)
        /Users/gura/sdk/gotip/src/go/types/check.go:304
go/types.(*Config).Check(0x1031d7d60?, {0x140000b4f08?, 0x140000180a7?}, 0x5?, {0x140000ae5e8, 0x1, 0x1}, 0x102eff8dc?)
        /Users/gura/sdk/gotip/src/go/types/api.go:416 +0x70
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run(0x1400011e9c0, 0x140000a3c70, {0x1400011e880, 0x8, 0x1031d56c0?})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:238 +0x408
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.Run({0x16cf0afa0?, 0x1c?}, {0x1400011e880, 0x8, 0x8})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:132 +0x90
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.Main({0x1400011c000, 0x1c, 0x1c})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:119 +0x2d0
main.main()
        /Users/gura/sdk/gotip/src/cmd/vet/main.go:45 +0x458
# gotiperr/option_test
panic: nil underlying [recovered]
        panic: nil underlying

goroutine 1 [running]:
go/types.(*Checker).handleBailout(0x140001781e0, 0x14000197a88)
        /Users/gura/sdk/gotip/src/go/types/check.go:299 +0x98
panic({0x103056b00, 0x1030aabf8})
        /Users/gura/sdk/gotip/src/runtime/panic.go:884 +0x1f4
go/types.(*Named).under(0x1400018eaf0)
        /Users/gura/sdk/gotip/src/go/types/named.go:494 +0x34c
go/types.under({0x1030ad1f0?, 0x1400018eaf0?})
        /Users/gura/sdk/gotip/src/go/types/type.go:23 +0x4c
go/types.computeInterfaceTypeSet(0x0, 0x0?, 0x140001b6230)
        /Users/gura/sdk/gotip/src/go/types/typeset.go:273 +0x2c8
go/types.(*Interface).typeSet(0x103062e60?)
        /Users/gura/sdk/gotip/src/go/types/interface.go:29 +0x2c
go/types.(*typeWriter).typ(0x140001b80f0, {0x1030ad178, 0x140001b6230})
        /Users/gura/sdk/gotip/src/go/types/typestring.go:239 +0x4c0
go/types.(*typeWriter).typeList(0x140001b80f0?, {0x14000119980, 0x1, 0x0?})
        /Users/gura/sdk/gotip/src/go/types/typestring.go:372 +0x44
go/types.(*Context).instanceHash(0x14000142da0, {0x1030ad1f0, 0x1400018ea80}, {0x14000119980, 0x1, 0x1})
        /Users/gura/sdk/gotip/src/go/types/context.go:79 +0xd4
go/types.(*Checker).instance(0xffffffffffffffff?, 0x400000005?, {0x1030ad1f0?, 0x1400018ea80?}, {0x14000119980, 0x1, 0x1}, 0x0, 0x14000142da0)
        /Users/gura/sdk/gotip/src/go/types/instantiate.go:97 +0x1e8
go/types.Instantiate(0x140001965a0?, {0x1030ad1f0?, 0x1400018ea80}, {0x14000119980, 0x1, 0x1}, 0x0?)
        /Users/gura/sdk/gotip/src/go/types/instantiate.go:63 +0x2f0
go/internal/gcimporter.(*reader).doTyp(0x140001965a0)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:334 +0xd0
go/internal/gcimporter.(*pkgReader).typIdx(0x140001490e0, {0x29?, 0x0?}, 0x140001332c0)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:308 +0x154
go/internal/gcimporter.(*reader).typ(0x14000133320)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:278 +0x4c
go/internal/gcimporter.(*reader).param(0x14000133320)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:453 +0x58
go/internal/gcimporter.(*reader).params(0x14000133320)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:442 +0x78
go/internal/gcimporter.(*reader).signature(0x14000133320, 0x140001b23f8?, {0x0, 0x0, 0x0}, {0x0, 0x0, 0x0})
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:431 +0x58
go/internal/gcimporter.(*pkgReader).objIdx(0x140001490e0, 0x6?)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:529 +0x7f4
go/internal/gcimporter.readUnifiedPackage(0x1400019e9c0, 0x0, 0x1400011bc80, {0x1, 0x0, {0x14000136f00, 0xf}, {0x140001b21e0, 0x4b3}, {0x140001721c0, ...}, ...})
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/ureader.go:83 +0x2d4
go/internal/gcimporter.Import(0x1?, 0x0?, {0x14000136f00, 0xf}, {0x0?, 0x103069fe0?}, 0x14000119700?)
        /Users/gura/sdk/gotip/src/go/internal/gcimporter/gcimporter.go:202 +0x60c
go/importer.(*gcimports).ImportFrom(0x14000197058?, {0x14000136f00?, 0x102f8d150?}, {0x0?, 0x10306a3e0?}, 0x14000148d80?)
        /Users/gura/sdk/gotip/src/go/importer/importer.go:102 +0x4c
go/importer.(*gcimports).Import(0x1030643c0?, {0x14000136f00?, 0x14000140259?})
        /Users/gura/sdk/gotip/src/go/importer/importer.go:95 +0x2c
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run.func2({0x14000140259?, 0x14000142be0?})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:222 +0x6c
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.importerFunc.Import(0x140001971e8?, {0x14000140259?, 0x14000142be0?})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:401 +0x30
go/types.(*Checker).importPackage(0x140001781e0, {0x1030ab920, 0x14000142a20}, {0x14000140259, 0xf}, {0x14000158180, 0x1f})
        /Users/gura/sdk/gotip/src/go/types/resolver.go:163 +0x1c0
go/types.(*Checker).collectObjects.func1({0x1030ac300?, 0x1400011bad0?})
        /Users/gura/sdk/gotip/src/go/types/resolver.go:267 +0xc0
go/types.(*Checker).walkDecl(0x14000158180?, {0x1030ada38?, 0x1400019ea00?}, 0x14000197950)
        /Users/gura/sdk/gotip/src/go/types/decl.go:404 +0x1a4
go/types.(*Checker).walkDecls(...)
        /Users/gura/sdk/gotip/src/go/types/decl.go:391
go/types.(*Checker).collectObjects(0x140001781e0)
        /Users/gura/sdk/gotip/src/go/types/resolver.go:254 +0x990
go/types.(*Checker).checkFiles(0x140001781e0, {0x140001305f8?, 0x1400010d9f0?, 0x0?})
        /Users/gura/sdk/gotip/src/go/types/check.go:326 +0xa4
go/types.(*Checker).Files(...)
        /Users/gura/sdk/gotip/src/go/types/check.go:304
go/types.(*Config).Check(0x103063d60?, {0x14000140240?, 0x140000180a7?}, 0x5?, {0x140001305f8, 0x1, 0x1}, 0x102d8b8dc?)
        /Users/gura/sdk/gotip/src/go/types/api.go:416 +0x70
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.run(0x1400019e9c0, 0x140001a01a0, {0x1400019e880, 0x8, 0x1030616c0?})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:238 +0x408
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.Run({0x16d07ef70?, 0x1c?}, {0x1400019e880, 0x8, 0x8})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:132 +0x90
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.Main({0x1400019c000, 0x1c, 0x1c})
        /Users/gura/sdk/gotip/src/cmd/vendor/golang.org/x/tools/go/analysis/unitchecker/unitchecker.go:119 +0x2d0
main.main()
        /Users/gura/sdk/gotip/src/cmd/vet/main.go:45 +0x458
FAIL    gotiperr/option [build failed]
FAIL
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Dec 1, 2022
@cuonglm
Copy link
Member

cuonglm commented Dec 1, 2022

I can't reproduce this locally, either with current tip or with c85848a

@toothrot
Copy link
Contributor

toothrot commented Dec 1, 2022

I haven't been able to reproduce this either. Can you still reproduce this, @csgura?

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 1, 2022
@toothrot toothrot added this to the Backlog milestone Dec 1, 2022
@mdempsky mdempsky added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Dec 1, 2022
@csgura
Copy link
Author

csgura commented Dec 2, 2022

Sorry for the late reply.
I posted the wrong source. This is the code causing the error.

https://go.dev/play/p/bnxnM-U3JLm?v=gotip

@randall77
Copy link
Contributor

I can reproduce.

This is vet crashing inside of go/types.
@findleyr @griesemer

@randall77 randall77 modified the milestones: Backlog, Go1.20 Dec 5, 2022
@findleyr
Copy link
Contributor

findleyr commented Dec 5, 2022

@randall77 what exactly did you do to reproduce? I copied the files exactly (I thought), and don't see the same at tip.

Without go 1.18 in the go.mod, for example, I get an error about -lang. With go 1.18, I get no error.

@findleyr
Copy link
Contributor

findleyr commented Dec 5, 2022

Nevermind, I copied from the wrong playground link. I can reproduce.

@randall77
Copy link
Contributor

issue57015 % find . 
.
./go.mod
./option
./option/option.go
./option/option_test.go
./main.go

go.mod:

module gotiperr

go 1.18

main.go:

package main

import "gotiperr/option"

func main() {
	_ = option.Option[int]{}
}

option.go:

package option

import "io"

type Option[T any] struct {
}

func GetCloser() Option[interface {
	io.Closer
}] {
	panic("")
}

option_test.go:

package option_test

import (
	"gotiperr/option"
	"testing"
)

func TestNil(t *testing.T) {
	_ = option.Option[int]{}

}

@findleyr
Copy link
Contributor

findleyr commented Dec 5, 2022

Thanks, I am able to reproduce.

Will take a closer look later this afternoon. The error suggests that we're trying to instantiate with a type that isn't completely set up. This is related to the new ureader code, but may be an artifact of overly tricky setup for importing named types via go/types.

CC @mdempsky

@heschi
Copy link
Contributor

heschi commented Dec 5, 2022

Does this need to block the RC?

@findleyr
Copy link
Contributor

findleyr commented Dec 5, 2022

Ok, I looked into it. This panic can be reached via importing the following package:

package p

import "io"

type Option[T any] struct {
}

var X Option[interface{ io.Closer }]

The issue is that in the unified importer, io.Closer has not had its underlying set when Option[interface{ io.Closer }] is created. We need the underlying in order to compute the identify (i.e. hash) of the interface type argument, for the purpose of looking up in the types.Context. There's no way around this: we always need to be able to hash the type argument when instantiating, even if a nil Context is passed, for the purpose of cycle-breaking.

Playing around with the unified importer, I see that we can make this work by invoking laterFors[t] whenever we embed t, i.e. ensure that any embedded type is complete. But that is an unsatisfying and partial solution. I think perhaps https://go.dev/cl/424876 may be a better solution.

@randall77
Copy link
Contributor

I don't think so. It affects vet, so at most it would affect testing setups. You could still build a binary and test it in production without fixing this issue. Although if your test doesn't compile, you may not want to push an untested binary to production...

@findleyr
Copy link
Contributor

findleyr commented Dec 5, 2022

@heschi @randall77 I'm not sure what the threshold is for the RC, but this seems like it would be pretty easy encounter when using the RC.

@findleyr
Copy link
Contributor

findleyr commented Dec 5, 2022

Assigning to @mdempsky since this is related to the unified importer and https://go.dev/cl/424876 looks like a candidate fix.

Matthew: can we revisit that CL?

@findleyr findleyr changed the title cmd/compile: untyped interface type parameter causes "panic: nil underlying" during testing go/internal/gcimporter: untyped interface type parameter causes "panic: nil underlying" during testing Dec 5, 2022
@gopherbot
Copy link

Change https://go.dev/cl/424876 mentions this issue: go/internal/gcimporter: simplify unified IR importer

@gopherbot
Copy link

Change https://go.dev/cl/456376 mentions this issue: internal/gcimporter: port CL 424876 from std importer

gopherbot pushed a commit to golang/tools that referenced this issue Dec 9, 2022
This is a port of CL 424876, except it temporarily maintains a
fallback path for older cmd/compile behavior before CL 455279, to
avoid breaking users following Go tip.

Updates golang/go#57015.

Change-Id: I168d171153d96485e92be19645422fe65ab4b345
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456376
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@golang golang locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

8 participants