]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/cgo: make JNI's jobject type map to uintptr in Go
authorKeith Randall <khr@golang.org>
Tue, 5 Dec 2017 05:29:38 +0000 (21:29 -0800)
committerKeith Randall <khr@golang.org>
Fri, 8 Dec 2017 16:13:14 +0000 (16:13 +0000)
The jobject type is declared as a pointer, but some JVMs
(Dalvik, ART) store non-pointer values in them. In Go, we must
use uintptr instead of a real pointer for these types.

This is similar to the CoreFoundation types on Darwin which
were "fixed" in CL 66332.

Update #22906
Update #21897

RELNOTE=yes

Change-Id: I0d4c664501d89a696c2fb037c995503caabf8911
Reviewed-on: https://go-review.googlesource.com/81876
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
misc/cgo/test/cgo_test.go
misc/cgo/test/test22906.go [new file with mode: 0644]
src/cmd/cgo/gcc.go
src/cmd/fix/cftype.go
src/cmd/fix/jnitype.go [new file with mode: 0644]
src/cmd/fix/jnitype_test.go [new file with mode: 0644]

index 67abfff2c033fda71966d3c90ffb13d94091f409..cfacb9c40d09f6f1d5c5d7da37e26673b8fc6984 100644 (file)
@@ -86,5 +86,6 @@ func Test21809(t *testing.T)                 { test21809(t) }
 func Test6907(t *testing.T)                  { test6907(t) }
 func Test6907Go(t *testing.T)                { test6907Go(t) }
 func Test21897(t *testing.T)                 { test21897(t) }
+func Test22906(t *testing.T)                 { test22906(t) }
 
 func BenchmarkCgoCall(b *testing.B) { benchCgoCall(b) }
