[release-branch.go1.10] 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.

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

6 files changed, 108 insertions, 0 deletions

diff --git a/misc/cgo/test/issue24161_darwin_test.go b/misc/cgo/test/issue24161_darwin_test.go
new file mode 100644
index 0000000000..cb15b3c5a0
--- /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
index 0000000000..d33479a891
--- /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
index 0000000000..3e74944013
--- /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
index 0000000000..e5719f22a4
--- /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 2ebe906699..ca1a7d754e 100644
--- 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 80502262a5..f2eeb99bd2 100644
--- 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.