cmd/compile: incorrect assignment to uint64 via pointer converted to *uint16 (new in 1.7)

[kelseyfrancis]

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

go version go1.7 darwin/amd64 and go version go1.7 linux/amd64.

This issue is not present in go1.6.2.

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

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"

and

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

Assign a uint16 (or uint32) to a previously unused uint64 via converted pointer on a little-endian architecture. go run the following program:

package main

import (
    "fmt"
    "unsafe"
)

func main() {
    var x uint64
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)

    var y uint64
    fmt.Printf("%x\n", y)
    *(*uint16)(unsafe.Pointer(&y)) = 0xffee
    fmt.Printf("%x\n", y)
}

What did you expect to see?

ffee
0
ffee

What did you see instead?

c42000ffee
0
ffee

A test that passes with Go 1.6 but not with Go 1.7:

// +build amd64

package main

import (
    "fmt"
    "unsafe"
)

func ExampleUint16NoAccessBeforeAssignment() {
    var x uint64
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)
    // Output:
    // ffee
}

func ExampleUint16AccessBeforeAssignment() {
    var x uint64
    fmt.Printf("%x\n", x)
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)
    // Output:
    // 0
    // ffee
}

func ExampleUint32NoAccessBeforeAssignment() {
    var x uint64
    *(*uint32)(unsafe.Pointer(&x)) = 0xffeeddcc
    fmt.Printf("%x\n", x)
    // Output:
    // ffeeddcc
}

func ExampleUint32AccessBeforeAssignment() {
    var x uint64
    fmt.Printf("%x\n", x)
    *(*uint32)(unsafe.Pointer(&x)) = 0xffeeddcc
    fmt.Printf("%x\n", x)
    // Output:
    // 0
    // ffeeddcc
}

[dr2chase]

Unsafe code, not a compiler bug.

It would be a minor bit of a work to fix this, and a larger bit of work to fix this in a way that did not leave useless local-zeroing in the code for correctly-written unsafe code.

[mdempsky]

@dr2chase Can you clarify what's incorrect about @kelseyfrancis's code? It looks valid to me.

[bradfitz]

@mdempsky:

    var x uint64
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee

That's treating a uint64 as a uint16.

[mdempsky]

Right, I see that. What's invalid about that? I don't recall package unsafe ever saying type punning like that isn't allowed in Go.

[bradfitz]

Package unsafe has also never promised it is valid. As @randall77 has said elsewhere, we're allowed to change the unsafe behavior between releases because because it makes no promises.

https://golang.org/doc/go1compat even says:

Use of package unsafe. Packages that import unsafe may depend on internal properties of the Go implementation. We reserve the right to make changes to the implementation that may break such programs.

[kelseyfrancis]

Fair enough. Incidentally, explicit initialization of x produces the same result:

    var x uint64 = 0x1020304050607080
    *(*uint16)(unsafe.Pointer(&x)) = 0xffee
    fmt.Printf("%x\n", x)  // prints c42000ffee rather than 102030405060ffee

[mdempsky]

https://golang.org/pkg/unsafe/#Pointer says:

(1) Conversion of a *T1 to Pointer to *T2.

Provided that T2 is no larger than T1 and that the two share an equivalent memory layout, this conversion allows reinterpreting data of one type as data of another type.

My interpretation of "equivalent memory layout" is referring to having the same pointer slots. Since uint16 is no larger than uint64 and neither contain pointers, it seems explicitly documented to be valid.

If not, we should clarify the documentation to define "memory layout". That term isn't used in the Go language spec or memory model.

[mdempsky]

Reopening because at the very least I think there's a documentation issue here.

[randall77]

We should fix this. The failure mode is bad - I believe we're exposing uninitialized stack memory this way. That's a (very minor, but still) security bug.

[dr2chase]

Workaround is to assign the local to a global sink before the unsafe use; this will force the zeroing to occur.

[dr2chase]

Just treat unsafe stores as not real stores, and thus no redundant store elimination?
Would only hurt code that ought not use unsafe anyway, since it has to be locals anyhow.

Keith, how can use of unsafe not be a security violation in general?
What prevents taking address of auto, casting to *byte[100000], and happily iterating over it?