mirror of
https://github.com/golang/go
synced 2024-09-04 23:44:16 +00:00
cmd/compile: handle unsafe.Pointer(f()) correctly
Previously statements like f(unsafe.Pointer(g()), int(h())) would be reordered into a sequence of statements like autotmp_g := g() autotmp_h := h() f(unsafe.Pointer(autotmp_g), int(autotmp_h)) which can leave g's temporary value on the stack as a uintptr, rather than an unsafe.Pointer. Instead, recognize uintptr-to-unsafe.Pointer conversions when reordering function calls to instead produce: autotmp_g := unsafe.Pointer(g()) autotmp_h := h() f(autotmp_g, int(autotmp_h)) Fixes #15329. Change-Id: I2cdbd89d233d0d5c94791513a9fd5fd958d11ed5 Reviewed-on: https://go-review.googlesource.com/22273 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
This commit is contained in:
parent
c6a5b3602a
commit
c65647d620
|
@ -1082,6 +1082,20 @@ func orderexpr(n *Node, order *Order, lhs *Node) *Node {
|
|||
n.Left = orderaddrtemp(n.Left, order)
|
||||
}
|
||||
|
||||
case OCONVNOP:
|
||||
if n.Type.IsKind(TUNSAFEPTR) && n.Left.Type.IsKind(TUINTPTR) && (n.Left.Op == OCALLFUNC || n.Left.Op == OCALLINTER || n.Left.Op == OCALLMETH) {
|
||||
// When reordering unsafe.Pointer(f()) into a separate
|
||||
// statement, the conversion and function call must stay
|
||||
// together. See golang.org/issue/15329.
|
||||
orderinit(n.Left, order)
|
||||
ordercall(n.Left, order)
|
||||
if lhs == nil || lhs.Op != ONAME || instrumenting {
|
||||
n = ordercopyexpr(n, n.Type, order, 0)
|
||||
}
|
||||
} else {
|
||||
n.Left = orderexpr(n.Left, order, nil)
|
||||
}
|
||||
|
||||
case OANDAND, OOROR:
|
||||
mark := marktemp(order)
|
||||
n.Left = orderexpr(n.Left, order, nil)
|
||||
|
|
79
test/fixedbugs/issue15329.go
Normal file
79
test/fixedbugs/issue15329.go
Normal file
|
@ -0,0 +1,79 @@
|
|||
// run
|
||||
|
||||
// Copyright 2016 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
// Previously, cmd/compile would rewrite
|
||||
//
|
||||
// check(unsafe.Pointer(testMeth(1).Pointer()), unsafe.Pointer(testMeth(2).Pointer()))
|
||||
//
|
||||
// to
|
||||
//
|
||||
// var autotmp_1 uintptr = testMeth(1).Pointer()
|
||||
// var autotmp_2 uintptr = testMeth(2).Pointer()
|
||||
// check(unsafe.Pointer(autotmp_1), unsafe.Pointer(autotmp_2))
|
||||
//
|
||||
// However, that means autotmp_1 is the only reference to the int
|
||||
// variable containing the value "1", but it's not a pointer type,
|
||||
// so it was at risk of being garbage collected by the evaluation of
|
||||
// testMeth(2).Pointer(), even though package unsafe's documentation
|
||||
// says the original code was allowed.
|
||||
//
|
||||
// Now cmd/compile rewrites it to
|
||||
//
|
||||
// var autotmp_1 unsafe.Pointer = unsafe.Pointer(testMeth(1).Pointer())
|
||||
// var autotmp_2 unsafe.Pointer = unsafe.Pointer(testMeth(2).Pointer())
|
||||
// check(autotmp_1, autotmp_2)
|
||||
//
|
||||
// to ensure the pointed-to variables are visible to the GC.
|
||||
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"reflect"
|
||||
"runtime"
|
||||
"unsafe"
|
||||
)
|
||||
|
||||
func main() {
|
||||
// Test all the different ways we can invoke reflect.Value.Pointer.
|
||||
|
||||
// Direct method invocation.
|
||||
check(unsafe.Pointer(testMeth(1).Pointer()), unsafe.Pointer(testMeth(2).Pointer()))
|
||||
|
||||
// Invocation via method expression.
|
||||
check(unsafe.Pointer(reflect.Value.Pointer(testMeth(1))), unsafe.Pointer(reflect.Value.Pointer(testMeth(2))))
|
||||
|
||||
// Invocation via interface.
|
||||
check(unsafe.Pointer(testInter(1).Pointer()), unsafe.Pointer(testInter(2).Pointer()))
|
||||
|
||||
// Invocation via method value.
|
||||
check(unsafe.Pointer(testFunc(1)()), unsafe.Pointer(testFunc(2)()))
|
||||
}
|
||||
|
||||
func check(p, q unsafe.Pointer) {
|
||||
a, b := *(*int)(p), *(*int)(q)
|
||||
if a != 1 || b != 2 {
|
||||
fmt.Printf("got %v, %v; expected 1, 2\n", a, b)
|
||||
}
|
||||
}
|
||||
|
||||
func testMeth(x int) reflect.Value {
|
||||
// Force GC to run.
|
||||
runtime.GC()
|
||||
return reflect.ValueOf(&x)
|
||||
}
|
||||
|
||||
type Pointerer interface {
|
||||
Pointer() uintptr
|
||||
}
|
||||
|
||||
func testInter(x int) Pointerer {
|
||||
return testMeth(x)
|
||||
}
|
||||
|
||||
func testFunc(x int) func() uintptr {
|
||||
return testMeth(x).Pointer
|
||||
}
|
Loading…
Reference in a new issue