code review 7094061: pkg/runtime: avoid defining the same variable in more t...

runtime: avoid defining the same variable in more than one translation unit
For gccgo runtime and Darwin where -fno-common is the default.

[minux1, 2013-01-21 03:52:35]

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

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

[dave_cheney.net, 2013-01-21 03:58:36]

Don't we already pass -fno-common on all platforms in cmd/dist/build.c ?

On 21/01/2013, at 14:52, minux.ma@gmail.com wrote:

> Reviewers: iant,
> 
> Message:
> Hello iant@golang.org (cc: golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/
> 
> 
> Description:
> pkg/runtime: avoid defining the same variable in more than one
> translation unit
> For gccgo runtime and Darwin where -fno-common is the default.
> 
> Please review this at https://codereview.appspot.com/7094061/
> 
> Affected files:
>  M src/pkg/runtime/malloc.goc
>  M src/pkg/runtime/malloc.h
>  M src/pkg/runtime/proc.c
>  M src/pkg/runtime/runtime.h
> 
> 
> Index: src/pkg/runtime/malloc.goc
> ===================================================================
> --- a/src/pkg/runtime/malloc.goc
> +++ b/src/pkg/runtime/malloc.goc
> @@ -19,6 +19,8 @@
> #pragma dataflag 16 /* mark mheap as 'no pointers', hiding from garbage
collector */
> MHeap runtime·mheap;
> 
> +int32    runtime·checking;
> +
> extern MStats mstats;    // defined in zruntime_def_$GOOS_$GOARCH.go
> 
> extern volatile intgo runtime·MemProfileRate;
> Index: src/pkg/runtime/malloc.h
> ===================================================================
> --- a/src/pkg/runtime/malloc.h
> +++ b/src/pkg/runtime/malloc.h
> @@ -446,7 +446,7 @@
> void    runtime·checkallocated(void *v, uintptr n);
> void    runtime·markfreed(void *v, uintptr n);
> void    runtime·checkfreed(void *v, uintptr n);
> -int32    runtime·checking;
> +extern    int32    runtime·checking;
> void    runtime·markspan(void *v, uintptr size, uintptr n, bool leftover);
> void    runtime·unmarkspan(void *v, uintptr size);
> bool    runtime·blockspecial(void*);
> Index: src/pkg/runtime/proc.c
> ===================================================================
> --- a/src/pkg/runtime/proc.c
> +++ b/src/pkg/runtime/proc.c
> @@ -24,6 +24,13 @@
> 
> int32    runtime·gcwaiting;
> 
> +G*    runtime·allg;
> +G*    runtime·lastg;
> +M*    runtime·allm;
> +
> +int8*    runtime·goos;
> +int32    runtime·ncpu;
> +
> // Go scheduler
> //
> // The go scheduler's job is to match ready-to-run goroutines (`g's)
> Index: src/pkg/runtime/runtime.h
> ===================================================================
> --- a/src/pkg/runtime/runtime.h
> +++ b/src/pkg/runtime/runtime.h
> @@ -562,15 +562,15 @@
>  */
> extern    String    runtime·emptystring;
> extern    uintptr runtime·zerobase;
> -G*    runtime·allg;
> -G*    runtime·lastg;
> -M*    runtime·allm;
> +extern    G*    runtime·allg;
> +extern    G*    runtime·lastg;
> +extern    M*    runtime·allm;
> extern    int32    runtime·gomaxprocs;
> extern    bool    runtime·singleproc;
> extern    uint32    runtime·panicking;
> extern    int32    runtime·gcwaiting;        // gc is waiting to run
> -int8*    runtime·goos;
> -int32    runtime·ncpu;
> +extern    int8*    runtime·goos;
> +extern    int32    runtime·ncpu;
> extern    bool    runtime·iscgo;
> extern    void    (*runtime·sysargs)(int32, uint8**);
> extern    uint32    runtime·maxstring;
> 
> 

[minux1, 2013-01-21 04:03:04]

On Mon, Jan 21, 2013 at 11:58 AM, Dave Cheney <dave@cheney.net> wrote:

> Don't we already pass -fno-common on all platforms in cmd/dist/build.c ?

This change is for gccgo's runtime where the runtime is compiled by gcc.

see https://codereview.appspot.com/7096063/ for relevant discussions.

[iant, 2013-01-25 23:51:06]

LGTM

Thanks.

[minux1, 2013-01-26 01:57:17]

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

runtime: avoid defining the same variable in more than one translation unit
For gccgo runtime and Darwin where -fno-common is the default.

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

[minux1, 2013-01-26 01:59:55]

Hi Ian, could you please merge this into gofrontend and gcc trunk if
possible?
or do you want me to submit CL to gofrontend?

[iant2, 2013-01-29 23:31:54]

On Fri, Jan 25, 2013 at 5:59 PM, minux <minux.ma@gmail.com> wrote:
> Hi Ian, could you please merge this into gofrontend and gcc trunk if
> possible?
> or do you want me to submit CL to gofrontend?

I've merged the current library into gofrontend.

Ian

[minux1, 2013-01-30 05:45:35]

On Wed, Jan 30, 2013 at 7:31 AM, Ian Lance Taylor <iant@google.com> wrote:

> On Fri, Jan 25, 2013 at 5:59 PM, minux <minux.ma@gmail.com> wrote:
> > Hi Ian, could you please merge this into gofrontend and gcc trunk if
> > possible?
> > or do you want me to submit CL to gofrontend?
>
> I've merged the current library into gofrontend.
>
Thank you very much!