cmd/fix: add intermediate cast for *C.CFTypeRef <-> *unsafe.Pointer

When casting between *C.CFTypeRef and *unsafe.Pointer, we used to be
able to do the cast directly. Now with C.CFTypeRef being a uintptr
instead of an unsafe.Pointer, we need an intermediate cast.

Add the insertion of the intermediate cast to the cftype fix module.

Fixes #23091

Change-Id: I891be2f4a08cfd7de1cc4c6ab841b1e0d8c388a6
Reviewed-on: https://go-review.googlesource.com/88175
Reviewed-by: Robert Griesemer <gri@golang.org>

2 files changed, 107 insertions, 26 deletions

diff --git a/src/cmd/fix/cftype.go b/src/cmd/fix/cftype.go
index 3341348272..df1cc18f9e 100644
--- a/src/cmd/fix/cftype.go
+++ b/src/cmd/fix/cftype.go
@@ -19,7 +19,7 @@ var cftypeFix = fix{
 	name:     "cftype",
 	date:     "2017-09-27",
 	f:        cftypefix,
-	desc:     `Fixes initializers of C.*Ref types`,
+	desc:     `Fixes initializers and casts of C.*Ref and JNI types`,
 	disabled: false,
 }
 
@@ -41,6 +41,7 @@ func typefix(f *ast.File, badType func(string) bool) bool {
 		return false
 	}
 	typeof, _ := typecheck(&TypeConfig{}, f)
+	changed := false
 
 	// step 1: Find all the nils with the offending types.
 	// Compute their replacement.
@@ -50,47 +51,93 @@ func typefix(f *ast.File, badType func(string) bool) bool {
 			badNils[n] = &ast.BasicLit{ValuePos: i.NamePos, Kind: token.INT, Value: "0"}
 		}
 	})
-	if len(badNils) == 0 {
-		return false
-	}
 
 	// step 2: find all uses of the bad nils, replace them with 0.
 	// There's no easy way to map from an ast.Expr to all the places that use them, so
 	// we use reflect to find all such references.
-	exprType := reflect.TypeOf((*ast.Expr)(nil)).Elem()
-	exprSliceType := reflect.TypeOf(([]ast.Expr)(nil))
+	if len(badNils) > 0 {
+		exprType := reflect.TypeOf((*ast.Expr)(nil)).Elem()
+		exprSliceType := reflect.TypeOf(([]ast.Expr)(nil))
+		walk(f, func(n interface{}) {
+			if n == nil {
+				return
+			}
+			v := reflect.ValueOf(n)
+			if v.Type().Kind() != reflect.Ptr {
+				return
+			}
+			if v.IsNil() {
+				return
+			}
+			v = v.Elem()
+			if v.Type().Kind() != reflect.Struct {
+				return
+			}
+			for i := 0; i < v.NumField(); i++ {
+				f := v.Field(i)
+				if f.Type() == exprType {
+					if r := badNils[f.Interface()]; r != nil {
+						f.Set(reflect.ValueOf(r))
+						changed = true
+					}
+				}
+				if f.Type() == exprSliceType {
+					for j := 0; j < f.Len(); j++ {
+						e := f.Index(j)
+						if r := badNils[e.Interface()]; r != nil {
+							e.Set(reflect.ValueOf(r))
+							changed = true
+						}
+					}
+				}
+			}
+		})
+	}
+
+	// step 3: fix up invalid casts.
+	// It used to be ok to cast between *unsafe.Pointer and *C.CFTypeRef in a single step.
+	// Now we need unsafe.Pointer as an intermediate cast.
+	// (*unsafe.Pointer)(x) where x is type *bad -> (*unsafe.Pointer)(unsafe.Pointer(x))
+	// (*bad.type)(x) where x is type *unsafe.Pointer -> (*bad.type)(unsafe.Pointer(x))
 	walk(f, func(n interface{}) {
 		if n == nil {
 			return
 		}
-		v := reflect.ValueOf(n)
-		if v.Type().Kind() != reflect.Ptr {
+		// Find pattern like (*a.b)(x)
+		c, ok := n.(*ast.CallExpr)
+		if !ok {
 			return
 		}
-		if v.IsNil() {
+		if len(c.Args) != 1 {
 			return
 		}
-		v = v.Elem()
-		if v.Type().Kind() != reflect.Struct {
+		p, ok := c.Fun.(*ast.ParenExpr)
+		if !ok {
 			return
 		}
-		for i := 0; i < v.NumField(); i++ {
-			f := v.Field(i)
-			if f.Type() == exprType {
-				if r := badNils[f.Interface()]; r != nil {
-					f.Set(reflect.ValueOf(r))
-				}
-			}
-			if f.Type() == exprSliceType {
-				for j := 0; j < f.Len(); j++ {
-					e := f.Index(j)
-					if r := badNils[e.Interface()]; r != nil {
-						e.Set(reflect.ValueOf(r))
-					}
-				}
+		s, ok := p.X.(*ast.StarExpr)
+		if !ok {
+			return
+		}
+		t := s.X.(*ast.SelectorExpr)
+		if !ok {
+			return
+		}
+		pkg, ok := t.X.(*ast.Ident)
+		if !ok {
+			return
+		}
+		dst := pkg.Name + "." + t.Sel.Name
+		src := typeof[c.Args[0]]
+		if badType(dst) && src == "*unsafe.Pointer" ||
+			dst == "unsafe.Pointer" && strings.HasPrefix(src, "*") && badType(src[1:]) {
+			c.Args[0] = &ast.CallExpr{
+				Fun:  &ast.SelectorExpr{X: &ast.Ident{Name: "unsafe"}, Sel: &ast.Ident{Name: "Pointer"}},
+				Args: []ast.Expr{c.Args[0]},
 			}
+			changed = true
 		}
 	})
 
-	return true
+	return changed
 }
diff --git a/src/cmd/fix/cftype_test.go b/src/cmd/fix/cftype_test.go
index adaed2114f..a18eb25261 100644
--- a/src/cmd/fix/cftype_test.go
+++ b/src/cmd/fix/cftype_test.go
@@ -182,4 +182,38 @@ import "C"
 var x = map[int]C.CFTypeRef{0: 0}
 `,
 	},
+	{
+		Name: "cftype.Conversion1",
+		In: `package main
+
+import "C"
+
+var x C.CFTypeRef
+var y = (*unsafe.Pointer)(&x)
+`,
+		Out: `package main
+
+import "C"
+
+var x C.CFTypeRef
+var y = (*unsafe.Pointer)(unsafe.Pointer(&x))
+`,
+	},
+	{
+		Name: "cftype.Conversion2",
+		In: `package main
+
+import "C"
+
+var x unsafe.Pointer
+var y = (*C.CFTypeRef)(&x)
+`,
+		Out: `package main
+
+import "C"
+
+var x unsafe.Pointer
+var y = (*C.CFTypeRef)(unsafe.Pointer(&x))
+`,
+	},
 }