Go: cmd/compile: access to negative slice indices improperly permitted

Created on 10 Oct 2019 · 13Comments · Source: golang/go

What version of Go are you using (`go version`)?

go version go1.13.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (`go env`)?

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/aaron/Library/Caches/go-build"
GOENV="/Users/aaron/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/aaron/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/n0/33wyl5yn1x77xsgjmz3p53zw0000gn/T/go-build007240201=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Accessed a negative slice index. Example: https://play.golang.org/p/Abpv3H1sgol

What did you expect to see?

Expected to see error

What did you see instead?

No error

FrozenDueToAge NeedsInvestigation release-blocker

Source

AaronReboot

👀11 👍4

Most helpful comment

@networkimprov yes, it can overwrite values on the stack.

$ cat x.go
package main

import (
    "fmt"
)

var n int

func main() {
    var a [5]int
    var b [5]int
    var c [5]int
    for i := -1; i <= 0; i-- {
        b[i] = i
        n++
        if n > 100 {
            break
        }
    }
    for i := range a {
        fmt.Println(a[i], b[i], c[i])
    }
}
0:1 /tmp $ go run x.go
0 0 -5
0 0 -4
0 0 -3
0 0 -2
0 0 -1

ericlagergren on 10 Oct 2019

👍4

All 13 comments

Interesting. This succeeds on 1.13.1 and master, but fails on 1.12.9. Sounds like a regression somewhere. Perhaps because of the recent improvements to the prove pass?

/cc @randall77 @rasky @zdjones

mvdan on 10 Oct 2019

Bisected to ddef157

marigonzes on 10 Oct 2019

👍2

Don't the trybots test for this?

networkimprov on 10 Oct 2019

@gopherbot Please open a backport to 1.13.

If this is indeed a compiler bug present in 1.13, we should backport it.

ianlancetaylor on 10 Oct 2019

Backport issue(s) opened: #34807 (for 1.13).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

gopherbot on 10 Oct 2019

Don't the trybots test for this?

Trybots only run tests that exist.

bradfitz on 10 Oct 2019

There are tests that check that negative indexes panic. Those tests just don't cover this case (induction variable which starts at a negative value).

randall77 on 10 Oct 2019

@randall77 could a negative index corrupt memory in this case?

networkimprov on 10 Oct 2019

I'll look into this in detail tomorrow.

zdjones on 10 Oct 2019

@networkimprov yes, it can overwrite values on the stack.

$ cat x.go
package main

import (
    "fmt"
)

var n int

func main() {
    var a [5]int
    var b [5]int
    var c [5]int
    for i := -1; i <= 0; i-- {
        b[i] = i
        n++
        if n > 100 {
            break
        }
    }
    for i := range a {
        fmt.Println(a[i], b[i], c[i])
    }
}
0:1 /tmp $ go run x.go
0 0 -5
0 0 -4
0 0 -3
0 0 -2
0 0 -1

ericlagergren on 10 Oct 2019

👍4

Found the issue, mailing the fix shortly.

zdjones on 11 Oct 2019

❤1

Change https://golang.org/cl/200759 mentions this issue: cmd/compile: make poset use sufficient conditions for orderedOrEqual

gopherbot on 11 Oct 2019

Change https://golang.org/cl/201060 mentions this issue: [release-branch.go1.13] cmd/compile: make poset use sufficient conditions for OrderedOrEqual

gopherbot on 14 Oct 2019

Was this page helpful?

0 / 5 - 0 ratings