code review 11669043: [release-branch.go1.1] cmd/gc: fix issue with method wr...

[release-branch.go1.1] cmd/gc: fix issue with method wrappers not having escape analysis run on them.

««« CL 10383048 / 58e15340e78f
cmd/gc: fix issue with method wrappers not having escape analysis run on them.

Escape analysis needs the right curfn value on a dclfunc node, otherwise it will not analyze the function.
When generating method value wrappers, we forgot to set the curfn correctly.

Fixes issue 5753.

R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/10383048
»»»

Update issue 5928

[adg, 2013-07-22 08:44:39]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[dfc, 2013-07-22 10:14:06]

Just waiting for a few more builders to report in.

On Mon, Jul 22, 2013 at 6:44 PM,  <adg@golang.org> wrote:
> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> [release-branch.go1.1] cmd/gc: fix issue with method wrappers not having
> escape analysis run on them.
>
> ««« CL 10383048 / 58e15340e78f
> cmd/gc: fix issue with method wrappers not having escape analysis run on
> them.
>
> Escape analysis needs the right curfn value on a dclfunc node, otherwise
> it will not analyze the function.
> When generating method value wrappers, we forgot to set the curfn
> correctly.
>
> Fixes issue 5753.
>
> R=golang-dev, rsc
> CC=golang-dev
> https://codereview.appspot.com/10383048
> »»»
>
> Update issue 5928
>
> Please review this at https://codereview.appspot.com/11669043/
>
> Affected files:
>   M src/cmd/gc/closure.c
>   A test/fixedbugs/issue5753.go
>
>
> Index: src/cmd/gc/closure.c
> ===================================================================
> --- a/src/cmd/gc/closure.c
> +++ b/src/cmd/gc/closure.c
> @@ -280,7 +280,7 @@
>  static Node*
>  makepartialcall(Node *fn, Type *t0, Node *meth)
>  {
> -       Node *ptr, *n, *fld, *call, *xtype, *xfunc, *cv;
> +       Node *ptr, *n, *fld, *call, *xtype, *xfunc, *cv, *savecurfn;
>         Type *rcvrtype, *basetype, *t;
>         NodeList *body, *l, *callargs, *retargs;
>         char *p;
> @@ -304,6 +304,9 @@
>         if(sym->flags & SymUniq)
>                 return sym->def;
>         sym->flags |= SymUniq;
> +
> +       savecurfn = curfn;
> +       curfn = N;
>
>         xtype = nod(OTFUNC, N, N);
>         i = 0;
> @@ -311,6 +314,7 @@
>         callargs = nil;
>         ddd = 0;
>         xfunc = nod(ODCLFUNC, N, N);
> +       curfn = xfunc;
>         for(t = getinargx(t0)->type; t; t = t->down) {
>                 snprint(namebuf, sizeof namebuf, "a%d", i++);
>                 n = newname(lookup(namebuf));
> @@ -385,6 +389,7 @@
>         typecheck(&xfunc, Etop);
>         sym->def = xfunc;
>         xtop = list(xtop, xfunc);
> +       curfn = savecurfn;
>
>         return xfunc;
>  }
> Index: test/fixedbugs/issue5753.go
> ===================================================================
> new file mode 100644
> --- /dev/null
> +++ b/test/fixedbugs/issue5753.go
> @@ -0,0 +1,29 @@
> +// run
> +
> +// Copyright 2013 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 5753: bad typecheck info causes escape analysis to
> +// not run on method thunks.
> +
> +package main
> +
> +type Thing struct{}
> +
> +func (t *Thing) broken(s string) []string {
> +       foo := [1]string{s}
> +       return foo[:]
> +}
> +
> +func main() {
> +       t := &Thing{}
> +
> +       f := t.broken
> +       s := f("foo")
> +       _ = f("bar")
> +       if s[0] != "foo" {
> +               panic(`s[0] != "foo"`)
> +       }
> +
> +}
>
>
> --
>
> ---You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[dfc, 2013-07-22 10:54:19]

LGTM.

On Mon, Jul 22, 2013 at 8:14 PM, Dave Cheney <dave@cheney.net> wrote:
> Just waiting for a few more builders to report in.
>
> On Mon, Jul 22, 2013 at 6:44 PM,  <adg@golang.org> wrote:
>> Reviewers: golang-dev1,
>>
>> Message:
>> Hello golang-dev@googlegroups.com,
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go
>>
>>
>> Description:
>> [release-branch.go1.1] cmd/gc: fix issue with method wrappers not having
>> escape analysis run on them.
>>
>> ««« CL 10383048 / 58e15340e78f
>> cmd/gc: fix issue with method wrappers not having escape analysis run on
>> them.
>>
>> Escape analysis needs the right curfn value on a dclfunc node, otherwise
>> it will not analyze the function.
>> When generating method value wrappers, we forgot to set the curfn
>> correctly.
>>
>> Fixes issue 5753.
>>
>> R=golang-dev, rsc
>> CC=golang-dev
>> https://codereview.appspot.com/10383048
>> »»»
>>
>> Update issue 5928
>>
>> Please review this at https://codereview.appspot.com/11669043/
>>
>> Affected files:
>>   M src/cmd/gc/closure.c
>>   A test/fixedbugs/issue5753.go
>>
>>
>> Index: src/cmd/gc/closure.c
>> ===================================================================
>> --- a/src/cmd/gc/closure.c
>> +++ b/src/cmd/gc/closure.c
>> @@ -280,7 +280,7 @@
>>  static Node*
>>  makepartialcall(Node *fn, Type *t0, Node *meth)
>>  {
>> -       Node *ptr, *n, *fld, *call, *xtype, *xfunc, *cv;
>> +       Node *ptr, *n, *fld, *call, *xtype, *xfunc, *cv, *savecurfn;
>>         Type *rcvrtype, *basetype, *t;
>>         NodeList *body, *l, *callargs, *retargs;
>>         char *p;
>> @@ -304,6 +304,9 @@
>>         if(sym->flags & SymUniq)
>>                 return sym->def;
>>         sym->flags |= SymUniq;
>> +
>> +       savecurfn = curfn;
>> +       curfn = N;
>>
>>         xtype = nod(OTFUNC, N, N);
>>         i = 0;
>> @@ -311,6 +314,7 @@
>>         callargs = nil;
>>         ddd = 0;
>>         xfunc = nod(ODCLFUNC, N, N);
>> +       curfn = xfunc;
>>         for(t = getinargx(t0)->type; t; t = t->down) {
>>                 snprint(namebuf, sizeof namebuf, "a%d", i++);
>>                 n = newname(lookup(namebuf));
>> @@ -385,6 +389,7 @@
>>         typecheck(&xfunc, Etop);
>>         sym->def = xfunc;
>>         xtop = list(xtop, xfunc);
>> +       curfn = savecurfn;
>>
>>         return xfunc;
>>  }
>> Index: test/fixedbugs/issue5753.go
>> ===================================================================
>> new file mode 100644
>> --- /dev/null
>> +++ b/test/fixedbugs/issue5753.go
>> @@ -0,0 +1,29 @@
>> +// run
>> +
>> +// Copyright 2013 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 5753: bad typecheck info causes escape analysis to
>> +// not run on method thunks.
>> +
>> +package main
>> +
>> +type Thing struct{}
>> +
>> +func (t *Thing) broken(s string) []string {
>> +       foo := [1]string{s}
>> +       return foo[:]
>> +}
>> +
>> +func main() {
>> +       t := &Thing{}
>> +
>> +       f := t.broken
>> +       s := f("foo")
>> +       _ = f("bar")
>> +       if s[0] != "foo" {
>> +               panic(`s[0] != "foo"`)
>> +       }
>> +
>> +}
>>
>>
>> --
>>
>> ---You received this message because you are subscribed to the Google Groups
>> "golang-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-dev+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>>

[adg, 2013-07-22 13:45:31]

*** Submitted as https://code.google.com/p/go/source/detail?r=e503b72350b2 ***

[release-branch.go1.1] cmd/gc: fix issue with method wrappers not having escape
analysis run on them.

««« CL 10383048 / 58e15340e78f
cmd/gc: fix issue with method wrappers not having escape analysis run on them.

Escape analysis needs the right curfn value on a dclfunc node, otherwise it will
not analyze the function.
When generating method value wrappers, we forgot to set the curfn correctly.

Fixes issue 5753.

R=golang-dev, rsc
CC=golang-dev
https://codereview.appspot.com/10383048
»»»

Update issue 5928

R=golang-dev, dave
CC=golang-dev
https://codereview.appspot.com/11669043