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

cmd/compile: assertion failure when building syntax package with gotypesalias enabled #66873

Closed
griesemer opened this issue Apr 17, 2024 · 10 comments
Closed

cmd/compile: assertion failure when building syntax package with gotypesalias enabled #66873

griesemer opened this issue Apr 17, 2024 · 10 comments
Assignees
@mdempsky @griesemer
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.23

Comments

@griesemer
Copy link
Contributor

griesemer commented Apr 17, 2024 •
edited
Loading

To reproduce:

  1. make.bash at tip
  2. comment out cmd/compile/internal/noder/irgen.go:91 (i.e., don't call os.Setenv)
  3. go install cmd/compile
  4. go build cmd/compile/internal/syntax

$ go build cmd/compile/internal/syntax
# cmd/compile/internal/syntax
cmd/compile/internal/syntax/tokens.go:7:6: internal compiler error: assertion failed

goroutine 1 [running]:
runtime/debug.Stack()
./runtime/debug/stack.go:26 +0x5e
cmd/compile/internal/base.FatalfAt({0x1853a40?, 0xc0?}, {0x69dcafc, 0x10}, {0x0, 0x0, 0x0})
cmd/compile/internal/base/print.go:225 +0x1d7
cmd/compile/internal/base.Fatalf(...)
cmd/compile/internal/base/print.go:194
cmd/compile/internal/base.Assert(...)
cmd/compile/internal/base/print.go:237
cmd/compile/internal/noder.assert(...)
cmd/compile/internal/noder/stencil.go:15
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:430
cmd/compile/internal/noder.(*reader).typWrapped(0xc0018537c0, 0x1)
cmd/compile/internal/noder/reader.go:388 +0x156
cmd/compile/internal/noder.(*reader).typ(...)
cmd/compile/internal/noder/reader.go:382
cmd/compile/internal/noder.(*reader).param(0xc0018537c0)
cmd/compile/internal/noder/reader.go:622 +0x18d
cmd/compile/internal/noder.(*reader).method(0xc0018537c0, 0xc001853900)
cmd/compile/internal/noder/reader.go:1022 +0x7c
cmd/compile/internal/noder.(*pkgReader).objIdxMayFail(0xc000ad56c0, 0x25, {0x0, 0x0, 0x0}, {0x721b420, 0x0, 0x0}, 0x0)
cmd/compile/internal/noder/reader.go:816 +0xd76
cmd/compile/internal/noder.(*pkgReader).objIdx(...)
cmd/compile/internal/noder/reader.go:670
cmd/compile/internal/noder.(*pkgReader).objInstIdx(0xc000ad56c0, {0xffffffff?, {0x721b420?, 0xc000600008?, 0xc0018468f0?}}, 0xc0017f5790, 0x0)
cmd/compile/internal/noder/reader.go:662 +0x96
cmd/compile/internal/noder.(*reader).obj(0xc001853400)
cmd/compile/internal/noder/reader.go:634 +0x46
cmd/compile/internal/noder.(*reader).doTyp(0xc001853400)
cmd/compile/internal/noder/reader.go:500 +0x72
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:429
cmd/compile/internal/noder.(*reader).typWrapped(0xc001853040, 0x1)
cmd/compile/internal/noder/reader.go:388 +0x125
cmd/compile/internal/noder.(*reader).typ(...)
cmd/compile/internal/noder/reader.go:382
cmd/compile/internal/noder.(*pkgReader).objIdxMayFail(0xc000ad56c0, 0x24, {0x0, 0x0, 0x0}, {0x721b420, 0x0, 0x0}, 0x0)
cmd/compile/internal/noder/reader.go:744 +0xca5
cmd/compile/internal/noder.(*pkgReader).objIdx(...)
cmd/compile/internal/noder/reader.go:670
cmd/compile/internal/noder.(*pkgReader).objInstIdx(0xc000ad56c0, {0xffffffff?, {0x721b420?, 0xc000ccea40?, 0x60c1056?}}, 0xc0017f56c0, 0x0)
cmd/compile/internal/noder/reader.go:662 +0x96
cmd/compile/internal/noder.(*reader).obj(0xc001852c80)
cmd/compile/internal/noder/reader.go:634 +0x46
cmd/compile/internal/noder.(*reader).doTyp(0xc001852c80)
cmd/compile/internal/noder/reader.go:500 +0x72
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:429
cmd/compile/internal/noder.(*reader).typWrapped(0xc001852b40, 0x1)
cmd/compile/internal/noder/reader.go:388 +0x125
cmd/compile/internal/noder.(*reader).typ(...)
cmd/compile/internal/noder/reader.go:382
cmd/compile/internal/noder.(*reader).structType(0xc001852b40)
cmd/compile/internal/noder/reader.go:589 +0xad
cmd/compile/internal/noder.(*reader).doTyp(0xc001852b40)
cmd/compile/internal/noder/reader.go:522 +0x37f
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:429
cmd/compile/internal/noder.(*reader).typWrapped(0xc0018528c0, 0x0)
cmd/compile/internal/noder/reader.go:388 +0x125
cmd/compile/internal/noder.(*pkgReader).objIdxMayFail(0xc000ad56c0, 0x23, {0x0, 0x0, 0x0}, {0x721b420, 0x0, 0x0}, 0x0)
cmd/compile/internal/noder/reader.go:805 +0x53c
cmd/compile/internal/noder.(*pkgReader).objIdx(...)
cmd/compile/internal/noder/reader.go:670
cmd/compile/internal/noder.(*pkgReader).objInstIdx(0xc000ad56c0, {0xffffffff?, {0x721b420?, 0x39300000005?, 0x0?}}, 0xc0017f4dd0, 0x0)
cmd/compile/internal/noder/reader.go:662 +0x96
cmd/compile/internal/noder.(*reader).obj(0xc001852500)
cmd/compile/internal/noder/reader.go:634 +0x46
cmd/compile/internal/noder.(*reader).doTyp(0xc001852500)
cmd/compile/internal/noder/reader.go:500 +0x72
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:429
cmd/compile/internal/noder.(*reader).typWrapped(0xc0018523c0, 0x1)
cmd/compile/internal/noder/reader.go:388 +0x125
cmd/compile/internal/noder.(*reader).typ(...)
cmd/compile/internal/noder/reader.go:382
cmd/compile/internal/noder.(*reader).doTyp(0xc0018523c0)
cmd/compile/internal/noder/reader.go:516 +0xdb
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:429
cmd/compile/internal/noder.(*reader).typWrapped(0xc001852280, 0x1)
cmd/compile/internal/noder/reader.go:388 +0x125
cmd/compile/internal/noder.(*reader).typ(...)
cmd/compile/internal/noder/reader.go:382
cmd/compile/internal/noder.(*reader).doTyp(0xc001852280)
cmd/compile/internal/noder/reader.go:520 +0x297
cmd/compile/internal/noder.(*pkgReader).typIdx(...)
cmd/compile/internal/noder/reader.go:429
cmd/compile/internal/noder.(*reader).typWrapped(0xc001844140, 0x1)
cmd/compile/internal/noder/reader.go:388 +0x125
cmd/compile/internal/noder.(*reader).typ(...)
cmd/compile/internal/noder/reader.go:382
cmd/compile/internal/noder.(*reader).param(...)
cmd/compile/internal/noder/reader.go:622
cmd/compile/internal/noder.(*reader).params(0xc001844140)
cmd/compile/internal/noder/reader.go:615 +0xad
cmd/compile/internal/noder.(*reader).signature(0xc001844140, 0xc001848870)
cmd/compile/internal/noder/reader.go:603 +0x5c
cmd/compile/internal/noder.(*reader).method(0xc001844140, 0xc001844280)
cmd/compile/internal/noder/reader.go:1023 +0x94
cmd/compile/internal/noder.(*pkgReader).objIdxMayFail(0xc000ad56c0, 0xcd, {0x0, 0x0, 0x0}, {0x721b420, 0x0, 0x0}, 0x0)
cmd/compile/internal/noder/reader.go:816 +0xd76
cmd/compile/internal/noder.(*pkgReader).objIdx(...)
cmd/compile/internal/noder/reader.go:670
cmd/compile/internal/noder.(*pkgReader).objInstIdx(0xc000ad56c0, {0xccf628?, {0x721b420?, 0x610d669?, 0x8?}}, 0x0, 0x0)
cmd/compile/internal/noder/reader.go:662 +0x96
cmd/compile/internal/noder.(*reader).obj(0xc000fed400)
cmd/compile/internal/noder/reader.go:634 +0x46
cmd/compile/internal/noder.(*reader).pkgObjs(0xc000fed400, 0xc000000240)
cmd/compile/internal/noder/reader.go:3314 +0x9d
cmd/compile/internal/noder.(*reader).pkgDecls(0xc000fed400, 0xc000000240)
cmd/compile/internal/noder/reader.go:3303 +0x18f
cmd/compile/internal/noder.(*reader).pkgInit(0xc000fed400, 0xc00007a320?, 0xc000000240)
cmd/compile/internal/noder/reader.go:3209 +0xe5
cmd/compile/internal/noder.unified({0x0?, {0x0?, 0x0?}}, {0xc000158380?, 0x6b64b00?, 0x0?})
cmd/compile/internal/noder/unified.go:188 +0x17c
cmd/compile/internal/noder.LoadPackage({0xc000022358, 0x10, 0x12})
cmd/compile/internal/noder/noder.go:77 +0x43a
cmd/compile/internal/gc.Main(0x6c18470)
cmd/compile/internal/gc/main.go:197 +0xbbd
main.main()
cmd/compile/main.go:57 +0xf9
@griesemer griesemer added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 17, 2024
@griesemer griesemer added this to the Go1.23 milestone Apr 17, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 17, 2024
@griesemer
Copy link
Contributor Author

@adonovan for visibility

@griesemer
Copy link
Contributor Author

griesemer commented Apr 17, 2024 •
edited
Loading

The package object being compiled is checkBranches (cmd/compile/internal/syntax/branches.go) and then as part of it, (*labelScope).enclosingTarget.

@ianlancetaylor ianlancetaylor mentioned this issue Apr 17, 2024
Closed
@griesemer
Copy link
Contributor Author

Follo-up: With pending CL 579935 (which may eventually get submitted), to enable type alias nodes, set the new types2.Config.EnableAlias flag in irgen.go.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/568455 mentions this issue: cmd/compile: add test case for using Alias types

@griesemer griesemer mentioned this issue May 8, 2024
Closed
@griesemer
Copy link
Contributor Author

Simple reproducer:

package p

func f(A) {}

type T int

type A = T

func (A) m() {}

Compiling this package (go tool compile p.go) with types2.Config.EnableAlias enabled causes the assertion failure. Changing the order of the declarations avoids the failure.

@mdempsky any insights?

@cuonglm
Copy link
Member

cuonglm commented May 13, 2024

See also my investigation here: #65893 (comment)

We should probably land CL https://go-review.googlesource.com/c/go/+/568455, so this is the only issue that track the problem.

@griesemer
Copy link
Contributor Author

The type checker seems to be doing the right thing for the above program:

== packageObjects ==

testdata/manual.go:14:6:        -- checking type T (white, objPath = )
testdata/manual.go:14:8:        .  -- type int
testdata/manual.go:14:8:        .  => int // *Basic
testdata/manual.go:14:6:        => type T int (black)

testdata/manual.go:16:6:        -- checking type A (white, objPath = )
testdata/manual.go:16:10:       .  -- type T
testdata/manual.go:16:10:       .  => T (under = int) // *Named
testdata/manual.go:16:6:        => type A = T (black)

testdata/manual.go:12:6:        -- checking func f (white, objPath = )
testdata/manual.go:12:8:        .  -- type A
testdata/manual.go:12:8:        .  => A (under = int) // *Alias
testdata/manual.go:12:6:        => func f(A) (black)

testdata/manual.go:18:10:       -- checking func m (white, objPath = )
testdata/manual.go:18:7:        .  -- type A
testdata/manual.go:18:7:        .  => A (under = int) // *Alias
testdata/manual.go:18:10:       => func (A).m() (black)

== processDelayed ==
testdata/manual.go:14:6:        -- validType(T)
testdata/manual.go:14:6:        .  validType(T) nest , path 
testdata/manual.go:14:6:        .  .  validType(int) nest T, path T

testdata/manual.go:14:6:        -- verifying field uniqueness for T

testdata/manual.go:16:6:        -- validType(A)
testdata/manual.go:14:6:        .  validType(T) nest , path 
testdata/manual.go:14:6:        .  .  validType(int) nest T, path T

testdata/manual.go:12:8:        -- check var type A

testdata/manual.go:12:6:        -- func f
testdata/manual.go:12:11:       -- f: func(A)

testdata/manual.go:18:7:        -- check var type A

testdata/manual.go:18:7:        -- validate receiver var  A

testdata/manual.go:18:10:       -- func m
testdata/manual.go:18:14:       -- m: func()

@mdempsky
Copy link
Contributor

I think types2 is behaving correctly. The issue is we don't have proper alias types in compiler IR, so the importer can't rely on them to break cycles.

We might have to do a two-phase initialization to ensure IR is constructed in the right order. Thinking about it.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/585399 mentions this issue: cmd/compile/internal/noder: enable type aliases in type checker

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 15, 2024
gopherbot pushed a commit that referenced this issue May 16, 2024
@cuonglm @gopherbot
cmd/compile: add test case for using Alias types
8ce2fed 
CL 579935 disabled usage of Alias types in the compiler, and tracks
the problem with issue #66873. The test case in #65893 passes now
with the current tip. This CL adds a test case to ensure there is no
regression once Alias types are enabled for the compiler.

Updates #66873
Fixes #65893

Change-Id: I51b51bb13ca59549bc5925dd95f73da40465556d
Reviewed-on: https://go-review.googlesource.com/c/go/+/568455
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Cuong Manh Le <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604099 mentions this issue: cmd/compile/internal/importer: enable aliases

gopherbot pushed a commit that referenced this issue Aug 15, 2024
@timothy-king
cmd/compile/internal/importer: enable aliases
209ed1a 
Flips the pkgReader.enableAlias flag to true when reading unified IR.
This was disabled while resolving #66873. This resolves the TODO to
flip it back to true.

Updates #66873
Updates #68778

Change-Id: Ifd52b0f9510d6bcf151de1c9a18d71ab548c14e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/604099
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: David Chase <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdempsky mdempsky

@griesemer griesemer

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Go1.23
Development

No branches or pull requests
5 participants
@mdempsky @dmitshur @cuonglm @griesemer @gopherbot