cmd/cgo, runtime: recognize unsafe.Pointer(&s[0]) in cgo pointer checks

It's fairly common to call cgo functions with conversions to
unsafe.Pointer or other C types.  Apply the simpler checking of address
expressions when possible when the address expression occurs within a
type conversion.

Change-Id: I5187d4eb4d27a6542621c396cad9ee4b8647d1cd
Reviewed-on: https://go-review.googlesource.com/18391
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/misc/cgo/errors/ptr.go b/misc/cgo/errors/ptr.go
index 153460152411d11a29ca272f1837c1110d3697f8..0dd291f5ed132e17d868208b3c104d24400d14de 100644 (file)
--- a/misc/cgo/errors/ptr.go
+++ b/misc/cgo/errors/ptr.go
@@ -106,6 +106,16 @@ var ptrTests = []ptrTest{
                body:    `i := 0; p := &S{p:&i, s:[]unsafe.Pointer{nil}}; C.f(&p.s[0])`,
                fail:    false,
        },
+       {
+               // Passing the address of a slice of an array that is
+               // an element in a struct, with a type conversion.
+               name:    "slice-ok-3",
+               c:       `void f(void* p) {}`,
+               imports: []string{"unsafe"},
+               support: `type S struct { p *int; a [4]byte }`,
+               body:    `i := 0; p := &S{p:&i}; s := p.a[:]; C.f(unsafe.Pointer(&s[0]))`,
+               fail:    false,
+       },
        {
                // Passing the address of a static variable with no
                // pointers doesn't matter.

diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index 750b89b5405d504e7a21584a65c4a2d5b4086a09..fb5049c1a1de596916edc9e0c554944ef69aea77 100644 (file)
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -626,9 +626,7 @@ func (p *Package) rewriteCall(f *File, call *ast.CallExpr, name *Name) {
 
                // Add optional additional arguments for an address
                // expression.
-               if u, ok := call.Args[i].(*ast.UnaryExpr); ok && u.Op == token.AND {
-                       c.Args = p.checkAddrArgs(f, c.Args, u.X)
-               }
+               c.Args = p.checkAddrArgs(f, c.Args, call.Args[i])
 
                // _cgoCheckPointer returns interface{}.
                // We need to type assert that to the type we want.
@@ -773,7 +771,19 @@ func (p *Package) hasPointer(f *File, t ast.Expr, top bool) bool {
 // only pass the slice or array if we can refer to it without side
 // effects.
 func (p *Package) checkAddrArgs(f *File, args []ast.Expr, x ast.Expr) []ast.Expr {
-       index, ok := x.(*ast.IndexExpr)
+       // Strip type conversions.
+       for {
+               c, ok := x.(*ast.CallExpr)
+               if !ok || len(c.Args) != 1 || !p.isType(c.Fun) {
+                       break
+               }
+               x = c.Args[0]
+       }
+       u, ok := x.(*ast.UnaryExpr)
+       if !ok || u.Op != token.AND {
+               return args
+       }
+       index, ok := u.X.(*ast.IndexExpr)
        if !ok {
                // This is the address of something that is not an
                // index expression.  We only need to examine the
@@ -804,6 +814,42 @@ func (p *Package) hasSideEffects(f *File, x ast.Expr) bool {
        return found
 }
 
+// isType returns whether the expression is definitely a type.
+// This is conservative--it returns false for an unknown identifier.
+func (p *Package) isType(t ast.Expr) bool {
+       switch t := t.(type) {
+       case *ast.SelectorExpr:
+               if t.Sel.Name != "Pointer" {
+                       return false
+               }
+               id, ok := t.X.(*ast.Ident)
+               if !ok {
+                       return false
+               }
+               return id.Name == "unsafe"
+       case *ast.Ident:
+               // TODO: This ignores shadowing.
+               switch t.Name {
+               case "unsafe.Pointer", "bool", "byte",
+                       "complex64", "complex128",
+                       "error",
+                       "float32", "float64",
+                       "int", "int8", "int16", "int32", "int64",
+                       "rune", "string",
+                       "uint", "uint8", "uint16", "uint32", "uint64", "uintptr":
+
+                       return true
+               }
+       case *ast.StarExpr:
+               return p.isType(t.X)
+       case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType,
+               *ast.MapType, *ast.ChanType:
+
+               return true
+       }
+       return false
+}
+
 // unsafeCheckPointerName is given the Go version of a C type.  If the
 // type uses unsafe.Pointer, we arrange to build a version of
 // _cgoCheckPointer that returns that type.  This avoids using a type
@@ -832,6 +878,8 @@ func (p *Package) unsafeCheckPointerName(t ast.Expr) string {
 func (p *Package) hasUnsafePointer(t ast.Expr) bool {
        switch t := t.(type) {
        case *ast.Ident:
+               // We don't see a SelectorExpr for unsafe.Pointer;
+               // this is created by code in this file.
                return t.Name == "unsafe.Pointer"
        case *ast.ArrayType:
                return p.hasUnsafePointer(t.Elt)

diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go
index 17799fc9851537632d61923145536dc63d326c45..9710c418b22041d16186e40634305f038e28f6f6 100644 (file)
--- a/src/runtime/cgocall.go
+++ b/src/runtime/cgocall.go
@@ -354,7 +354,7 @@ func cgoCheckPointer(ptr interface{}, args ...interface{}) interface{} {
        t := ep._type
 
        top := true
-       if len(args) > 0 && t.kind&kindMask == kindPtr {
+       if len(args) > 0 && (t.kind&kindMask == kindPtr || t.kind&kindMask == kindUnsafePointer) {
                p := ep.data
                if t.kind&kindDirectIface == 0 {
                        p = *(*unsafe.Pointer)(p)
@@ -365,6 +365,10 @@ func cgoCheckPointer(ptr interface{}, args ...interface{}) interface{} {
                aep := (*eface)(unsafe.Pointer(&args[0]))
                switch aep._type.kind & kindMask {
                case kindBool:
+                       if t.kind&kindMask == kindUnsafePointer {
+                               // We don't know the type of the element.
+                               break
+                       }
                        pt := (*ptrtype)(unsafe.Pointer(t))
                        cgoCheckArg(pt.elem, p, true, false, cgoCheckPointerFail)
                        return ptr