From 34804b1db3c90a092d5f990f6feb9b80c1191c04 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 8 Oct 2018 21:01:00 -0400 Subject: [PATCH] go/analysis/passes/unsafeptr: split out from vet Change-Id: Ic5f41b62b4539709f83d9a960f0ab6b87dd2092c Reviewed-on: https://go-review.googlesource.com/c/140758 Reviewed-by: Michael Matloob Run-TryBot: Michael Matloob --- .../testdata/src/a/a.go} | 24 +++---- .../passes/{vet => unsafeptr}/unsafeptr.go | 67 ++++++++++++------- .../passes/unsafeptr/unsafeptr_test.go | 13 ++++ 3 files changed, 69 insertions(+), 35 deletions(-) rename go/analysis/passes/{vet/testdata/unsafeptr.go => unsafeptr/testdata/src/a/a.go} (55%) rename go/analysis/passes/{vet => unsafeptr}/unsafeptr.go (56%) create mode 100644 go/analysis/passes/unsafeptr/unsafeptr_test.go diff --git a/go/analysis/passes/vet/testdata/unsafeptr.go b/go/analysis/passes/unsafeptr/testdata/src/a/a.go similarity index 55% rename from go/analysis/passes/vet/testdata/unsafeptr.go rename to go/analysis/passes/unsafeptr/testdata/src/a/a.go index ce852009..f1f61035 100644 --- a/go/analysis/passes/vet/testdata/unsafeptr.go +++ b/go/analysis/passes/unsafeptr/testdata/src/a/a.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package testdata +package a import ( "reflect" @@ -12,18 +12,18 @@ import ( func f() { var x unsafe.Pointer var y uintptr - x = unsafe.Pointer(y) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(y) // want "possible misuse of unsafe.Pointer" y = uintptr(x) // only allowed pointer arithmetic is ptr +/-/&^ num. // num+ptr is technically okay but still flagged: write ptr+num instead. x = unsafe.Pointer(uintptr(x) + 1) - x = unsafe.Pointer(1 + uintptr(x)) // ERROR "possible misuse of unsafe.Pointer" - x = unsafe.Pointer(uintptr(x) + uintptr(x)) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(1 + uintptr(x)) // want "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(uintptr(x) + uintptr(x)) // want "possible misuse of unsafe.Pointer" x = unsafe.Pointer(uintptr(x) - 1) - x = unsafe.Pointer(1 - uintptr(x)) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(1 - uintptr(x)) // want "possible misuse of unsafe.Pointer" x = unsafe.Pointer(uintptr(x) &^ 3) - x = unsafe.Pointer(1 &^ uintptr(x)) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(1 &^ uintptr(x)) // want "possible misuse of unsafe.Pointer" // certain uses of reflect are okay var v reflect.Value @@ -34,18 +34,18 @@ func f() { var s2 *reflect.SliceHeader x = unsafe.Pointer(s2.Data) var s3 reflect.StringHeader - x = unsafe.Pointer(s3.Data) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(s3.Data) // want "possible misuse of unsafe.Pointer" var s4 reflect.SliceHeader - x = unsafe.Pointer(s4.Data) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(s4.Data) // want "possible misuse of unsafe.Pointer" // but only in reflect var vv V - x = unsafe.Pointer(vv.Pointer()) // ERROR "possible misuse of unsafe.Pointer" - x = unsafe.Pointer(vv.UnsafeAddr()) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(vv.Pointer()) // want "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(vv.UnsafeAddr()) // want "possible misuse of unsafe.Pointer" var ss1 *StringHeader - x = unsafe.Pointer(ss1.Data) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(ss1.Data) // want "possible misuse of unsafe.Pointer" var ss2 *SliceHeader - x = unsafe.Pointer(ss2.Data) // ERROR "possible misuse of unsafe.Pointer" + x = unsafe.Pointer(ss2.Data) // want "possible misuse of unsafe.Pointer" } diff --git a/go/analysis/passes/vet/unsafeptr.go b/go/analysis/passes/unsafeptr/unsafeptr.go similarity index 56% rename from go/analysis/passes/vet/unsafeptr.go rename to go/analysis/passes/unsafeptr/unsafeptr.go index 1bf32599..aa5518ec 100644 --- a/go/analysis/passes/vet/unsafeptr.go +++ b/go/analysis/passes/unsafeptr/unsafeptr.go @@ -1,34 +1,44 @@ -// +build ignore - // Copyright 2014 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 uintptr -> unsafe.Pointer conversions. - -package main +package unsafeptr import ( "go/ast" "go/token" "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" ) -func init() { - register("unsafeptr", - "check for misuse of unsafe.Pointer", - checkUnsafePointer, - callExpr) +var Analyzer = &analysis.Analyzer{ + Name: "unsafeptr", + Doc: "check for invalid conversions of uintptr to unsafe.Pointer", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, } -func checkUnsafePointer(f *File, node ast.Node) { - x := node.(*ast.CallExpr) - if len(x.Args) != 1 { - return - } - if f.hasBasicType(x.Fun, types.UnsafePointer) && f.hasBasicType(x.Args[0], types.Uintptr) && !f.isSafeUintptr(x.Args[0]) { - f.Badf(x.Pos(), "possible misuse of unsafe.Pointer") +func run(pass *analysis.Pass) (interface{}, error) { + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), } + inspect.Preorder(nodeFilter, func(n ast.Node) { + x := n.(*ast.CallExpr) + if len(x.Args) != 1 { + return + } + if hasBasicType(pass.TypesInfo, x.Fun, types.UnsafePointer) && + hasBasicType(pass.TypesInfo, x.Args[0], types.Uintptr) && + !isSafeUintptr(pass.TypesInfo, x.Args[0]) { + pass.Reportf(x.Pos(), "possible misuse of unsafe.Pointer") + } + }) + return nil, nil } // isSafeUintptr reports whether x - already known to be a uintptr - @@ -36,10 +46,10 @@ func checkUnsafePointer(f *File, node ast.Node) { // directly from an unsafe.Pointer via conversion and pointer arithmetic // or if x is the result of reflect.Value.Pointer or reflect.Value.UnsafeAddr // or obtained from the Data field of a *reflect.SliceHeader or *reflect.StringHeader. -func (f *File) isSafeUintptr(x ast.Expr) bool { +func isSafeUintptr(info *types.Info, x ast.Expr) bool { switch x := x.(type) { case *ast.ParenExpr: - return f.isSafeUintptr(x.X) + return isSafeUintptr(info, x.X) case *ast.SelectorExpr: switch x.Sel.Name { @@ -56,7 +66,7 @@ func (f *File) isSafeUintptr(x ast.Expr) bool { // by the time we get to the conversion at the end. // For now approximate by saying that *Header is okay // but Header is not. - pt, ok := f.pkg.types[x.X].Type.(*types.Pointer) + pt, ok := info.Types[x.X].Type.(*types.Pointer) if ok { t, ok := pt.Elem().(*types.Named) if ok && t.Obj().Pkg().Path() == "reflect" { @@ -78,7 +88,7 @@ func (f *File) isSafeUintptr(x ast.Expr) bool { } switch sel.Sel.Name { case "Pointer", "UnsafeAddr": - t, ok := f.pkg.types[sel.X].Type.(*types.Named) + t, ok := info.Types[sel.X].Type.(*types.Named) if ok && t.Obj().Pkg().Path() == "reflect" && t.Obj().Name() == "Value" { return true } @@ -86,14 +96,25 @@ func (f *File) isSafeUintptr(x ast.Expr) bool { case 1: // maybe conversion of uintptr to unsafe.Pointer - return f.hasBasicType(x.Fun, types.Uintptr) && f.hasBasicType(x.Args[0], types.UnsafePointer) + return hasBasicType(info, x.Fun, types.Uintptr) && + hasBasicType(info, x.Args[0], types.UnsafePointer) } case *ast.BinaryExpr: switch x.Op { case token.ADD, token.SUB, token.AND_NOT: - return f.isSafeUintptr(x.X) && !f.isSafeUintptr(x.Y) + return isSafeUintptr(info, x.X) && !isSafeUintptr(info, x.Y) } } return false } + +// hasBasicType reports whether x's type is a types.Basic with the given kind. +func hasBasicType(info *types.Info, x ast.Expr, kind types.BasicKind) bool { + t := info.Types[x].Type + if t != nil { + t = t.Underlying() + } + b, ok := t.(*types.Basic) + return ok && b.Kind() == kind +} diff --git a/go/analysis/passes/unsafeptr/unsafeptr_test.go b/go/analysis/passes/unsafeptr/unsafeptr_test.go new file mode 100644 index 00000000..9d82e99f --- /dev/null +++ b/go/analysis/passes/unsafeptr/unsafeptr_test.go @@ -0,0 +1,13 @@ +package unsafeptr_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "golang.org/x/tools/go/analysis/passes/unsafeptr" +) + +func TestFromFileSystem(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, unsafeptr.Analyzer, "a") +}