code review 5500094: misc/cgo/test: make tests run on windows

misc/cgo/test: make tests run on windows

- use proper Win64 gcc calling convention when
  calling initcgo on amd64
- increase g0 stack size to 64K on amd64 to make
  it the same as 386
- implement C.sleep
- do not use C.stat, since it is renamed to C._stat by mingw
- use fopen to implement TestErrno, since C.strtol
  always succeeds on windows
- skip TestSetEnv on windows, because os.Setenv
  sets windows process environment, while C.getenv
  inspects internal C runtime variable instead

[brainman, 2011-12-30 04:36:49]

Hello golang-dev@googlegroups.com,

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

[brainman, 2011-12-30 04:59:48]

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

Please take another look.

[vcc, 2012-01-05 03:06:44]

http://codereview.appspot.com/5500094/diff/5001/misc/cgo/test/callback_c.c
File misc/cgo/test/callback_c.c (right):

http://codereview.appspot.com/5500094/diff/5001/misc/cgo/test/callback_c.c#ne...
misc/cgo/test/callback_c.c:14: // with "__chkstk: not defined" during build
__chkstk in mingw's libgcc.a, extract and add it to CGO_OFILES, Makefile diff:
+ifeq ($(GOOS),windows)
+CGOFILES+=sleep_windows.go
+GCCVERSION=$(shell gcc -dumpversion)
+CGO_OFILES+=_chkstk.o
+
+_chkstk.o:
+ifeq ($(GOARCH),386)
+	ar -x "/mingw/lib/gcc/mingw32/$(GCCVERSION)/libgcc.a" _chkstk.o
+else
+	ar -x "/mingw/lib/gcc/x86_64-w64-mingw32/$(GCCVERSION)/libgcc.a" _chkstk.o
+endif	
+endif
+

[brainman, 2012-01-05 05:53:28]

http://codereview.appspot.com/5500094/diff/5001/misc/cgo/test/callback_c.c
File misc/cgo/test/callback_c.c (right):

http://codereview.appspot.com/5500094/diff/5001/misc/cgo/test/callback_c.c#ne...
misc/cgo/test/callback_c.c:14: // with "__chkstk: not defined" during build
Made this change:

diff -r 8e3c3ee24f77 misc/cgo/test/Makefile
--- a/misc/cgo/test/Makefile	Thu Jan 05 14:17:32 2012 +1100
+++ b/misc/cgo/test/Makefile	Thu Jan 05 16:47:55 2012 +1100
@@ -25,4 +25,21 @@
 OFILES=\
 	runtime.$O\
 
+ifeq ($(GOOS),windows)
+GCCVERSION=$(shell gcc -dumpversion)
+ifeq ($(GOARCH),386)
+GCCLIBDIR=/mingw/lib/gcc/mingw32/$(GCCVERSION)
+CHKSTK=_chkstk.o
+else
+GCCLIBDIR=/mingw/lib/gcc/x86_64-w64-mingw32/$(GCCVERSION)
+CHKSTK=_chkstk_ms.o
+endif
+
+CGOFILES+=sleep_windows.go
+CGO_OFILES+=$(CHKSTK)
+
+$(CHKSTK):
+	ar -x "$(GCCLIBDIR)/libgcc.a" $@
+endif
+
 include ../../../src/Make.pkg

and it works on 386. But test program crashing on amd64, hitting hardware
breakpoint while running cgotest.TestCallback:

	<ProblemSignatures>
		<EventType>BEX64</EventType>
		<Parameter0>6.out.exe</Parameter0>
		<Parameter1>0.0.0.0</Parameter1>
		<Parameter2>4f0534ae</Parameter2>
		<Parameter3>6.out.exe</Parameter3>
		<Parameter4>0.0.0.0</Parameter4>
		<Parameter5>4f0534ae</Parameter5>
		<Parameter6>00000000000182ca</Parameter6>
		<Parameter7>80000003</Parameter7>
		<Parameter8>00000000badc0de1</Parameter8>
	</ProblemSignatures>

[vcc, 2012-01-05 07:09:39]

2012/1/5  <alex.brainman@gmail.com>:
> ... and it works on 386. But test program crashing on amd64, hitting
> hardware breakpoint while running cgotest.TestCallback:

If disable gcc do -O2, amd64 crash even remove volatile, -O2 just make
callback function to one line:
$ objdump -d callback_c.o

callback_c.o:     file format pe-x86-64


Disassembly of section .text:

0000000000000000 <callback>:
   0:   e9 00 00 00 00          jmpq   5 <callback+0x5>
   5:   90                      nop



>
>        <ProblemSignatures>
>                <EventType>BEX64</EventType>
>                <Parameter0>6.out.exe</Parameter0>
>                <Parameter1>0.0.0.0</Parameter1>
>                <Parameter2>4f0534ae</Parameter2>
>                <Parameter3>6.out.exe</Parameter3>
>                <Parameter4>0.0.0.0</Parameter4>
>                <Parameter5>4f0534ae</Parameter5>
>                <Parameter6>00000000000182ca</Parameter6>
>                <Parameter7>80000003</Parameter7>
>                <Parameter8>00000000badc0de1</Parameter8>
>        </ProblemSignatures>
>
> http://codereview.appspot.com/5500094/

[vcc, 2012-01-05 07:29:23]

