cmd/cgo: don't copy a simple variable x in &x[0]

Fixes #30065

Change-Id: I3d0fb03bab397548653d5f3b386cfe2980ac1030
Reviewed-on: https://go-review.googlesource.com/c/160830
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/misc/cgo/test/cgo_test.go b/misc/cgo/test/cgo_test.go
index 242ba6c0e5d01ffa3ddedef809683f858755572e..2cb93d9c2eb283555bbe486a65d177e021e9e0c0 100644 (file)
--- a/misc/cgo/test/cgo_test.go
+++ b/misc/cgo/test/cgo_test.go
@@ -94,6 +94,7 @@ func Test26066(t *testing.T)                 { test26066(t) }
 func Test26213(t *testing.T)                 { test26213(t) }
 func Test27660(t *testing.T)                 { test27660(t) }
 func Test28896(t *testing.T)                 { test28896(t) }
+func Test30065(t *testing.T)                 { test30065(t) }
 
 func BenchmarkCgoCall(b *testing.B)  { benchCgoCall(b) }
 func BenchmarkGoString(b *testing.B) { benchGoString(b) }

diff --git a/misc/cgo/test/issue30065.go b/misc/cgo/test/issue30065.go
new file mode 100644 (file)
index 0000000..396d437
--- /dev/null
+++ b/misc/cgo/test/issue30065.go
@@ -0,0 +1,38 @@
+// Copyright 2019 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.
+
+// Don't make a private copy of an array when taking the address of an
+// element.
+
+package cgotest
+
+// #include <string.h>
+import "C"
+
+import (
+       "testing"
+       "unsafe"
+)
+
+func test30065(t *testing.T) {
+       var a [256]byte
+       b := []byte("a")
+       C.memcpy(unsafe.Pointer(&a), unsafe.Pointer(&b[0]), 1)
+       if a[0] != 'a' {
+               t.Errorf("&a failed: got %c, want %c", a[0], 'a')
+       }
+
+       b = []byte("b")
+       C.memcpy(unsafe.Pointer(&a[0]), unsafe.Pointer(&b[0]), 1)
+       if a[0] != 'b' {
+               t.Errorf("&a[0] failed: got %c, want %c", a[0], 'b')
+       }
+
+       d := make([]byte, 256)
+       b = []byte("c")
+       C.memcpy(unsafe.Pointer(&d[0]), unsafe.Pointer(&b[0]), 1)
+       if d[0] != 'c' {
+               t.Errorf("&d[0] failed: got %c, want %c", d[0], 'c')
+       }
+}

diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index 65f9f6e4a19431e1cac60d6bbabcd798b9612d71..b5cf04cf4c7bd3e50daae03ea0ea451b143be955 100644 (file)
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -1121,14 +1121,19 @@ func (p *Package) mangle(f *File, arg *ast.Expr) (ast.Expr, bool) {
 }
 
 // checkIndex checks whether arg has the form &a[i], possibly inside
-// type conversions. If so, it writes
+// type conversions. If so, then in the general case it writes
 //    _cgoIndexNN := a
 //    _cgoNN := &cgoIndexNN[i] // with type conversions, if any
 // to sb, and writes
 //    _cgoCheckPointer(_cgoNN, _cgoIndexNN)
-// to sbCheck, and returns true. This tells _cgoCheckPointer to check
-// the complete contents of the slice or array being indexed, but no
-// other part of the memory allocation.
+// to sbCheck, and returns true. If a is a simple variable or field reference,
+// it writes
+//    _cgoIndexNN := &a
+// and dereferences the uses of _cgoIndexNN. Taking the address avoids
+// making a copy of an array.
+//
+// This tells _cgoCheckPointer to check the complete contents of the
+// slice or array being indexed, but no other part of the memory allocation.
 func (p *Package) checkIndex(sb, sbCheck *bytes.Buffer, arg ast.Expr, i int) bool {
        // Strip type conversions.
        x := arg
@@ -1148,13 +1153,23 @@ func (p *Package) checkIndex(sb, sbCheck *bytes.Buffer, arg ast.Expr, i int) boo
                return false
        }
 
-       fmt.Fprintf(sb, "_cgoIndex%d := %s; ", i, gofmtPos(index.X, index.X.Pos()))
+       addr := ""
+       deref := ""
+       if p.isVariable(index.X) {
+               addr = "&"
+               deref = "*"
+       }
+
+       fmt.Fprintf(sb, "_cgoIndex%d := %s%s; ", i, addr, gofmtPos(index.X, index.X.Pos()))
        origX := index.X
        index.X = ast.NewIdent(fmt.Sprintf("_cgoIndex%d", i))
+       if deref == "*" {
+               index.X = &ast.StarExpr{X: index.X}
+       }
        fmt.Fprintf(sb, "_cgo%d := %s; ", i, gofmtPos(arg, arg.Pos()))
        index.X = origX
 
-       fmt.Fprintf(sbCheck, "_cgoCheckPointer(_cgo%d, _cgoIndex%d); ", i, i)
+       fmt.Fprintf(sbCheck, "_cgoCheckPointer(_cgo%d, %s_cgoIndex%d); ", i, deref, i)
 
        return true
 }
@@ -1280,6 +1295,17 @@ func (p *Package) isConst(f *File, x ast.Expr) bool {
        return false
 }
 
+// isVariable reports whether x is a variable, possibly with field references.
+func (p *Package) isVariable(x ast.Expr) bool {
+       switch x := x.(type) {
+       case *ast.Ident:
+               return true
+       case *ast.SelectorExpr:
+               return p.isVariable(x.X)
+       }
+       return false
+}
+
 // rewriteUnsafe returns a version of t with references to unsafe.Pointer
 // rewritten to use _cgo_unsafe.Pointer instead.
 func (p *Package) rewriteUnsafe(t ast.Expr) ast.Expr {