diff --git a/misc/cgo/test/test22906.go b/misc/cgo/test/test22906.go
new file mode 100644 (file)
index 0000000..02bae9c
--- /dev/null
@@ -0,0 +1,74 @@
+// Copyright 2017 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.
+
+// +build cgo
+
+package cgotest
+
+/*
+
+// It's going to be hard to include a whole real JVM to test this.
+// So we'll simulate a really easy JVM using just the parts we need.
+
+// This is the relevant part of jni.h.
+
+struct _jobject;
+
+typedef struct _jobject *jobject;
+typedef jobject jclass;
+typedef jobject jthrowable;
+typedef jobject jstring;
+typedef jobject jarray;
+typedef jarray jbooleanArray;
+typedef jarray jbyteArray;
+typedef jarray jcharArray;
+typedef jarray jshortArray;
+typedef jarray jintArray;
+typedef jarray jlongArray;
+typedef jarray jfloatArray;
+typedef jarray jdoubleArray;
+typedef jarray jobjectArray;
+
+typedef jobject jweak;
+
+// Note: jvalue is already a non-pointer type due to it being a C union.
+
+*/
+import "C"
+import (
+       "testing"
+)
+
+func test22906(t *testing.T) {
+       var x1 C.jobject = 0 // Note: 0, not nil. That makes sure we use uintptr for these types.
+       _ = x1
+       var x2 C.jclass = 0
+       _ = x2
+       var x3 C.jthrowable = 0
+       _ = x3
+       var x4 C.jstring = 0
+       _ = x4
+       var x5 C.jarray = 0
+       _ = x5
+       var x6 C.jbooleanArray = 0
+       _ = x6
+       var x7 C.jbyteArray = 0
+       _ = x7
+       var x8 C.jcharArray = 0
+       _ = x8
+       var x9 C.jshortArray = 0
+       _ = x9
+       var x10 C.jintArray = 0
+       _ = x10
+       var x11 C.jlongArray = 0
+       _ = x11
+       var x12 C.jfloatArray = 0
+       _ = x12
+       var x13 C.jdoubleArray = 0
+       _ = x13
+       var x14 C.jobjectArray = 0
+       _ = x14
+       var x15 C.jweak = 0
+       _ = x15
+}
index 5cd6ac953c39935f1c17e0e83c92f10c178d56c7..bf5e3a927b7996ac9a266a4b98c9b4f10f4f99af 100644 (file)
@@ -2057,7 +2057,7 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type {
                name := c.Ident("_Ctype_" + dt.Name)
                goIdent[name.Name] = name
                sub := c.Type(dt.Type, pos)
-               if badPointerTypedef(dt.Name) {
+               if badPointerTypedef(dt) {
                        // Treat this typedef as a uintptr.
                        s := *sub
                        s.Go = c.uintptr
@@ -2223,7 +2223,7 @@ func (c *typeConv) FuncArg(dtype dwarf.Type, pos token.Pos) *Type {
                        }
                        // ...or the typedef is one in which we expect bad pointers.
                        // It will be a uintptr instead of *X.
-                       if badPointerTypedef(dt.Name) {
+                       if badPointerTypedef(dt) {
                                break
                        }
 
@@ -2571,13 +2571,23 @@ func fieldPrefix(fld []*ast.Field) string {
 // A typedef is bad if C code sometimes stores non-pointers in this type.
 // TODO: Currently our best solution is to find these manually and list them as
 // they come up. A better solution is desired.
-func badPointerTypedef(t string) bool {
-       // The real bad types are CFNumberRef and CFTypeRef.
+func badPointerTypedef(dt *dwarf.TypedefType) bool {
+       if badCFType(dt) {
+               return true
+       }
+       if badJNI(dt) {
+               return true
+       }
+       return false
+}
+
+func badCFType(dt *dwarf.TypedefType) bool {
+       // The real bad types are CFNumberRef and CFDateRef.
        // Sometimes non-pointers are stored in these types.
        // CFTypeRef is a supertype of those, so it can have bad pointers in it as well.
        // We return true for the other CF*Ref types just so casting between them is easier.
        // See comment below for details about the bad pointers.
-       return goos == "darwin" && strings.HasPrefix(t, "CF") && strings.HasSuffix(t, "Ref")
+       return goos == "darwin" && strings.HasPrefix(dt.Name, "CF") && strings.HasSuffix(dt.Name, "Ref")
 }
 
 // Comment from Darwin's CFInternal.h
@@ -2614,3 +2624,61 @@ enum {
     kCFTaggedObjectID_Undefined7 = (7 << 1) + 1,
 };
 */
+
+func badJNI(dt *dwarf.TypedefType) bool {
+       // In Dalvik and ART, the jobject type in the JNI interface of the JVM has the
+       // property that it is sometimes (always?) a small integer instead of a real pointer.
+       // Note: although only the android JVMs are bad in this respect, we declare the JNI types
+       // bad regardless of platform, so the same Go code compiles on both android and non-android.
+       if parent, ok := jniTypes[dt.Name]; ok {
+               // Try to make sure we're talking about a JNI type, not just some random user's
+               // type that happens to use the same name.
+               // C doesn't have the notion of a package, so it's hard to be certain.
+
+               // Walk up to jobject, checking each typedef on the way.
+               w := dt
+               for parent != "" {
+                       t, ok := w.Type.(*dwarf.TypedefType)
+                       if !ok || t.Name != parent {
+                               return false
+                       }
+                       w = t
+                       parent, ok = jniTypes[w.Name]
+                       if !ok {
+                               return false
+                       }
+               }
+
+               // Check that the typedef is:
+               //     struct _jobject;
+               //     typedef struct _jobject *jobject;
+               if ptr, ok := w.Type.(*dwarf.PtrType); ok {
+                       if str, ok := ptr.Type.(*dwarf.StructType); ok {
+                               if str.StructName == "_jobject" && str.Kind == "struct" && len(str.Field) == 0 && str.Incomplete {
+                                       return true
+                               }
+                       }
+               }
+       }
+       return false
+}
+
+// jniTypes maps from JNI types that we want to be uintptrs, to the underlying type to which
+// they are mapped.  The base "jobject" maps to the empty string.
+var jniTypes = map[string]string{
+       "jobject":       "",
+       "jclass":        "jobject",
+       "jthrowable":    "jobject",
+       "jstring":       "jobject",
+       "jarray":        "jobject",
+       "jbooleanArray": "jarray",
+       "jbyteArray":    "jarray",
+       "jcharArray":    "jarray",
+       "jshortArray":   "jarray",
+       "jintArray":     "jarray",
+       "jlongArray":    "jarray",
+       "jfloatArray":   "jarray",
+       "jdoubleArray":  "jarray",
+       "jobjectArray":  "jarray",
+       "jweak":         "jobject",
+}
index da1627fbfb4e6d04beab9ecfca336d056a8a663d..1f06cd6c3397495626f5c8c2f6558215539fb61c 100644 (file)
@@ -30,6 +30,13 @@ var cftypeFix = fix{
 // and similar for other CF*Ref types.
 // This fix finds nils initializing these types and replaces the nils with 0s.
 func cftypefix(f *ast.File) bool {
+       return typefix(f, func(s string) bool {
+               return strings.HasPrefix(s, "C.CF") && strings.HasSuffix(s, "Ref")
+       })
+}
+
+// typefix replaces nil with 0 for all nils whose type, when passed to badType, returns true.
+func typefix(f *ast.File, badType func(string) bool) bool {
        if !imports(f, "C") {
                return false
        }
@@ -39,7 +46,7 @@ func cftypefix(f *ast.File) bool {
        // Compute their replacement.
        badNils := map[interface{}]ast.Expr{}
        walk(f, func(n interface{}) {
-               if i, ok := n.(*ast.Ident); ok && i.Name == "nil" && badPointerType(typeof[n]) {
+               if i, ok := n.(*ast.Ident); ok && i.Name == "nil" && badType(typeof[n]) {
                        badNils[n] = &ast.BasicLit{ValuePos: i.NamePos, Kind: token.INT, Value: "0"}
                }
        })
@@ -87,7 +94,3 @@ func cftypefix(f *ast.File) bool {
 
        return true
 }
-
-func badPointerType(s string) bool {
-       return strings.HasPrefix(s, "C.CF") && strings.HasSuffix(s, "Ref")
-}
diff --git a/src/cmd/fix/jnitype.go b/src/cmd/fix/jnitype.go
new file mode 100644 (file)
index 0000000..29abe0f
--- /dev/null
@@ -0,0 +1,65 @@
+// Copyright 2017 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 main
+
+import (
+       "go/ast"
+)
+
+func init() {
+       register(jniFix)
+}
+
+var jniFix = fix{
+       name:     "jni",
+       date:     "2017-12-04",
+       f:        jnifix,
+       desc:     `Fixes initializers of JNI's jobject and subtypes`,
+       disabled: false,
+}
+
+// Old state:
+//   type jobject *_jobject
+// New state:
+//   type jobject uintptr
+// and similar for subtypes of jobject.
+// This fix finds nils initializing these types and replaces the nils with 0s.
+func jnifix(f *ast.File) bool {
+       return typefix(f, func(s string) bool {
+               switch s {
+               case "C.jobject":
+                       return true
+               case "C.jclass":
+                       return true
+               case "C.jthrowable":
+                       return true
+               case "C.jstring":
+                       return true
+               case "C.jarray":
+                       return true
+               case "C.jbooleanArray":
+                       return true
+               case "C.jbyteArray":
+                       return true
+               case "C.jcharArray":
+                       return true
+               case "C.jshortArray":
+                       return true
+               case "C.jintArray":
+                       return true
+               case "C.jlongArray":
+                       return true
+               case "C.jfloatArray":
+                       return true
+               case "C.jdoubleArray":
+                       return true
+               case "C.jobjectArray":
+                       return true
+               case "C.jweak":
+                       return true
+               }
+               return false
+       })
+}
diff --git a/src/cmd/fix/jnitype_test.go b/src/cmd/fix/jnitype_test.go
new file mode 100644 (file)
index 0000000..a6420f7
--- /dev/null
@@ -0,0 +1,185 @@
+// Copyright 2017 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 main
+
+func init() {
+       addTestCases(jniTests, jnifix)
+}
+
+var jniTests = []testCase{
+       {
+               Name: "jni.localVariable",
+               In: `package main
+
+import "C"
+
+func f() {
+       var x C.jobject = nil
+       x = nil
+       x, x = nil, nil
+}
+`,
+               Out: `package main
+
+import "C"
+
+func f() {
+       var x C.jobject = 0
+       x = 0
+       x, x = 0, 0
+}
+`,
+       },
+       {
+               Name: "jni.globalVariable",
+               In: `package main
+
+import "C"
+
+var x C.jobject = nil
+
+func f() {
+       x = nil
+}
+`,
+               Out: `package main
+
+import "C"
+
+var x C.jobject = 0
+
+func f() {
+       x = 0
+}
+`,
+       },
+       {
+               Name: "jni.EqualArgument",
+               In: `package main
+
+import "C"
+
+var x C.jobject
+var y = x == nil
+var z = x != nil
+`,
+               Out: `package main
+
+import "C"
+
+var x C.jobject
+var y = x == 0
+var z = x != 0
+`,
+       },
+       {
+               Name: "jni.StructField",
+               In: `package main
+
+import "C"
+
+type T struct {
+       x C.jobject
+}
+
+var t = T{x: nil}
+`,
+               Out: `package main
+
+import "C"
+
+type T struct {
+       x C.jobject
+}
+
+var t = T{x: 0}
+`,
+       },
+       {
+               Name: "jni.FunctionArgument",
+               In: `package main
+
+import "C"
+
+func f(x C.jobject) {
+}
+
+func g() {
+       f(nil)
+}
+`,
+               Out: `package main
+
+import "C"
+
+func f(x C.jobject) {
+}
+
+func g() {
+       f(0)
+}
+`,
+       },
+       {
+               Name: "jni.ArrayElement",
+               In: `package main
+
+import "C"
+
+var x = [3]C.jobject{nil, nil, nil}
+`,
+               Out: `package main
+
+import "C"
+
+var x = [3]C.jobject{0, 0, 0}
+`,
+       },
+       {
+               Name: "jni.SliceElement",
+               In: `package main
+
+import "C"
+
+var x = []C.jobject{nil, nil, nil}
+`,
+               Out: `package main
+
+import "C"
+
+var x = []C.jobject{0, 0, 0}
+`,
+       },
+       {
+               Name: "jni.MapKey",
+               In: `package main
+
+import "C"
+
+var x = map[C.jobject]int{nil: 0}
+`,
+               Out: `package main
+
+import "C"
+
+var x = map[C.jobject]int{0: 0}
+`,
+       },
+       {
+               Name: "jni.MapValue",
+               In: `package main
+
+import "C"
+
+var x = map[int]C.jobject{0: nil}
+`,
+               Out: `package main
+
+import "C"
+
+var x = map[int]C.jobject{0: 0}
+`,
+       },
+}