golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.46k stars 17.59k forks source link

cmd/compile: bootstrap glitch with version checking on min/max usage #65886

Open thanm opened 7 months ago

thanm commented 7 months ago

Working on a linux/amd64 machine on tip:

$ go version 
go version devel go1.23-638b9023e3 Thu Feb 22 15:37:30 2024 +0000 linux/amd64
$

I have run into a compiler bootstrapping issue with make.bash. To reproduce the issue, check out a Go repo at tip, then do this:

$ cat > cmd/compile/internal/inline/x.go << EOF
package inline
func Doit(i, j int) int {
  return min(i,j)
}
EOF
$

This adds a new file that makes use of the predeclared "min" function (added in Go 1.21). Once this file is in place, the boot strap crashes as follows:

$ GOTOOLCHAIN=local GOROOT_BOOTSTRAP=/w/xgo bash make.bash 
Building Go cmd/dist using /w/xgo. (devel go1.23-39ec246e73 Tue Feb 6 22:21:42 2024 +0000 linux/amd64)
Building Go toolchain1 using /w/xgo.
# bootstrap/cmd/compile/internal/liveness
<unknown line number>: internal compiler error: panic: runtime error: invalid memory address or nil pointer dereference

goroutine 1 [running]:
runtime/debug.Stack()
    /w/xgo/src/runtime/debug/stack.go:26 +0x5e
cmd/compile/internal/base.FatalfAt({0xcd6eb0?, 0xc0?}, {0xdec667, 0x9}, {0xc000cd6ee0, 0x1, 0x1})
    /w/xgo/src/cmd/compile/internal/base/print.go:225 +0x1d7
cmd/compile/internal/base.Fatalf(...)
    /w/xgo/src/cmd/compile/internal/base/print.go:194
cmd/compile/internal/gc.handlePanic()
    /w/xgo/src/cmd/compile/internal/gc/main.go:52 +0x90
panic({0xd6a340?, 0x146a790?})
    /w/xgo/src/runtime/panic.go:759 +0x132
cmd/compile/internal/types2.(*Checker).handleBailout(0xc00052e800, 0xc000cd9060)
    /w/xgo/src/cmd/compile/internal/types2/check.go:363 +0x88
panic({0xd6a340?, 0x146a790?})
    /w/xgo/src/runtime/panic.go:759 +0x132
cmd/compile/internal/noder.checkFiles.func1({0xf54580?, 0xc000dd4600?})
    /w/xgo/src/cmd/compile/internal/noder/irgen.go:77 +0x209
cmd/compile/internal/types2.(*Checker).err(0xc00052e800, {0xf54880, 0xc000480730}, 0x87, {0xc00052bd40?, 0x0?}, 0x1)
    /w/xgo/src/cmd/compile/internal/types2/errors.go:276 +0x464
cmd/compile/internal/types2.(*Checker).versionErrorf(0xc00052e800, {0xf54880, 0xc000480730}, {0xde9257, 0x6}, {0xde4b0b?, 0x3?}, {0x0, 0x0, 0x0})
    /w/xgo/src/cmd/compile/internal/types2/errors.go:303 +0x165
cmd/compile/internal/types2.(*Checker).verifyVersionf(0xc00052e800, {0xf54880, 0xc000480730}, {0xde9257, 0x6}, {0xde4b0b, 0x3}, {0x0, 0x0, 0x0})
    /w/xgo/src/cmd/compile/internal/types2/version.go:107 +0xd1
cmd/compile/internal/types2.(*Checker).builtin(0xc00052e800, 0xc000dd4500, 0xc0001912d0, 0xb)
    /w/xgo/src/cmd/compile/internal/types2/builtins.go:537 +0x4586
cmd/compile/internal/types2.(*Checker).callExpr(0xc00052e800, 0xc000dd4500, 0xc0001912d0)
    /w/xgo/src/cmd/compile/internal/types2/call.go:228 +0xba5
cmd/compile/internal/types2.(*Checker).exprInternal(0xc00052e800, 0x0, 0xc000dd4500, {0xf58bf0, 0xc0001912d0}, {0x0, 0x0})
    /w/xgo/src/cmd/compile/internal/types2/expr.go:1395 +0xe5
