x/tools/go/analysis/passes/unsafeptr: report Header misuse
This CL updates unsafeptr to report about *x and &x expressions where
the pointed-to variable has type reflect.SliceHeader or
reflect.StringHeader.
- Disallowing *x means that reflect.{Slice,String}Header.Data can only
be accessed using field selection via a *reflect.{Slice,String}Header
value.
- Disallowing &x means that a *reflect.{Slice,String}Header value can
only be created by converting from an unsafe.Pointer.
Well, almost only. There are still tricks that can be played to
workaround both of these. For example, a pointer can be dereferenced
via reflection, or a user could write a conversion like:
type T reflect.SliceHeader
_ = (*reflect.SliceHeader)(&T{})
But presumably this at least raises the bar enough that someone is
likely to pause to figure out the correct way to use
reflect.{Slice,String}Header.
Notably, disallowing *x and &x does *not* emit warnings for code that
uses reflect.{Slice,String}Header purely as values. For example, the
tests in internal/unsafeheader. Such code is arguably still a
violation of the unsafe.Pointer safety rules ("reflect.SliceHeader and
reflect.StringHeader should be used [...] never as plain structs"),
but is benign.
Updates golang/go#40701.
Change-Id: Id21996bfee07acc0d927a525797dca344bc804d8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/248192
Reviewed-by: Michael Matloob <matloob@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
diff --git a/go/analysis/passes/unsafeptr/testdata/src/a/issue40701.go b/go/analysis/passes/unsafeptr/testdata/src/a/issue40701.go
new file mode 100644
index 0000000..1f87697
--- /dev/null
+++ b/go/analysis/passes/unsafeptr/testdata/src/a/issue40701.go
@@ -0,0 +1,44 @@
+// Copyright 2020 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.
+
+package a
+
+import (
+ "reflect"
+ "unsafe"
+)
+
+// Explicitly allocating a variable of type reflect.SliceHeader.
+func _(p *byte, n int) []byte {
+ var sh reflect.SliceHeader
+ sh.Data = uintptr(unsafe.Pointer(p))
+ sh.Len = n
+ sh.Cap = n
+ return *(*[]byte)(unsafe.Pointer(&sh)) // want "possible misuse of reflect.SliceHeader"
+}
+
+// Implicitly allocating a variable of type reflect.SliceHeader.
+func _(p *byte, n int) []byte {
+ return *(*[]byte)(unsafe.Pointer(&reflect.SliceHeader{ // want "possible misuse of reflect.SliceHeader"
+ Data: uintptr(unsafe.Pointer(p)),
+ Len: n,
+ Cap: n,
+ }))
+}
+
+// Use reflect.StringHeader as a composite literal value.
+func _(p *byte, n int) []byte {
+ var res []byte
+ *(*reflect.StringHeader)(unsafe.Pointer(&res)) = reflect.StringHeader{ // want "possible misuse of reflect.StringHeader"
+ Data: uintptr(unsafe.Pointer(p)),
+ Len: n,
+ }
+ return res
+}
+
+func _() {
+ // don't crash when obj.Pkg() == nil
+ var err error
+ _ = &err
+}
diff --git a/go/analysis/passes/unsafeptr/unsafeptr.go b/go/analysis/passes/unsafeptr/unsafeptr.go
index 5594149..ed86e5e 100644
--- a/go/analysis/passes/unsafeptr/unsafeptr.go
+++ b/go/analysis/passes/unsafeptr/unsafeptr.go
@@ -37,16 +37,29 @@
nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
+ (*ast.StarExpr)(nil),
+ (*ast.UnaryExpr)(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.ReportRangef(x, "possible misuse of unsafe.Pointer")
+ switch x := n.(type) {
+ case *ast.CallExpr:
+ if len(x.Args) == 1 &&
+ hasBasicType(pass.TypesInfo, x.Fun, types.UnsafePointer) &&
+ hasBasicType(pass.TypesInfo, x.Args[0], types.Uintptr) &&
+ !isSafeUintptr(pass.TypesInfo, x.Args[0]) {
+ pass.ReportRangef(x, "possible misuse of unsafe.Pointer")
+ }
+ case *ast.StarExpr:
+ if t := pass.TypesInfo.Types[x].Type; isReflectHeader(t) {
+ pass.ReportRangef(x, "possible misuse of %s", t)
+ }
+ case *ast.UnaryExpr:
+ if x.Op != token.AND {
+ return
+ }
+ if t := pass.TypesInfo.Types[x.X].Type; isReflectHeader(t) {
+ pass.ReportRangef(x, "possible misuse of %s", t)
+ }
}
})
return nil, nil
@@ -144,7 +157,7 @@
// isReflectHeader reports whether t is reflect.SliceHeader or reflect.StringHeader.
func isReflectHeader(t types.Type) bool {
if named, ok := t.(*types.Named); ok {
- if obj := named.Obj(); obj.Pkg().Path() == "reflect" {
+ if obj := named.Obj(); obj.Pkg() != nil && obj.Pkg().Path() == "reflect" {
switch obj.Name() {
case "SliceHeader", "StringHeader":
return true
