cmd/cgo: check function argument/return types for bad C pointer types

We need to determine whether arguments to and return values from C
functions are "bad" typedef'd pointer types which need to be uintptr
on the Go side.

The type of those arguments are not specified explicitly. As a result,
we never look through the C declarations for the GetTypeID functions
associated with that type, and never realize that they are bad.
However, in another function in the same package there might be an
explicit reference. Then we end up with the declaration being uintptr
in one file and *struct{...} in another file. Badness ensues.

Fix this by doing a 2-pass algorithm. In the first pass, we run as
normal, but record all the argument and result types we see. In the
second pass, we include those argument types also when reading the C
types.

Fixes #24161

Change-Id: I8d727e73a2fbc88cb9d9899f8719ae405f59f753
Reviewed-on: https://go-review.googlesource.com/122575
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/misc/cgo/test/issue24161_darwin_test.go b/misc/cgo/test/issue24161_darwin_test.go
new file mode 100644 (file)
index 0000000..cb15b3c
--- /dev/null
+++ b/misc/cgo/test/issue24161_darwin_test.go
@@ -0,0 +1,19 @@
+// Copyright 2018 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 cgotest
+
+import (
+       "testing"
+
+       "./issue24161arg"
+       "./issue24161res"
+)
+
+func Test24161Arg(t *testing.T) {
+       issue24161arg.Test(t)
+}
+func Test24161Res(t *testing.T) {
+       issue24161res.Test(t)
+}

diff --git a/misc/cgo/test/issue24161arg/def.go b/misc/cgo/test/issue24161arg/def.go
new file mode 100644 (file)
index 0000000..d33479a
--- /dev/null
+++ b/misc/cgo/test/issue24161arg/def.go
@@ -0,0 +1,17 @@
+// Copyright 2018 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 darwin
+
+package issue24161arg
+
+/*
+#cgo LDFLAGS: -framework CoreFoundation
+#include <CoreFoundation/CoreFoundation.h>
+*/
+import "C"
+
+func test24161array() C.CFArrayRef {
+       return C.CFArrayCreate(0, nil, 0, nil)
+}

diff --git a/misc/cgo/test/issue24161arg/use.go b/misc/cgo/test/issue24161arg/use.go
new file mode 100644 (file)
index 0000000..3e74944
--- /dev/null
+++ b/misc/cgo/test/issue24161arg/use.go
@@ -0,0 +1,19 @@
+// Copyright 2018 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 darwin
+
+package issue24161arg
+
+/*
+#cgo LDFLAGS: -framework CoreFoundation
+#include <CoreFoundation/CoreFoundation.h>
+*/
+import "C"
+import "testing"
+
+func Test(t *testing.T) {
+       a := test24161array()
+       C.CFArrayCreateCopy(0, a)
+}

diff --git a/misc/cgo/test/issue24161res/restype.go b/misc/cgo/test/issue24161res/restype.go
new file mode 100644 (file)
index 0000000..e5719f2
--- /dev/null
+++ b/misc/cgo/test/issue24161res/restype.go
@@ -0,0 +1,23 @@
+// Copyright 2018 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 darwin
+
+package issue24161res
+
+/*
+#cgo LDFLAGS: -framework CoreFoundation
+#include <CoreFoundation/CoreFoundation.h>
+*/
+import "C"
+import (
+       "reflect"
+       "testing"
+)
+
+func Test(t *testing.T) {
+       if k := reflect.TypeOf(C.CFArrayCreate(0, nil, 0, nil)).Kind(); k != reflect.Uintptr {
+               t.Fatalf("bad kind %s\n", k)
+       }
+}

diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index 169894273bea10550b16a5a9d80dde1ea7eb0098..9f4a611f19138714e339a3c0b9409fb89b0e3dab 100644 (file)
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -167,6 +167,21 @@ func (p *Package) Translate(f *File) {
        needType := p.guessKinds(f)
        if len(needType) > 0 {
                p.loadDWARF(f, needType)
+               // If there are typedefs used as arguments, add those
+               // types to the list of types we're interested in, and
+               // try again.
+               if len(p.ArgTypedefs) > 0 {
+                       for _, a := range p.ArgTypedefs {
+                               f.Name[a] = &Name{
+                                       Go: a,
+                                       C:  a,
+                               }
+                       }
+                       needType := p.guessKinds(f)
+                       if len(needType) > 0 {
+                               p.loadDWARF(f, needType)
+                       }
+               }
        }
        if p.rewriteCalls(f) {
                // Add `import _cgo_unsafe "unsafe"` after the package statement.
@@ -615,6 +630,7 @@ func (p *Package) loadDWARF(f *File, names []*Name) {
                }
                conv.FinishType(pos)
        }
+       p.ArgTypedefs = conv.argTypedefs
 }
 
 // mangleName does name mangling to translate names
@@ -1696,6 +1712,9 @@ type typeConv struct {
 
        ptrSize int64
        intSize int64
+
+       // Typedefs used as argument types for C calls.
+       argTypedefs []string
 }
 
 var tagGen int
@@ -2103,6 +2122,10 @@ func (c *typeConv) Type(dtype dwarf.Type, pos token.Pos) *Type {
                        s := *sub
                        s.Go = c.uintptr
                        sub = &s
+                       // Make sure we update any previously computed type.
+                       if oldType := typedef[name.Name]; oldType != nil {
+                               oldType.Go = sub.Go
+                       }
                }
                t.Go = name
                if unionWithPointer[sub.Go] {
@@ -2252,6 +2275,9 @@ func (c *typeConv) FuncArg(dtype dwarf.Type, pos token.Pos) *Type {
                        C:     tr,
                }
        case *dwarf.TypedefType:
+               // Keep track of all the typedefs used as arguments.
+               c.argTypedefs = append(c.argTypedefs, dt.Name)
+
                // C has much more relaxed rules than Go for
                // implicit type conversions. When the parameter
                // is type T defined as *X, simulate a little of the
@@ -2308,6 +2334,9 @@ func (c *typeConv) FuncType(dtype *dwarf.FuncType, pos token.Pos) *FuncType {
                gr = []*ast.Field{{Type: c.goVoid}}
        } else if dtype.ReturnType != nil {
                r = c.Type(unqual(dtype.ReturnType), pos)
+               if dt, ok := dtype.ReturnType.(*dwarf.TypedefType); ok {
+                       c.argTypedefs = append(c.argTypedefs, dt.Name)
+               }
                gr = []*ast.Field{{Type: r.Go}}
        }
        return &FuncType{

diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go
index 09c0624adb62f05cddd0beec918ef624bc358370..84b1daadbead2f7f53161b65d3f9b201f18f1167 100644 (file)
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -46,6 +46,7 @@ type Package struct {
        GoFiles     []string // list of Go files
        GccFiles    []string // list of gcc output files
        Preamble    string   // collected preamble for _cgo_export.h
+       ArgTypedefs []string // typedefs used as arguments to or results of C functions
 }
 
 // A File collects information about a single Go input file.