volatile char data[7*1024+320]; // OK
volatile char data[7*1024+321]; // crash

2012/1/5 Wei guangjing <vcc.163@gmail.com>:
> 2012/1/5  <alex.brainman@gmail.com>:
>> ... and it works on 386. But test program crashing on amd64, hitting
>> hardware breakpoint while running cgotest.TestCallback:
>
> If disable gcc do -O2, amd64 crash even remove volatile, -O2 just make
> callback function to one line:
> $ objdump -d callback_c.o
>
> callback_c.o:     file format pe-x86-64
>
>
> Disassembly of section .text:
>
> 0000000000000000 <callback>:
>   0:   e9 00 00 00 00          jmpq   5 <callback+0x5>
>   5:   90                      nop
>
>
>
>>
>>        <ProblemSignatures>
>>                <EventType>BEX64</EventType>
>>                <Parameter0>6.out.exe</Parameter0>
>>                <Parameter1>0.0.0.0</Parameter1>
>>                <Parameter2>4f0534ae</Parameter2>
>>                <Parameter3>6.out.exe</Parameter3>
>>                <Parameter4>0.0.0.0</Parameter4>
>>                <Parameter5>4f0534ae</Parameter5>
>>                <Parameter6>00000000000182ca</Parameter6>
>>                <Parameter7>80000003</Parameter7>
>>                <Parameter8>00000000badc0de1</Parameter8>
>>        </ProblemSignatures>
>>
>> http://codereview.appspot.com/5500094/

[brainman, 2012-01-06 05:28:57]

Hello golang-dev@googlegroups.com, vcc.163@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[vcc, 2012-01-06 05:35:27]

LGTM

On 2012/01/06 05:28:57, brainman wrote:
> Hello mailto:golang-dev@googlegroups.com, mailto:vcc.163@gmail.com (cc:
> mailto:golang-dev@googlegroups.com),
> 
> Please take another look.

[rsc, 2012-01-09 17:19:55]

I am confused about the misc/cgo/test/Makefile changes.
When we switch to the go tool that will go away, so it is
important that we are not adding special case hacks
to specific Makefiles.  What is going on?

[brainman, 2012-01-10 04:15:57]

On 2012/01/09 17:19:55, rsc wrote:
> ...  What is going on?

It seems 

volatile char data[64*1024];

from callback_c.c is compiled into a call into __chkstk function. If I do not
change Makefile, then test fails to build:

gcc -m32 -g -fPIC -O2 -o _cgo1_.o _cgo_main.o callback_c.o align.cgo2.o
basic.cgo2.o callback.cgo2.o env.cgo2.o exports.cgo2.o helpers.cgo2.o
issue1222.cgo2.o issue1328.cgo2.o issue1560.cgo2.o issue2462.cgo2.o
duplicate_symbol.cgo2.o sleep_windows.cgo2.o _cgo_export.o
cgo -dynimport _cgo1_.o >_obj/_cgo_import.c_ && mv -f _obj/_cgo_import.c_
_obj/_cgo_import.c
8c -FVw -I . -o "_cgo_import.8" _obj/_cgo_import.c
rm -f _test/runtime/cgotest.a
gopack grc _test/runtime/cgotest.a _gotest_.8 runtime.8  _cgo_defun.8
_cgo_import.8 callback_c.o  align.cgo2.o basic.cgo2.o callback.cgo2.o env.cgo2.o
exports.cgo2.o helpers.cgo2.o issue1222.cgo2.o issue1328.cgo2.o issue1560.cgo2.o
issue2462.cgo2.o duplicate_symbol.cgo2.o sleep_windows.cgo2.o _cgo_export.o
_test/runtime/cgotest.a(callback_c.o)(.text): __chkstk: not defined
__chkstk(0): not defined
gotest: "c:\\MinGW\\go\\bin\\8l.exe -L _test _testmain.8" failed: exit status 1
make: *** [test] Error 2

Wei told me that __chkstk function lives in /mingw/lib/gcc/mingw32/libgcc.a in a
_chkstk.o file. So I just extracted that file from the library and add it to our
build, so reference is resolved. I do not know enough about cgo to implement it
in a way "compatible" with new "go" tool. If you have suggestions, I am all
ears.

Alex

[brainman, 2012-01-18 04:07:03]

Russ,

Can I submit this? There are some bug fixes in src/pkg/runtime/asm_amd64.s.

We can leave figuring out how to build it with "go build" for later CL.
misc/cgo/life is in the same boat, while misc/cgo/stdio and misc/cgo/testso do
not even build with make on windows.

Alex

[rsc, 2012-01-19 21:09:43]

LGTM

[brainman, 2012-01-20 01:59:52]

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

misc/cgo/test: make tests run on windows

- use proper Win64 gcc calling convention when
  calling initcgo on amd64
- increase g0 stack size to 64K on amd64 to make
  it the same as 386
- implement C.sleep
- do not use C.stat, since it is renamed to C._stat by mingw
- use fopen to implement TestErrno, since C.strtol
  always succeeds on windows
- skip TestSetEnv on windows, because os.Setenv
  sets windows process environment, while C.getenv
  inspects internal C runtime variable instead

R=golang-dev, vcc.163, rsc
CC=golang-dev
http://codereview.appspot.com/5500094