code review 80850044: cmd/dist: zero output variables on entry to goc2c functions

cmd/dist: zero output variables on entry to goc2c functions

Zeroing the outputs makes sure that during function calls
in those functions we do not let the garbage collector
treat uninitialized values as pointers.

The garbage collector may still see uninitialized values
if a preemption occurs during the function prologue,
before the zeroing has had a chance to run.

This reduces the number of 'bad pointer' messages when
that runtime check is enabled, but it doesn't fix all of them,
so the check is still disabled.

It will also avoid leaks, although I doubt any of these were
particularly serious.

[rsc, 2014-03-26 23:13:09]

Hello iant, khr (cc: golang-codereviews@googlegroups.com),

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

[iant, 2014-03-26 23:38:57]

LGTM

[khr1, 2014-03-26 23:44:00]

LGTM.



On Wed, Mar 26, 2014 at 4:13 PM, <rsc@golang.org> wrote:

> Reviewers: iant, khr,
>
> Message:
> Hello iant, khr (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> cmd/dist: zero output variables on entry to goc2c functions
>
> Zeroing the outputs makes sure that during function calls
> in those functions we do not let the garbage collector
> treat uninitialized values as pointers.
>
> The garbage collector may still see uninitialized values
> if a preemption occurs during the function prologue,
> before the zeroing has had a chance to run.
>
> This reduces the number of 'bad pointer' messages when
> that runtime check is enabled, but it doesn't fix all of them,
> so the check is still disabled.
>
> It will also avoid leaks, although I doubt any of these were
> particularly serious.
>
> Please review this at https://codereview.appspot.com/80850044/
>
> Affected files (+19, -0 lines):
>   M src/cmd/dist/goc2c.c
>
>
> Index: src/cmd/dist/goc2c.c
> ===================================================================
> --- a/src/cmd/dist/goc2c.c
> +++ b/src/cmd/dist/goc2c.c
> @@ -524,6 +524,7 @@
>                      int paramwid, struct params *rets)
>  {
>         int first, n;
> +       struct params *p;
>
>         bwritef(output, "void\n");
>         if(!contains(name, "·"))
> @@ -546,6 +547,24 @@
>
>         write_params(rets, &first);
>         bwritef(output, ")\n{\n");
> +
> +       for (p = rets; p != nil; p = p->next) {
> +               if(streq(p->name, "..."))
> +                       continue;
> +               if(streq(p->type, "Slice"))
> +                       bwritef(output, "\t%s.array = 0;\n\t%s.len =
> 0;\n\t%s.cap = 0;\n", p->name, p->name, p->name);
> +               else if(streq(p->type, "String"))
> +                       bwritef(output, "\t%s.str = 0;\n\t%s.len = 0;\n",
> p->name, p->name);
> +               else if(streq(p->type, "Eface"))
> +                       bwritef(output, "\t%s.type = 0;\n\t%s.data =
> 0;\n", p->name, p->name);
> +               else if(streq(p->type, "Iface"))
> +                       bwritef(output, "\t%s.tab = 0;\n\t%s.data = 0;\n",
> p->name, p->name);
> +               else if(streq(p->type, "Complex128"))
> +                       bwritef(output, "\t%s.real = 0;\n\t%s.imag =
> 0;\n", p->name, p->name);
> +               else
> +                       bwritef(output, "\t%s = 0;\n", p->name);
> +               bwritef(output, "\tFLUSH(&%s);\n", p->name);
> +       }
>  }
>
>  /* Write a 6g function trailer.  */
>
>
>

[rsc, 2014-03-27 18:05:30]

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

cmd/dist: zero output variables on entry to goc2c functions

Zeroing the outputs makes sure that during function calls
in those functions we do not let the garbage collector
treat uninitialized values as pointers.

The garbage collector may still see uninitialized values
if a preemption occurs during the function prologue,
before the zeroing has had a chance to run.

This reduces the number of 'bad pointer' messages when
that runtime check is enabled, but it doesn't fix all of them,
so the check is still disabled.

It will also avoid leaks, although I doubt any of these were
particularly serious.

LGTM=iant, khr
R=iant, khr
CC=golang-codereviews
https://codereview.appspot.com/80850044