code review 5334046: ld: fix .bss for ldpe.

ld: fix .bss for ldpe.

Fixes issue 2409.

[mattn, 2011-11-07 04:30:11]

http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c
File src/cmd/ld/ldpe.c (right):

http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c#newcode286
src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
probably, it should check whether the object file include main() or not?

[vcc, 2011-11-07 05:17:36]

2011/11/7  <mattn.jp@gmail.com>:
>
> http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c
> File src/cmd/ld/ldpe.c (right):
>
> http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c#newcode286
> src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
> probably, it should check whether the object file include main() or not?

Why? Can you show some example code?

>
> http://codereview.appspot.com/5334046/
>

[mattn, 2011-11-07 06:42:43]

try "gomake test" with chaning some lines of github.com/mattn/go-iconv.

HMODULE iconv_lib = NULL;
HMODULE msvcrt_lib = NULL;

to

static HMODULE iconv_lib = NULL;
static HMODULE msvcrt_lib = NULL;



On 2011/11/07 05:17:36, vcc wrote:
> 2011/11/7  <mattn.jp@gmail.com>:
> >
> > http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c
> > File src/cmd/ld/ldpe.c (right):
> >
> > http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c#newcode286
> > src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
> > probably, it should check whether the object file include main() or not?
> 
> Why? Can you show some example code?
> 
> >
> > http://codereview.appspot.com/5334046/
> >

[mattn, 2011-11-07 06:53:29]

Sorry.
It's not a problem about "third library or not".
It's a problem about "CGO or not"?


On 2011/11/07 06:42:43, mattn wrote:
> try "gomake test" with chaning some lines of github.com/mattn/go-iconv.
> 
> HMODULE iconv_lib = NULL;
> HMODULE msvcrt_lib = NULL;
> 
> to
> 
> static HMODULE iconv_lib = NULL;
> static HMODULE msvcrt_lib = NULL;
> 
> 
> 
> On 2011/11/07 05:17:36, vcc wrote:
> > 2011/11/7  <mattn.jp@gmail.com>:
> > >
> > > http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c
> > > File src/cmd/ld/ldpe.c (right):
> > >
> > >
http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c#newcode286
> > > src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
> > > probably, it should check whether the object file include main() or not?
> > 
> > Why? Can you show some example code?
> > 
> > >
> > > http://codereview.appspot.com/5334046/
> > >

[vcc, 2011-11-07 09:02:48]

hg uploaded, please try again, should fix this problem.

2011/11/7  <mattn.jp@gmail.com>:
> try "gomake test" with chaning some lines of github.com/mattn/go-iconv.
>
> HMODULE iconv_lib = NULL;
> HMODULE msvcrt_lib = NULL;
>
> to
>
> static HMODULE iconv_lib = NULL;
> static HMODULE msvcrt_lib = NULL;
>
>
>
> On 2011/11/07 05:17:36, vcc wrote:
>>
>> 2011/11/7  <mattn.jp@gmail.com>:
>> >
>> > http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c
>> > File src/cmd/ld/ldpe.c (right):
>> >
>> >
>
> http://codereview.appspot.com/5334046/diff/2001/src/cmd/ld/ldpe.c#newcode286
>>
>> > src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
>> > probably, it should check whether the object file include main() or
>
> not?
>
>> Why? Can you show some example code?
>
>> >
>> > http://codereview.appspot.com/5334046/
>> >
>
>
> http://codereview.appspot.com/5334046/
>

[mattn, 2011-11-07 09:13:07]

LGTM

[vcc, 2011-11-08 04:23:21]

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

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

[rsc, 2011-11-09 21:14:15]

LGTM

http://codereview.appspot.com/5334046/diff/11002/src/cmd/ld/ldpe.c
File src/cmd/ld/ldpe.c (right):

http://codereview.appspot.com/5334046/diff/11002/src/cmd/ld/ldpe.c#newcode286
src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
s/=/= /

[brainman, 2011-11-09 22:51:47]

Fixes issue 2409.

?

Sorry, I do not know enough to comment.

[vcc, 2011-11-10 04:26:16]

On 2011/11/09 22:51:47, brainman wrote:
> Fixes issue 2409.

 Yes, updated CL description.

[vcc, 2011-11-10 04:26:28]

PTAL

http://codereview.appspot.com/5334046/diff/11002/src/cmd/ld/ldpe.c
File src/cmd/ld/ldpe.c (right):

http://codereview.appspot.com/5334046/diff/11002/src/cmd/ld/ldpe.c#newcode286
src/cmd/ld/ldpe.c:286: rp->add =le32(rsect->base+rp->off);
On 2011/11/09 21:14:15, rsc wrote:
> s/=/= /

Done.

[rsc, 2011-11-11 19:28:39]

LGTM

[rsc, 2011-11-11 19:40:29]

*** Submitted as http://code.google.com/p/go/source/detail?r=75af77c0c7db ***

ld: fix .bss for ldpe.

Fixes issue 2409.

R=mattn.jp, rsc, alex.brainman
CC=golang-dev
http://codereview.appspot.com/5334046

Committer: Russ Cox <rsc@golang.org>