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

Jump to bottom

cmd/compile: regression in writebarrier pass #19179

Closed
dsnet opened this issue Feb 18, 2017 · 8 comments
Closed

cmd/compile: regression in writebarrier pass #19179

dsnet opened this issue Feb 18, 2017 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.9

Comments

@dsnet
Copy link
Member

dsnet commented Feb 18, 2017

c4ef597 caused a regression where the following coder no longer compiles:

type Foo struct{ A, B time.Duration }

func Bar(fs []Foo) string {
	ss := make([]string, 2*len(fs))
	for i, h := range fs {
		ss[2*i] = fmt.Sprintf("%v %v", h.A, h.B.Seconds())
		ss[2*i+1] = fmt.Sprintf("%v %v", h.A, h.B.Seconds())
	}
	return strings.Join(ss, ",")
}

It now panics with:

./main.go:16: internal compiler error: attempt to load unspilled value v53 = MOVQload <time.Duration> {h} [8] v2 v117

goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0x0)
	/home/rawr/Projects/go/src/runtime/debug/stack.go:24 +0x79
cmd/compile/internal/gc.Fatalf(0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/subr.go:175 +0x230
cmd/compile/internal/gc.(*ssaExport).Fatalf(0xdd0af5, 0x100d00000001, 0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/ssa.go:4929 +0x67
cmd/compile/internal/ssa.(*Config).Fatalf(0xc420452000, 0x100d00000001, 0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/config.go:345 +0x76
cmd/compile/internal/ssa.(*Func).Fatalf(0xc4204b4000, 0xaccaeb, 0x22, 0xc4204a4600, 0x1, 0x1)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/func.go:416 +0x72
cmd/compile/internal/ssa.(*regAllocState).allocValToReg(0xc4204d83c0, 0xc420453988, 0xffce, 0x100000001, 0xc400001233, 0xc4204551e8)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/regalloc.go:454 +0x58e
cmd/compile/internal/ssa.(*regAllocState).regalloc(0xc4204d83c0, 0xc4204b4000)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/regalloc.go:1188 +0x1731
cmd/compile/internal/ssa.regalloc(0xc4204b4000)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/regalloc.go:135 +0x62
cmd/compile/internal/ssa.Compile(0xc4204b4000)
	/home/rawr/Projects/go/src/cmd/compile/internal/ssa/compile.go:70 +0x2c4
cmd/compile/internal/gc.buildssa(0xc4200b9180, 0x0)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/ssa.go:173 +0x1060
cmd/compile/internal/gc.compile(0xc4200b9180)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/pgen.go:366 +0x2d0
cmd/compile/internal/gc.funccompile(0xc4200b9180)
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/dcl.go:1226 +0xdc
cmd/compile/internal/gc.Main()
	/home/rawr/Projects/go/src/cmd/compile/internal/gc/main.go:473 +0x202d
main.main()
	/home/rawr/Projects/go/src/cmd/compile/main.go:50 +0x101

\cc @cherrymui @dr2chase @josharian
@dsnet dsnet added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 18, 2017
@dsnet dsnet added this to the Go1.9 milestone Feb 18, 2017
@cherrymui cherrymui self-assigned this Feb 19, 2017
@gopherbot
Copy link

CL https://golang.org/cl/37250 mentions this issue.

@funny-falcon
Copy link
Contributor

Catched with same bit in code with defer.
Code looks like:

    func (bld *DbBuilder) Finish() error {
        defer bld.Unlink() // <- here is an error
        // ... other code
    }

Unfortunately, can't build small reproduce case.

@funny-falcon
Copy link
Contributor

Changeset 37250 doesn't fix for me at the moment.

go version devel +5bbe1ce Sat Feb 18 19:44:10 2017 -0500 linux/amd64
$ go build
./builder.go:239: internal compiler error: attempt to load unspilled value v1780 = Phi <int> v282 v449
....

@funny-falcon
Copy link
Contributor

funny-falcon commented Feb 21, 2017 •
edited

I reduced case to minimal. Looks like removing any line makes it compile

package dbfile

import (
	"encoding/binary"
)

type DbBuilder struct {
	arr        []int
}

func (bld *DbBuilder) Finish() error {
	defer bld.Finish()

	var hash []byte
	for _, ixw := range bld.arr {
		for {
			if ixw != 0 {
				return nil
			}
			ixw--
		insertOne:
			for {
				for i := 0; i < 1; i++ {
					if binary.LittleEndian.Uint16(hash[i:]) == 0 {
						break insertOne
					}
				}
			}
		}
	}

	return nil
}

@cherrymui
Copy link
Member

@funny-falcon Thanks for the repro! However, the related values are not touched in the writebarrier pass, and they are in the correct order at that point. Later the tighten pass got something wrong. So it's a different bug. I'll take a look.

@funny-falcon
Copy link
Contributor

@cherrymui panic("") could be changed to any non-empty line: ixw = 0, return nil or break - all still trigger error. But removing that line makes it compileable.

@cherrymui cherrymui mentioned this issue Feb 21, 2017
Closed
@cherrymui
Copy link
Member

@funny-falcon, I filed issue #19217, since it is a different bug, and even apply to Go 1.8.

@funny-falcon
Copy link
Contributor

@cherrymui, thank you

@golang golang locked and limited conversation to collaborators Feb 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
4 participants
@funny-falcon @dsnet @gopherbot @cherrymui