cmd/compile/internal/types2.(*Checker).rawExpr(0xc00052e800, 0x0, 0xc000dd4500, {0xf58bf0?, 0xc0001912d0?}, {0x0?, 0x0?}, 0x0)
    /w/xgo/src/cmd/compile/internal/types2/expr.go:994 +0x1c5
cmd/compile/internal/types2.(*Checker).multiExpr(...)
    /w/xgo/src/cmd/compile/internal/types2/expr.go:1585
cmd/compile/internal/types2.(*Checker).initVars(...)
    /w/xgo/src/cmd/compile/internal/types2/assignments.go:409
cmd/compile/internal/types2.(*Checker).stmt(0xc00052e800, 0x0, {0xf57170, 0xc0000dea80})
    /w/xgo/src/cmd/compile/internal/types2/stmt.go:514 +0x4685
cmd/compile/internal/types2.(*Checker).stmtList(...)
    /w/xgo/src/cmd/compile/internal/types2/stmt.go:120
cmd/compile/internal/types2.(*Checker).funcBody(0xc00052e800, 0xc0004806e0?, {0xc000013748?, 0xc00052e800?}, 0xc000c849c0, 0xc000482240, {0x0?, 0x0?})
    /w/xgo/src/cmd/compile/internal/types2/stmt.go:40 +0x36c
cmd/compile/internal/types2.(*Checker).funcDecl.func1()
    /w/xgo/src/cmd/compile/internal/types2/decl.go:769 +0x3a
cmd/compile/internal/types2.(*Checker).processDelayed(...)
    /w/xgo/src/cmd/compile/internal/types2/check.go:463
cmd/compile/internal/types2.(*Checker).checkFiles(0xc00052e800, {0xc000621280, 0x4, 0x4})
    /w/xgo/src/cmd/compile/internal/types2/check.go:407 +0x64a
cmd/compile/internal/types2.(*Checker).Files(...)
    /w/xgo/src/cmd/compile/internal/types2/check.go:368
cmd/compile/internal/types2.(*Config).Check(0xc00062ab40, {0x7ffcab86373f?, 0xc000012157?}, {0xc000621280, 0x4, 0x4}, 0xc00062aba0)
    /w/xgo/src/cmd/compile/internal/types2/api.go:470 +0x66
cmd/compile/internal/noder.checkFiles({0x0, {0x0, 0x0}}, {0xc0000dea60, 0x4, 0xf64288?})
    /w/xgo/src/cmd/compile/internal/noder/irgen.go:88 +0x5b8
cmd/compile/internal/noder.writePkgStub({0x0?, {0x0?, 0x0?}}, {0xc0000dea60, 0x4, 0x4})
    /w/xgo/src/cmd/compile/internal/noder/unified.go:296 +0x6a
cmd/compile/internal/noder.unified({0x0?, {0x0?, 0x0?}}, {0xc0000dea60?, 0xd33300?, 0x0?})
    /w/xgo/src/cmd/compile/internal/noder/unified.go:172 +0x9a
cmd/compile/internal/noder.LoadPackage({0xc000022240, 0x4, 0x4})
    /w/xgo/src/cmd/compile/internal/noder/noder.go:77 +0x43a
cmd/compile/internal/gc.Main(0xe25290)
    /w/xgo/src/cmd/compile/internal/gc/main.go:197 +0xbbd
main.main()
    /w/xgo/src/cmd/compile/main.go:57 +0xf9

go tool dist: FAILED: /w/xgo/bin/go install -tags=math_big_pure_go compiler_bootstrap purego bootstrap/cmd/...: exit status 1
$

This doesn't seem like intended behavior; if "min" is not allowed it would be better to have a more descriptive error.

Thanks

thanm commented 7 months ago

@mdempsky

cuonglm commented 7 months ago

CL https://go-review.googlesource.com/c/go/+/534755 added the code to report mismatched version set by //go:build.

It seems to me that the error returned by types2 posBase := terr.Pos.Base() doesn't have the proper pos base?

I'm not sure this is intended, but sounds like the compiler should do it defensively by checking that file != nil to prevent this case when it does not appear in posBaseMap map.

cc @griesemer

gopherbot commented 7 months ago

Change https://go.dev/cl/569718 mentions this issue: cmd/compile: prevent nil dereference when using result from posBaseMap