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: crash instantiating with recursive type #50259

Closed
jhbrown94 opened this issue Dec 19, 2021 · 4 comments
Closed

cmd/compile: crash instantiating with recursive type #50259

jhbrown94 opened this issue Dec 19, 2021 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jhbrown94
Copy link

jhbrown94 commented Dec 19, 2021

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

go version go1.18beta1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

Can reproduce in playground, but locally

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jhbrown/Library/Caches/go-build"
GOENV="/Users/jhbrown/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/jhbrown/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jhbrown/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/jhbrown/sdk/go1.18beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/jhbrown/sdk/go1.18beta1/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18beta1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/d6/kh18314s5wj315ls2klq51nr0000gn/T/go-build1923624470=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I referred to a recursive type from another file, causing a recursive type error to turn into a compiler crash.
See https://go.dev/play/p/FMJLSwjhEMU?v=gotip

What did you expect to see?

Either a clean compile, or a clear warning that I'm breaking the type rules.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x50 pc=0x5c5918]

goroutine 1 [running]:
cmd/compile/internal/ir.TypeNodeAt({0x45e000?, 0xc0?}, 0xe8e630?)
	/usr/local/go/src/cmd/compile/internal/ir/type.go:314 +0x18
...
@ianlancetaylor ianlancetaylor changed the title Referring to a recursive type from another file in the same package causes compiler crash cmd/compile: crash when referring to a recursive type from another file in the same package Dec 19, 2021
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 19, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.18 milestone Dec 19, 2021
@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Dec 19, 2021

CC @randall77 @danscales @griesemer @findleyr

This is invalid code so not marking as a release blocker.

It may be a bug that this is getting past the type checker.

Here is a single file reproducer.

package foo

var v Box[Step]
type Box[T any] struct{}
type Step = Box[StepBox]
type StepBox Box[Step]

Running go build foo.go gives me

# command-line-arguments
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x50 pc=0x5c5918]

goroutine 1 [running]:
cmd/compile/internal/ir.TypeNodeAt({0x152240?, 0xc0?}, 0xe8e630?)
	/home/iant/go/src/cmd/compile/internal/ir/type.go:314 +0x18
cmd/compile/internal/ir.TypeNode(...)
	/home/iant/go/src/cmd/compile/internal/ir/type.go:303
cmd/compile/internal/noder.(*irgen).expr(0xc000152240, {0xe8f868?, 0xc0003dafc0?})
	/home/iant/go/src/cmd/compile/internal/noder/expr.go:46 +0x945
cmd/compile/internal/noder.(*irgen).typeExpr(0xc000152240, {0xe8f868?, 0xc0003dafc0})
	/home/iant/go/src/cmd/compile/internal/noder/expr.go:409 +0x31
cmd/compile/internal/noder.(*irgen).typeDecl(0xc000152240, 0xc0001520f0, 0xc00007bc20)
	/home/iant/go/src/cmd/compile/internal/noder/decl.go:215 +0x1ab
cmd/compile/internal/noder.(*irgen).generate(0xc000152240, {0xc00000e448, 0x1, 0x203000?})
	/home/iant/go/src/cmd/compile/internal/noder/irgen.go:260 +0x845
cmd/compile/internal/noder.check2({0xc00000e448, 0x1, 0x1})
	/home/iant/go/src/cmd/compile/internal/noder/irgen.go:92 +0x16d
cmd/compile/internal/noder.LoadPackage({0xc00001e1f0, 0x1, 0x0?})
	/home/iant/go/src/cmd/compile/internal/noder/noder.go:90 +0x335
cmd/compile/internal/gc.Main(0xd4bb90)
	/home/iant/go/src/cmd/compile/internal/gc/main.go:191 +0xb13
main.main()
	/home/iant/go/src/cmd/compile/main.go:55 +0xdd

@griesemer
Copy link
Contributor

griesemer commented Dec 19, 2021

Yes, I think this should not get past the type checker.

@griesemer griesemer self-assigned this Dec 19, 2021
@ianlancetaylor ianlancetaylor changed the title cmd/compile: crash when referring to a recursive type from another file in the same package cmd/compile: crash using recursive type Dec 19, 2021
@ianlancetaylor ianlancetaylor changed the title cmd/compile: crash using recursive type cmd/compile: crash instantiating with recursive type Dec 19, 2021
@gopherbot
Copy link

gopherbot commented Jan 21, 2022

Change https://golang.org/cl/379916 mentions this issue: cmd/compile/internal/types2: report an error when using a broken alias

@gopherbot
Copy link

gopherbot commented Jan 21, 2022

Change https://golang.org/cl/380056 mentions this issue: cmd/compile/internal/types2: reorder object processing to avoid broken aliases

gopherbot pushed a commit that referenced this issue Jan 24, 2022
By processing non-alias type declarations before alias type declaration,
and those before everything else we can avoid some of the remaining
errors which are due to alias types not being available.

For #25838.
For #50259.
For #50276.
For #50729.

Change-Id: I233da2899a6d4954c239638624dfa8c08662e6b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/380056
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
The type checker doesn't have a general mechanism to "use" the type
of a type alias whose type depends on a recursive type declaration
which is not yet completely type-checked. In some cases, the type of
a type alias is needed before it is determined; the type is incorrect
(invalid) in that case but no error is reported. The type-checker is
happy with this (incorrect type), but the compiler may crash under
some circumstances.

A correct fix will likely require some form of forwarding type which
is a fairly pervasive change and may also affect the type checker API.

This CL introduces a simple side table, a map of broken type aliases,
which is consulted before the type associated with a type alias is
used. If the type alias is broken, an error is reported.

This is a stop-gap solution that prevents the compiler from crashing.
The reported error refers to the corresponding issue which suggests
a work-around that may be applicable in some cases.

Also fix a minor error related to type cycles: If we have a cycle
that doesn't start with a type, don't use a compiler error message
that explicitly mentions "type".

Fixes golang#50259.
Fixes golang#50276.
Fixes golang#50779.

For golang#50729.

Change-Id: Ie8e38f49ef724e742e8e78625e6d4f3d4014a52c
Reviewed-on: https://go-review.googlesource.com/c/go/+/379916
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
By processing non-alias type declarations before alias type declaration,
and those before everything else we can avoid some of the remaining
errors which are due to alias types not being available.

For golang#25838.
For golang#50259.
For golang#50276.
For golang#50729.

Change-Id: I233da2899a6d4954c239638624dfa8c08662e6b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/380056
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

4 participants