From: Ian Lance Taylor Date: Wed, 18 Nov 2015 19:09:43 +0000 (-0800) Subject: cmd/vet: add some checks for invalid pointer passing using cgo X-Git-Tag: go1.6beta2~221 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ebf64bcc249539b6f66150ee98431301ab6ee679;p=gostls13.git cmd/vet: add some checks for invalid pointer passing using cgo Update #12416. Change-Id: I21d97cbe211ccc8048e5a78ea4d89664f4d195ba Reviewed-on: https://go-review.googlesource.com/17041 Reviewed-by: Russ Cox --- diff --git a/src/cmd/vet/cgo.go b/src/cmd/vet/cgo.go new file mode 100644 index 0000000000..8807952b48 --- /dev/null +++ b/src/cmd/vet/cgo.go @@ -0,0 +1,127 @@ +// Copyright 2015 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. + +// Check for invalid cgo pointer passing. +// This looks for code that uses cgo to call C code passing values +// whose types are almost always invalid according to the cgo pointer +// sharing rules. +// Specifically, it warns about attempts to pass a Go chan, map, func, +// or slice to C, either directly, or via a pointer, array, or struct. + +package main + +import ( + "go/ast" + "go/token" + "go/types" +) + +func init() { + register("cgocall", + "check for types that may not be passed to cgo calls", + checkCgoCall, + callExpr) +} + +func checkCgoCall(f *File, node ast.Node) { + x := node.(*ast.CallExpr) + + // We are only looking for calls to functions imported from + // the "C" package. + sel, ok := x.Fun.(*ast.SelectorExpr) + if !ok { + return + } + id, ok := sel.X.(*ast.Ident) + if !ok || id.Name != "C" { + return + } + + for _, arg := range x.Args { + if !typeOKForCgoCall(cgoBaseType(f, arg)) { + f.Badf(arg.Pos(), "possibly passing Go type with embedded pointer to C") + } + + // Check for passing the address of a bad type. + if conv, ok := arg.(*ast.CallExpr); ok && len(conv.Args) == 1 && f.hasBasicType(conv.Fun, types.UnsafePointer) { + arg = conv.Args[0] + } + if u, ok := arg.(*ast.UnaryExpr); ok && u.Op == token.AND { + if !typeOKForCgoCall(cgoBaseType(f, u.X)) { + f.Badf(arg.Pos(), "possibly passing Go type with embedded pointer to C") + } + } + } +} + +// cgoBaseType tries to look through type conversions involving +// unsafe.Pointer to find the real type. It converts: +// unsafe.Pointer(x) => x +// *(*unsafe.Pointer)(unsafe.Pointer(&x)) => x +func cgoBaseType(f *File, arg ast.Expr) types.Type { + switch arg := arg.(type) { + case *ast.CallExpr: + if len(arg.Args) == 1 && f.hasBasicType(arg.Fun, types.UnsafePointer) { + return cgoBaseType(f, arg.Args[0]) + } + case *ast.StarExpr: + call, ok := arg.X.(*ast.CallExpr) + if !ok || len(call.Args) != 1 { + break + } + // Here arg is *f(v). + t := f.pkg.types[call.Fun].Type + ptr, ok := t.Underlying().(*types.Pointer) + if !ok { + break + } + // Here arg is *(*p)(v) + elem, ok := ptr.Elem().Underlying().(*types.Basic) + if !ok || elem.Kind() != types.UnsafePointer { + break + } + // Here arg is *(*unsafe.Pointer)(v) + call, ok = call.Args[0].(*ast.CallExpr) + if !ok || len(call.Args) != 1 { + break + } + // Here arg is *(*unsafe.Pointer)(f(v)) + if !f.hasBasicType(call.Fun, types.UnsafePointer) { + break + } + // Here arg is *(*unsafe.Pointer)(unsafe.Pointer(v)) + u, ok := call.Args[0].(*ast.UnaryExpr) + if !ok || u.Op != token.AND { + break + } + // Here arg is *(*unsafe.Pointer)(unsafe.Pointer(&v)) + return cgoBaseType(f, u.X) + } + + return f.pkg.types[arg].Type +} + +// typeOKForCgoCall returns true if the type of arg is OK to pass to a +// C function using cgo. This is not true for Go types with embedded +// pointers. +func typeOKForCgoCall(t types.Type) bool { + if t == nil { + return true + } + switch t := t.Underlying().(type) { + case *types.Chan, *types.Map, *types.Signature, *types.Slice: + return false + case *types.Pointer: + return typeOKForCgoCall(t.Elem()) + case *types.Array: + return typeOKForCgoCall(t.Elem()) + case *types.Struct: + for i := 0; i < t.NumFields(); i++ { + if !typeOKForCgoCall(t.Field(i).Type()) { + return false + } + } + } + return true +} diff --git a/src/cmd/vet/doc.go b/src/cmd/vet/doc.go index 17e6f8aa05..53db6dde93 100644 --- a/src/cmd/vet/doc.go +++ b/src/cmd/vet/doc.go @@ -67,6 +67,12 @@ Flag: -buildtags Badly formed or misplaced +build tags. +Invalid uses of cgo + +Flag: -cgocall + +Detect some violations of the cgo pointer passing rules. + Unkeyed composite literals Flag: -composites diff --git a/src/cmd/vet/testdata/cgo.go b/src/cmd/vet/testdata/cgo.go new file mode 100644 index 0000000000..5ce6007fcb --- /dev/null +++ b/src/cmd/vet/testdata/cgo.go @@ -0,0 +1,54 @@ +// Copyright 2015 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. + +// This file contains tests for the cgo checker. + +package testdata + +// void f(void *); +import "C" + +import "unsafe" + +func CgoTests() { + var c chan bool + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&c))) // ERROR "embedded pointer" + C.f(unsafe.Pointer(&c)) // ERROR "embedded pointer" + + var m map[string]string + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&m))) // ERROR "embedded pointer" + C.f(unsafe.Pointer(&m)) // ERROR "embedded pointer" + + var f func() + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&f))) // ERROR "embedded pointer" + C.f(unsafe.Pointer(&f)) // ERROR "embedded pointer" + + var s []int + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&s))) // ERROR "embedded pointer" + C.f(unsafe.Pointer(&s)) // ERROR "embedded pointer" + + var a [1][]int + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&a))) // ERROR "embedded pointer" + C.f(unsafe.Pointer(&a)) // ERROR "embedded pointer" + + var st struct{ f []int } + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&st))) // ERROR "embedded pointer" + C.f(unsafe.Pointer(&st)) // ERROR "embedded pointer" + + // The following cases are OK. + var i int + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&i))) + C.f(unsafe.Pointer(&i)) + + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&s[0]))) + C.f(unsafe.Pointer(&s[0])) + + var a2 [1]int + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&a2))) + C.f(unsafe.Pointer(&a2)) + + var st2 struct{ i int } + C.f(*(*unsafe.Pointer)(unsafe.Pointer(&st2))) + C.f(unsafe.Pointer(&st2)) +}