[release-branch.go1.10] cmd/cgo: check function argument/return types for bad C point...

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.

Update #25036

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>
(cherry picked from commit 20803e0f52809fa6088285c1c87246642df2b62d)
Reviewed-on: https://go-review.googlesource.com/122818

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 2ebe9066992785348d33e386624bde2364e188a6..ca1a7d754e8c0ef0c1a4052db170d4d08ea46edf 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.
@@ -597,6 +612,7 @@ func (p *Package) loadDWARF(f *File, names []*Name) {
                }
                conv.FinishType(pos)
        }
+       p.ArgTypedefs = conv.argTypedefs
 }
 
 // mangleName does name mangling to translate names
@@ -1678,6 +1694,9 @@ type typeConv struct {
 
        ptrSize int64
        intSize int64
+
+       // Typedefs used as argument types for C calls.
+       argTypedefs []string
 }
 
 var tagGen int
@@ -2085,6 +2104,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] {
@@ -2234,6 +2257,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
@@ -2290,6 +2316,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 80502262a5605f2f1fd0cb2e943101b163a977bf..f2eeb99bd2d39574cc36363135f75b19089b34aa 100644 (file)
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -45,6 +45,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.