code review 102040043: runtime: mark some C globals as having no pointers.

runtime: mark some C globals as having no pointers.

C globals are conservatively scanned.  This helps
avoid false retention, especially for 32 bit.

[khr, 2014-05-30 18:01:24]

Hello golang-codereviews@googlegroups.com,

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

[khr1, 2014-05-31 18:02:54]

Russ, could you take a quick look?



On Fri, May 30, 2014 at 11:01 AM, <khr@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: mark some C globals as having no pointers.
>
> C globals are conservatively scanned.  This helps
> avoid false retention, especially for 32 bit.
>
> Please review this at https://codereview.appspot.com/102040043/
>
> Affected files (+16, -0 lines):
>   M src/pkg/runtime/alg.goc
>   M src/pkg/runtime/heapdump.c
>   M src/pkg/runtime/malloc.goc
>   M src/pkg/runtime/msize.c
>   M src/pkg/runtime/os_darwin.c
>   M src/pkg/runtime/os_dragonfly.c
>   M src/pkg/runtime/os_freebsd.c
>   M src/pkg/runtime/os_netbsd.c
>   M src/pkg/runtime/os_openbsd.c
>   M src/pkg/runtime/os_solaris.c
>   M src/pkg/runtime/proc.c
>
>
> Index: src/pkg/runtime/alg.goc
> ===================================================================
> --- a/src/pkg/runtime/alg.goc
> +++ b/src/pkg/runtime/alg.goc
> @@ -465,6 +465,7 @@
>  // Runtime helpers.
>
>  // used in asm_{386,amd64}.s
> +#pragma dataflag NOPTR
>  byte runtime·aeskeysched[HashRandomBytes];
>
>  void
> Index: src/pkg/runtime/heapdump.c
> ===================================================================
> --- a/src/pkg/runtime/heapdump.c
> +++ b/src/pkg/runtime/heapdump.c
> @@ -17,6 +17,7 @@
>  #include "typekind.h"
>  #include "funcdata.h"
>  #include "zaexperiment.h"
> +#include "../../cmd/ld/textflag.h"
>
>  extern byte data[];
>  extern byte edata[];
> @@ -67,6 +68,7 @@
>  enum {
>         BufSize = 4096,
>  };
> +#pragma dataflag NOPTR
>  static byte buf[BufSize];
>  static uintptr nbuf;
>
> Index: src/pkg/runtime/malloc.goc
> ===================================================================
> --- a/src/pkg/runtime/malloc.goc
> +++ b/src/pkg/runtime/malloc.goc
> @@ -19,6 +19,7 @@
>  // Mark mheap as 'no pointers', it does not contain interesting pointers
> but occupies ~45K.
>  #pragma dataflag NOPTR
>  MHeap runtime·mheap;
> +#pragma dataflag NOPTR
>  MStats mstats;
>
>  int32  runtime·checking;
> Index: src/pkg/runtime/msize.c
> ===================================================================
> --- a/src/pkg/runtime/msize.c
> +++ b/src/pkg/runtime/msize.c
> @@ -28,8 +28,11 @@
>  #include "runtime.h"
>  #include "arch_GOARCH.h"
>  #include "malloc.h"
> +#include "../../cmd/ld/textflag.h"
>
> +#pragma dataflag NOPTR
>  int32 runtime·class_to_size[NumSizeClasses];
> +#pragma dataflag NOPTR
>  int32 runtime·class_to_allocnpages[NumSizeClasses];
>
>  // The SizeToClass lookup is implemented using two arrays,
> @@ -41,7 +44,9 @@
>  // size divided by 128 (rounded up).  The arrays are filled in
>  // by InitSizes.
>
> +#pragma dataflag NOPTR
>  int8 runtime·size_to_class8[1024/8 + 1];
> +#pragma dataflag NOPTR
>  int8 runtime·size_to_class128[(MaxSmallSize-1024)/128 + 1];
>
>  void runtime·testdefersizes(void);
> Index: src/pkg/runtime/os_darwin.c
> ===================================================================
> --- a/src/pkg/runtime/os_darwin.c
> +++ b/src/pkg/runtime/os_darwin.c
> @@ -59,6 +59,7 @@
>  void
>  runtime·get_random_data(byte **rnd, int32 *rnd_len)
>  {
> +       #pragma dataflag NOPTR
>         static byte urandom_data[HashRandomBytes];
>         int32 fd;
>         fd = runtime·open("/dev/urandom", 0 /* O_RDONLY */, 0);
> Index: src/pkg/runtime/os_dragonfly.c
> ===================================================================
> --- a/src/pkg/runtime/os_dragonfly.c
> +++ b/src/pkg/runtime/os_dragonfly.c
> @@ -122,6 +122,7 @@
>  void
>  runtime·get_random_data(byte **rnd, int32 *rnd_len)
>  {
> +       #pragma dataflag NOPTR
>         static byte urandom_data[HashRandomBytes];
>         int32 fd;
>         fd = runtime·open("/dev/urandom", 0 /* O_RDONLY */, 0);
> Index: src/pkg/runtime/os_freebsd.c
> ===================================================================
> --- a/src/pkg/runtime/os_freebsd.c
> +++ b/src/pkg/runtime/os_freebsd.c
> @@ -130,6 +130,7 @@
>  void
>  runtime·get_random_data(byte **rnd, int32 *rnd_len)
>  {
> +       #pragma dataflag NOPTR
>         static byte urandom_data[HashRandomBytes];
>         int32 fd;
>         fd = runtime·open("/dev/urandom", 0 /* O_RDONLY */, 0);
> Index: src/pkg/runtime/os_netbsd.c
> ===================================================================
> --- a/src/pkg/runtime/os_netbsd.c
> +++ b/src/pkg/runtime/os_netbsd.c
> @@ -188,6 +188,7 @@
>  void
>  runtime·get_random_data(byte **rnd, int32 *rnd_len)
>  {
> +       #pragma dataflag NOPTR
>         static byte urandom_data[HashRandomBytes];
>         int32 fd;
>         fd = runtime·open("/dev/urandom", 0 /* O_RDONLY */, 0);
> Index: src/pkg/runtime/os_openbsd.c
> ===================================================================
> --- a/src/pkg/runtime/os_openbsd.c
> +++ b/src/pkg/runtime/os_openbsd.c
> @@ -167,6 +167,7 @@
>  void
>  runtime·get_random_data(byte **rnd, int32 *rnd_len)
>  {
> +       #pragma dataflag NOPTR
>         static byte urandom_data[HashRandomBytes];
>         int32 fd;
>         fd = runtime·open("/dev/urandom", 0 /* O_RDONLY */, 0);
> Index: src/pkg/runtime/os_solaris.c
> ===================================================================
> --- a/src/pkg/runtime/os_solaris.c
> +++ b/src/pkg/runtime/os_solaris.c
> @@ -161,6 +161,7 @@
>  void
>  runtime·get_random_data(byte **rnd, int32 *rnd_len)
>  {
> +       #pragma dataflag NOPTR
>         static byte urandom_data[HashRandomBytes];
>         int32 fd;
>         fd = runtime·open("/dev/urandom", 0 /* O_RDONLY */, 0);
> Index: src/pkg/runtime/proc.c
> ===================================================================
> --- a/src/pkg/runtime/proc.c
> +++ b/src/pkg/runtime/proc.c
> @@ -2597,6 +2597,7 @@
>         uint32  syscalltick;
>         int64   syscallwhen;
>  };
> +#pragma dataflag NOPTR
>  static Pdesc pdesc[MaxGomaxprocs];
>
>  static uint32
>
>
>

[rsc, 2014-05-31 23:01:30]

LGTM

[rsc, 2014-05-31 23:20:34]

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

runtime: mark some C globals as having no pointers.

C globals are conservatively scanned.  This helps
avoid false retention, especially for 32 bit.

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

Committer: Russ Cox <rsc@golang.org>

[rsc, 2014-05-31 23:20:37]

I'm going to submit this to get another line on the dashboard.

[gobot, 2014-05-31 23:30:40]

This CL appears to have broken the windows-amd64 builder.
See http://build.golang.org/log/f323442f6ba7ee82b6e0d02b09e80a6c70cf1f4f

[rsc, 2014-06-01 13:50:49]

No, the previous CL did, just not all the time.