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

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

cmd/compile: internal compiler error: panic during lower (part 2) #30257

ALTree opened this issue Feb 15, 2019 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@ALTree
Copy link
Member

ALTree commented Feb 15, 2019 •
edited 

$ 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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 15, 2019
@ALTree ALTree added this to the Go1.12 milestone Feb 15, 2019
@ALTree
Copy link
Member Author

ALTree commented Feb 15, 2019

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

@cherrymui
Copy link
Member

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
Copy link
Member

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
Copy link

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

@gopherbot
Copy link

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 pushed a commit to nebulabox/go that referenced this issue Feb 18, 2019
@cherrymui @nebulabox
cmd/compile: guard against loads with negative offset from readonly c…
1c7c8f7 
…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 pushed a commit to nebulabox/go that referenced this issue Feb 20, 2019
@cherrymui @nebulabox
cmd/compile: guard against loads with negative offset from readonly c…
42777b3 
…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
@cherrymui @ianlancetaylor
[release-branch.go1.12] cmd/compile: guard against loads with negativ…
a718f93 
…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>
@golang golang locked and limited conversation to collaborators Feb 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
3 participants
@ALTree @gopherbot @cherrymui