code review 6297087: runtime: fix struct Sigaction for Linux/ARM

runtime: fix struct Sigaction for Linux/ARM
        if we were to use sizeof(sa.sa_mask) instead of 8 as the last argument
        to rt_sigaction, we would have already fixed this bug, so also updated
        Linux/386 and Linux/amd64 files to use that; also test the return value
        of rt_sigaction.

[dave_cheney.net, 2012-06-14 21:09:55]

Thanks for fixing this.

http://codereview.appspot.com/6297087/diff/1002/src/pkg/runtime/defs_linux_arm.h
File src/pkg/runtime/defs_linux_arm.h (right):

http://codereview.appspot.com/6297087/diff/1002/src/pkg/runtime/defs_linux_ar...
src/pkg/runtime/defs_linux_arm.h:146: uint64 sa_mask;
If sa_mask is 64bit, then rt_sigaction should use the EABI even register
convention for passing it. Possibly rt_sigprocmask is affected as well. 


TEXT runtime·rt_sigaction(SB),7,$0
        MOVW    0(FP), R0
        MOVW    4(FP), R2
        MOVW    8(FP), R3
        MOVW    12(FP), R4
        MOVW    $SYS_rt_sigaction, R7
        SWI     $0
        MOVW    $0xfffff001, R6
        CMP     R6, R0
        MOVW.HI $0, R9  // crash on syscall failure
        MOVW.HI R9, (R9)
        RET

[dave_cheney.net, 2012-06-14 23:13:11]

http://codereview.appspot.com/6297087/diff/1002/src/pkg/runtime/defs_linux_ar...
> src/pkg/runtime/defs_linux_arm.h:146: uint64 sa_mask;
> If sa_mask is 64bit, then rt_sigaction should use the EABI even register
> convention for passing it. Possibly rt_sigprocmask is affected as well. 

Ignore this, sigaction takes a pointer to &sigmask (but I do think it's worth
checking the return value).

[minux1, 2012-06-15 09:27:36]

On 2012/06/14 23:13:11, dfc wrote:
> Ignore this, sigaction takes a pointer to &sigmask (but I do think it's worth
> checking the return value).
i think we can check the return value at the C side (the syscall retval is
automatically
that of runtime.rt_sigaction.

[minux1, 2012-06-15 09:40:00]

PTAL.

check runtime.rt_sigaction return value in signal_linux_$GOARCH.c.

[dave_cheney.net, 2012-06-15 10:58:38]

On 2012/06/15 09:40:00, minux wrote:
> PTAL.
> 
> check runtime.rt_sigaction return value in signal_linux_$GOARCH.c.

LGTM. tested on linux/amd64 and linux/arm.

[rsc, 2012-06-19 03:54:06]

LGTM

http://codereview.appspot.com/6297087/diff/1003/src/pkg/runtime/signal_linux_...
File src/pkg/runtime/signal_linux_386.c (right):

http://codereview.appspot.com/6297087/diff/1003/src/pkg/runtime/signal_linux_...
src/pkg/runtime/signal_linux_386.c:133: runtime·throw("rt_sigaction failure!");
"runtime: rt_sigaction failed"

the ! is unnecessary

http://codereview.appspot.com/6297087/diff/1003/src/pkg/runtime/signal_linux_...
File src/pkg/runtime/signal_linux_amd64.c (right):

http://codereview.appspot.com/6297087/diff/1003/src/pkg/runtime/signal_linux_...
src/pkg/runtime/signal_linux_amd64.c:143: runtime·throw("rt_sigaction
failure!");
Same.

http://codereview.appspot.com/6297087/diff/1003/src/pkg/runtime/signal_linux_...
File src/pkg/runtime/signal_linux_arm.c (right):

http://codereview.appspot.com/6297087/diff/1003/src/pkg/runtime/signal_linux_...
src/pkg/runtime/signal_linux_arm.c:143: runtime·throw("rt_sigaction failure!");
Same.

[minux1, 2012-06-19 17:16:52]

Hello dave@cheney.net, rsc@golang.org (cc: golang-dev@googlegroups.com),

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

[minux1, 2012-06-19 17:17:24]

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

runtime: fix struct Sigaction for Linux/ARM
        if we were to use sizeof(sa.sa_mask) instead of 8 as the last argument
        to rt_sigaction, we would have already fixed this bug, so also updated
        Linux/386 and Linux/amd64 files to use that; also test the return value
        of rt_sigaction.

R=dave, rsc
CC=golang-dev
http://codereview.appspot.com/6297087