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 <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>

Author: ianlancetaylor

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.

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)

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