code review 4962051: cgo: support for mingw 4.5.1 and newer

cgo: support for mingw-w64 4.5.1 and newer

[vcc, 2011-09-04 03:57:22]

Hello golang-dev@googlegroups.com,

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

[rsc, 2011-09-05 11:46:16]

Can you write this without the #ifdef?
You are assuming that 6l (or 8l) and the
object file being read were compiled
with the same compiler.  That may not be true.

I would suggest to first look for __imp__ and
then __imp_.

Russ

[jp, 2011-09-06 06:47:09]

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

http://codereview.appspot.com/4962051/diff/4001/src/cmd/ld/ldpe.c#newcode415
src/cmd/ld/ldpe.c:415: #if defined(_WIN64) && __GNUC__ >= 4 && __GNUC_MINOR__ >=
5 && __GNUC_PATCHLEVEL__ >= 1 //mingw-w64 4.5.1 and newer don't add underscore
#if defined(_WIN64) && (__GNUC__*10000+__GNUC_MINOR__*100+__GNUC_PATCHLEVEL__)
>= 40501

http://codereview.appspot.com/4962051/diff/4001/src/pkg/runtime/cgo/amd64.S
File src/pkg/runtime/cgo/amd64.S (right):

http://codereview.appspot.com/4962051/diff/4001/src/pkg/runtime/cgo/amd64.S#n...
src/pkg/runtime/cgo/amd64.S:9: #if defined(__APPLE__) || (defined(_WIN32) &&
__GNUC__ <= 4 && __GNUC_MINOR__ <= 5 && __GNUC_PATCHLEVEL__ < 1)
#if defined(__APPLE__) || (defined(_WIN32) &&
(__GNUC__*10000+__GNUC_MINOR__*100+__GNUC_PATCHLEVEL__) < 40501)

your test is not correct with gcc-4.4.3

[vcc, 2011-09-07 14:44:20]

I would suggest only support mingw-w64-4.5.1 and newer, since Win64
don't add underscore prefixes for C function names, code would be
simpler.

2011/9/5 Russ Cox <rsc@golang.org>:
> Can you write this without the #ifdef?
> You are assuming that 6l (or 8l) and the
> object file being read were compiled
> with the same compiler.  That may not be true.
>
> I would suggest to first look for __imp__ and
> then __imp_.
>
> Russ
>

[jp, 2011-09-09 13:31:39]

Does anybody use a version older than mingw-w64-4.5.1 ?

On 2011/09/07 14:44:20, vcc wrote:
> I would suggest only support mingw-w64-4.5.1 and newer, since Win64
> don't add underscore prefixes for C function names, code would be
> simpler.
> 
> 2011/9/5 Russ Cox <rsc@golang.org>:
> > Can you write this without the #ifdef?
> > You are assuming that 6l (or 8l) and the
> > object file being read were compiled
> > with the same compiler. &nbsp;That may not be true.
> >
> > I would suggest to first look for __imp__ and
> > then __imp_.
> >
> > Russ
> >

[rsc, 2011-09-12 18:39:59]

On Fri, Sep 9, 2011 at 09:31,  <jp@webmaster.ms> wrote:
> Does anybody use a version older than mingw-w64-4.5.1 ?

Let's assume no.

[vcc, 2011-09-13 12:14:18]

PTAL

[hector, 2011-10-04 21:04:30]

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

http://codereview.appspot.com/4962051/diff/12001/src/cmd/ld/ldpe.c#newcode412
src/cmd/ld/ldpe.c:412: if(sym->sclass == IMAGE_SYM_CLASS_STATIC && sym->value ==
0 && sym->type == 0) // section
It looks like the code would read more clearly as the following:

if(sym->sclass == IMAGE_SYM_CLASS_STATIC && sym->value == 0 && sym->type == 0)
// section
	name = obj->sect[sym->sectnum-1].sym->name;
else {
	name = sym->name;
	if(strncmp(name, "__imp_", 6) == 0)
		name = &name[6]; // __imp_Name => Name
	if(thechar == '8' && name[0] == '_')
		name = &name[1]; // _Name => Name
}

[rsc, 2011-10-05 15:57:14]

LGTM once Hector is happy with it.

[hector, 2011-10-06 06:06:50]

LGTM, but will submit with the change suggested in my comment.

[hector, 2011-10-06 06:22:52]

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

cgo: support for mingw-w64 4.5.1 and newer

R=rsc, jp, hectorchu
CC=golang-dev
http://codereview.appspot.com/4962051

Committer: Hector Chu <hectorchu@gmail.com>

[hector, 2011-10-13 12:59:06]

Is there a way to force close this issue?

[rsc, 2011-10-13 13:47:21]

removing reviewers

[rsc, 2011-10-13 13:48:09]

On Oct 13, 2011 at 08:59, hectorchu@gmail.com wrote:
> Is there a way to force close this issue?

The issue will close once Wei runs 'hg sync'.
The submit moves all the reviewers to the CC line,
so it should not show up in your code review
dashboard.

Your replying to the mail re-added you as a
reviewer, but I just moved you back to the 
CC line.

Russ