code review 4926042: runtime: use cgo runtime functions to call windows syscalls

runtime: use cgo runtime functions to call windows syscalls

[brainman, 2011-08-22 10:58:05]

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

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

[jp, 2011-08-25 05:54:29]

published comments

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
File src/pkg/runtime/windows/amd64/sys.s (right):

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
src/pkg/runtime/windows/amd64/sys.s:43: MOVQ	24(SI), R9
is it safe to read wincall.args[3] here ? the array can be shorter, down to
zero-length.

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
src/pkg/runtime/windows/amd64/sys.s:84: SUBQ	$(m_fflag+8), SP	// at least space
for m_fflag
m_fflag+4 is enough, it is uint32

[rsc, 2011-08-26 20:20:26]

LGTM

Was this prompted by something or just a simplification?

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/386/s...
File src/pkg/runtime/windows/386/sys.s (right):

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/386/s...
src/pkg/runtime/windows/386/sys.s:15: MOVL	(wincall_n)(DX), CX	// words
Are the parens around (wincall_n) necessary?
If not, please remove throughout.

[brainman, 2011-08-27 13:17:09]

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

runtime: use cgo runtime functions to call windows syscalls

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

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/386/s...
File src/pkg/runtime/windows/386/sys.s (right):

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/386/s...
src/pkg/runtime/windows/386/sys.s:15: MOVL	(wincall_n)(DX), CX	// words
On 2011/08/26 20:20:26, rsc wrote:
> Are the parens around (wincall_n) necessary?
> If not, please remove throughout.

Done.

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
File src/pkg/runtime/windows/amd64/sys.s (right):

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
src/pkg/runtime/windows/amd64/sys.s:43: MOVQ	24(SI), R9
On 2011/08/25 05:54:29, jp wrote:
> is it safe to read wincall.args[3] here ? ...

I think so. All our memory is readable.

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
src/pkg/runtime/windows/amd64/sys.s:84: SUBQ	$(m_fflag+8), SP	// at least space
for m_fflag
On 2011/08/25 05:54:29, jp wrote:
> m_fflag+4 is enough, it is uint32

Done.

[brainman, 2011-08-27 13:17:51]

On 2011/08/26 20:20:26, rsc wrote:
> 
> Was this prompted by something or just a simplification?
> 

I have used callback test from misc/cgo/test to test windows callbacks. All
tests passed, but TestCallbackPanic failed with error message "locked OS thread
on exit from TestCallbackPanic". It appears, you have implemented to call
unlockm even during panic inside callback some time ago. But we didn't do the
same for windows callbaks. So instead of chasing your tail all the time, I just
decided to use your code instead.

I like how windows stdcall asm code is simple now too. I always struggle with
asm <g>.

Alex

[jp, 2011-08-27 14:16:08]

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
File src/pkg/runtime/windows/amd64/sys.s (right):

http://codereview.appspot.com/4926042/diff/4001/src/pkg/runtime/windows/amd64...
src/pkg/runtime/windows/amd64/sys.s:43: MOVQ	24(SI), R9
On 2011/08/27 13:17:09, brainman wrote:
> On 2011/08/25 05:54:29, jp wrote:
> > is it safe to read wincall.args[3] here ? ...
> 
> I think so. All our memory is readable.

I would change is to SP-relative

[jp, 2011-08-27 16:49:34]

http://codereview.appspot.com/4926042/diff/11001/src/pkg/runtime/windows/386/...
File src/pkg/runtime/windows/386/sys.s (right):

http://codereview.appspot.com/4926042/diff/11001/src/pkg/runtime/windows/386/...
src/pkg/runtime/windows/386/sys.s:26: CALL	AX
CALL wincall_fn(DX)

[brainman, 2011-08-29 00:11:58]

On 2011/08/27 14:16:08, jp wrote:
> 
> I would change is to SP-relative

Why?

And I can't do it here, because if I have 4 or less args, I don't copy them to
the stack.

Alex