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: internal compiler error: panic during lower (part 2) #30257

Closed
ALTree opened this issue Feb 15, 2019 · 5 comments

Comments

Projects
None yet
3 participants
@ALTree
Copy link
Member

commented Feb 15, 2019

$ gotip version
go version devel +65c2069a9f Fri Feb 15 17:32:07 2019 +0000 linux/amd64

The following program:

package p

func f() {
	var i int
	var s string

	if true {
		s = "a"
	}

	if f := 0.0; -f < 0 {
		i = len(s[:4])
	}

	_ = s[i-1:0] != "bb" && true
}

Crashes the tip and the 1.12rc1 compilers, when built for GOARCH=arm64, as follows:

$ GOARCH=arm64 gotip tool compile crash.go 

crash.go:11:20: internal compiler error: 'f': panic during lower while compiling f:

runtime error: index out of range

goroutine 1 [running]:
cmd/compile/internal/ssa.Compile.func1(0xc0003c0908, 0xc0000ca580)
	/home/alberto/go/src/cmd/compile/internal/ssa/compile.go:45 +0xa5
panic(0xd64720, 0x1568110)
	/home/alberto/go/src/runtime/panic.go:522 +0x1b5
cmd/compile/internal/ssa.read8(...)

[...]

The crash appears to be the related to the one previously reported as #29215.

@ALTree ALTree added this to the Go1.12 milestone Feb 15, 2019

@ALTree

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

cc @randall77 for triaging and decision regarding the ReleaseBlocker label.

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Go 1.11 compiles it fine. So it is a regression. I think we should fix this in Go 1.12.

It panics in cmd/compile/internal/ssa.read8, which already guards off>=len:

// read8 reads one byte from the read-only global sym at offset off.
func read8(sym interface{}, off int64) uint8 {
        lsym := sym.(*obj.LSym)
        if off >= int64(len(lsym.P)) {
                // Invalid index into the global sym.
                // This can happen in dead code, so we don't want to panic.
                // Just return any value, it will eventually get ignored.
                // See issue 29215.
                return 0
        }
        return lsym.P[off]
}

So how can this happen? off is negative?

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Ok, it is loading s[-1] for s[i-1:0] != "bb", where i is constant 0. So off is indeed negative. So read8, etc. should guard against negative offset, too.

Thanks for finding this.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 15, 2019

Change https://golang.org/cl/162819 mentions this issue: cmd/compile: guard against loads with negative offset from readonly constants

@gopherbot gopherbot closed this in dca707b Feb 16, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Feb 16, 2019

Change https://golang.org/cl/162827 mentions this issue: [release-branch.go1.12] cmd/compile: guard against loads with negative offset from readonly constants

nebulabox added a commit to nebulabox/go that referenced this issue Feb 18, 2019

cmd/compile: guard against loads with negative offset from readonly c…
…onstants

CL 154057 adds guards agaist out-of-bound reads from readonly
constants. It turns out that in dead code, the offset can also
be negative. Guard against negative offset as well.

Fixes golang#30257.

Change-Id: I47c2a2e434dd466c08ae6f50f213999a358c796e
Reviewed-on: https://go-review.googlesource.com/c/162819
Reviewed-by: Keith Randall <khr@golang.org>

nebulabox added a commit to nebulabox/go that referenced this issue Feb 20, 2019

cmd/compile: guard against loads with negative offset from readonly c…
…onstants

CL 154057 adds guards agaist out-of-bound reads from readonly
constants. It turns out that in dead code, the offset can also
be negative. Guard against negative offset as well.

Fixes golang#30257.

Change-Id: I47c2a2e434dd466c08ae6f50f213999a358c796e
Reviewed-on: https://go-review.googlesource.com/c/162819
Reviewed-by: Keith Randall <khr@golang.org>

gopherbot pushed a commit that referenced this issue Feb 25, 2019

[release-branch.go1.12] cmd/compile: guard against loads with negativ…
…e offset from readonly constants

CL 154057 adds guards agaist out-of-bound reads from readonly
constants. It turns out that in dead code, the offset can also
be negative. Guard against negative offset as well.

Fixes #30257.

Change-Id: I47c2a2e434dd466c08ae6f50f213999a358c796e
Reviewed-on: https://go-review.googlesource.com/c/162819
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit dca707b)
Reviewed-on: https://go-review.googlesource.com/c/162827
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.