code review 5504108: gc: put limit on size of exported recursive interface

gc: put limit on size of exported recursive interface

Prevents edge-case recursive types from consuming excessive memory.

Fixes issue 1909.

[lstoakes, 2012-01-02 22:44:51]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg

[lstoakes, 2012-01-02 22:51:27]

Note I give a longer description of the patch/issue on the issue page
- http://code.google.com/p/go/issues/detail?id=1909.

This feels a little cargo cult/arbitrary/icky, but certainly resolves the issue.

As I discuss on the issue, I think it's indicative of a larger
underlying issue with recursive interface types. Unless I've missed an
issue, I don't think this has been broached yet, other than
http://code.google.com/p/go/issues/detail?id=1814 which discusses the
currently illegal type Foo interface { Foo }. I am happy to submit a
new issue for this larger problem.

On 2 January 2012 22:44,  <lstoakes@gmail.com> wrote:
> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg
>
>
> Description:
> gc: put limit on size of exported recursive interface
>
> Prevents edge-case recursive types from consuming excessive memory.
>
> Fixes issue 1909.
>
> Please review this at http://codereview.appspot.com/5504108/
>
> Affected files:
>  M src/cmd/gc/fmt.c
>
>
> Index: src/cmd/gc/fmt.c
> ===================================================================
> --- a/src/cmd/gc/fmt.c
> +++ b/src/cmd/gc/fmt.c
> @@ -1450,7 +1450,7 @@
>        if(t == T)
>                return fmtstrcpy(fp, "<T>");
>
> -       if(t->trecur > 4)
> +       if(t->trecur > 4 || fp->nfmt > 1000)
>                return fmtstrcpy(fp, "<...>");
>
>        t->trecur++;
>
>



-- 
Lorenzo Stoakes
http://www.codegrunt.co.uk

[lvd, 2012-01-04 07:01:17]

On Mon, Jan 2, 2012 at 23:51, Lorenzo Stoakes <lstoakes@gmail.com> wrote:
> Note I give a longer description of the patch/issue on the issue page
> - http://code.google.com/p/go/issues/detail?id=1909.
>
> This feels a little cargo cult/arbitrary/icky, but certainly resolves the
issue.
>
> As I discuss on the issue, I think it's indicative of a larger
> underlying issue with recursive interface types. Unless I've missed an
> issue, I don't think this has been broached yet, other than
> http://code.google.com/p/go/issues/detail?id=1814 which discusses the
> currently illegal type Foo interface { Foo }. I am happy to submit a
> new issue for this larger problem.

I intend to have a closer look at the underlying problem with
recursive interface types somewhere in the next week or two.   in the
meantime this would plaster over the symptom a bit, but i'd prefer it
if that stayed visible until a real fix was in.

i'm deferring to rsc on this CL

/L

[rsc, 2012-01-09 16:45:06]

LGTM

This does not fix issue 1909 but it does make the failure a little nicer.

[lvd, 2012-01-17 09:29:53]

On 2012/01/09 16:45:06, rsc wrote:
> LGTM
> 
> This does not fix issue 1909 but it does make the failure a little nicer.

meanwhile 1909 has been fixed