[release-branch.go1.4] cmd/gc: treat non-local vars inlined into wrapper as escaping

The compiler has a phase ordering problem.  Escape analysis runs
before wrapper generation.  When a generated wrapper calls a method
defined in a different package, if that call is inlined, there will be
no escape information for the variables defined in the inlined call.
Those variables will be placed on the stack, which fails if they
actually do escape.

There are probably various complex ways to fix this.  This is a simple
way to avoid it: when a generated wrapper calls a method defined in a
different package, treat all local variables as escaping.

Fixes #9537.

Change-Id: I530f39346de16ad173371c6c3f69cc189351a4e9
Reviewed-on: https://go-review.googlesource.com/3092
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit ec0ebc2281f79294c299ece35c5a690a6415e0e0)
Reviewed-on: https://go-review.googlesource.com/5003
Run-TryBot: Andrew Gerrand <adg@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

diff --git a/src/cmd/gc/go.h b/src/cmd/gc/go.h
index 965a0550d3683b64c097e8a6f07bb586d4180504..bbb883513a81d58ecbe57bc84d95506f92d597d3 100644 (file)
--- a/src/cmd/gc/go.h
+++ b/src/cmd/gc/go.h
@@ -974,6 +974,7 @@ EXTERN      int     funcdepth;
 EXTERN int     typecheckok;
 EXTERN int     compiling_runtime;
 EXTERN int     compiling_wrappers;
+EXTERN int     inl_nonlocal;
 EXTERN int     use_writebarrier;
 EXTERN int     pure_go;
 EXTERN char*   flag_installsuffix;

diff --git a/src/cmd/gc/inl.c b/src/cmd/gc/inl.c
index cf89b00902804fa301a0e810a44b2d9253f4c029..45e15bb9b7c92114c117994a3c5e23bec8f9482f 100644 (file)
--- a/src/cmd/gc/inl.c
+++ b/src/cmd/gc/inl.c
@@ -804,9 +804,12 @@ inlvar(Node *var)
        n->curfn = curfn;   // the calling function, not the called one
        n->addrtaken = var->addrtaken;
 
-       // esc pass wont run if we're inlining into a iface wrapper
-       // luckily, we can steal the results from the target func
-       if(var->esc == EscHeap)
+       // Esc pass wont run if we're inlining into a iface wrapper.
+       // Luckily, we can steal the results from the target func.
+       // If inlining a function defined in another package after
+       // escape analysis is done, treat all local vars as escaping.
+       // See issue 9537.
+       if(var->esc == EscHeap || (inl_nonlocal && var->op == ONAME))
                addrescapes(n);
 
        curfn->dcl = list(curfn->dcl, n);

diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index c3bc5af3b8d888c98bbf0f96980c279f02b111fa..26153d39b57d375ac49e4b7d7fc8258b69c979b8 100644 (file)
--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -2614,7 +2614,16 @@ genwrapper(Type *rcvr, Type *method, Sym *newnam, int iface)
                fn->dupok = 1;
        typecheck(&fn, Etop);
        typechecklist(fn->nbody, Etop);
+
+       // Set inl_nonlocal to whether we are calling a method on a
+       // type defined in a different package.  Checked in inlvar.
+       if(!methodrcvr->local)
+               inl_nonlocal = 1;
+
        inlcalls(fn);
+
+       inl_nonlocal = 0;
+
        curfn = nil;
        funccompile(fn, 0);
 }

diff --git a/test/fixedbugs/issue9537.dir/a.go b/test/fixedbugs/issue9537.dir/a.go
new file mode 100644 (file)
index 0000000..818c9eb
--- /dev/null
+++ b/test/fixedbugs/issue9537.dir/a.go
@@ -0,0 +1,25 @@
+// Copyright 2015 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 a
+
+type X struct {
+       T [32]byte
+}
+
+func (x *X) Get() []byte {
+       t := x.T
+       return t[:]
+}
+
+func (x *X) RetPtr(i int) *int {
+       i++
+       return &i
+}
+
+func (x *X) RetRPtr(i int) (r1 int, r2 *int) {
+       r1 = i + 1
+       r2 = &r1
+       return
+}

diff --git a/test/fixedbugs/issue9537.dir/b.go b/test/fixedbugs/issue9537.dir/b.go
new file mode 100644 (file)
index 0000000..52e64c8
--- /dev/null
+++ b/test/fixedbugs/issue9537.dir/b.go
@@ -0,0 +1,43 @@
+// Copyright 2015 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 (
+       "bytes"
+
+       "./a"
+)
+
+type X struct {
+       *a.X
+}
+
+type Intf interface {
+       Get()        []byte
+       RetPtr(int)  *int
+       RetRPtr(int) (int, *int)
+}
+
+func main() {
+       x := &a.X{T: [32]byte{1, 2, 3, 4}}
+       var ix Intf = X{x}
+       t1 := ix.Get()
+       t2 := x.Get()
+       if !bytes.Equal(t1, t2) {
+               panic(t1)
+       }
+
+       p1 := ix.RetPtr(5)
+       p2 := x.RetPtr(7)
+       if *p1 != 6 || *p2 != 8 {
+               panic(*p1)
+       }
+
+       r1, r2 := ix.RetRPtr(10)
+       r3, r4 := x.RetRPtr(13)
+       if r1 != 11 || *r2 != 11 || r3 != 14 || *r4 != 14 {
+               panic("bad RetRPtr")
+       }
+}

diff --git a/test/fixedbugs/issue9537.go b/test/fixedbugs/issue9537.go
new file mode 100644 (file)
index 0000000..ac2d41b
--- /dev/null
+++ b/test/fixedbugs/issue9537.go
@@ -0,0 +1,10 @@
+// rundir
+
+// Copyright 2015 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.
+
+// Issue 9537: Compiler does not run escape analysis on an inlined
+// generated method wrapper.
+
+package ignored