code review 72790043: runtime: fix nacl/amd64p32 signal handling

runtime: fix nacl/amd64p32 signal handling

The change to signal_amd64.c from CL 15790043 was not merged correctly.

This CL reapplies the change, renaming the file to signal_amd64x.c and adds the appropriate build tags.

[dfc, 2014-03-08 03:52:55]

Hello rsc@golang.org (cc: golang-codereviews@googlegroups.com),

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

[iant, 2014-03-08 22:55:19]

LGTM

Seems like runtime.h is still wrong in the way it defines uintreg on NaCL,
though.

[dfc, 2014-03-09 01:43:02]

Hi Ian,

Thanks for your review. Are you talking about

#ifdef _64BITREG

in runtime.h ?

_64BITREG is defined in 6c/txt.c. I agree that it should be
conditional. I'll check the changes to see if that bit got missed.

On Sun, Mar 9, 2014 at 9:55 AM,  <iant@golang.org> wrote:
> LGTM
>
> Seems like runtime.h is still wrong in the way it defines uintreg on
> NaCL, though.
>
> https://codereview.appspot.com/72790043/

[dfc, 2014-03-09 01:59:56]

Hi Ian,

I had a read of 6c/txt.go and I think the current logic is correct, _64BITREG is
always defined for 6c, but _64BIT is set only when ewidth[TIND] is set to 8 in
cc/lex.c.

Cheers

Dave

[iant, 2014-03-09 16:25:30]

On Sat, Mar 8, 2014 at 5:43 PM, Dave Cheney <dave@cheney.net> wrote:
>
> Thanks for your review. Are you talking about
>
> #ifdef _64BITREG
>
> in runtime.h ?
>
> _64BITREG is defined in 6c/txt.c. I agree that it should be
> conditional. I'll check the changes to see if that bit got missed.

Sorry for being unclear.  The code that I see in runtime.h is:

#ifdef _64BITREG
//typedef	uint64		uintreg;
#else
//typedef	uint32		uintreg;
#endif

It seems to me that that #ifdef does nothing because the lines it
controls are commented out.

Ian

[dfc, 2014-03-09 23:15:40]

*facepalm*, fixing.

On Mon, Mar 10, 2014 at 3:25 AM, Ian Lance Taylor <iant@golang.org> wrote:
> On Sat, Mar 8, 2014 at 5:43 PM, Dave Cheney <dave@cheney.net> wrote:
>>
>> Thanks for your review. Are you talking about
>>
>> #ifdef _64BITREG
>>
>> in runtime.h ?
>>
>> _64BITREG is defined in 6c/txt.c. I agree that it should be
>> conditional. I'll check the changes to see if that bit got missed.
>
> Sorry for being unclear.  The code that I see in runtime.h is:
>
> #ifdef _64BITREG
> //typedef       uint64          uintreg;
> #else
> //typedef       uint32          uintreg;
> #endif
>
> It seems to me that that #ifdef does nothing because the lines it
> controls are commented out.
>
> Ian

[dfc, 2014-03-11 04:25:55]

Hello rsc@golang.org, iant@golang.org (cc: golang-codereviews@googlegroups.com),

Please take another look.

[bradfitz, 2014-03-11 20:20:30]

LGTM

confirm it's the same as CL 15790043

[dfc, 2014-03-11 20:22:15]

*** Submitted as https://code.google.com/p/go/source/detail?r=104004fc1da3 ***

runtime: fix nacl/amd64p32 signal handling

The change to signal_amd64.c from CL 15790043 was not merged correctly.

This CL reapplies the change, renaming the file to signal_amd64x.c and adds the
appropriate build tags.

LGTM=iant, bradfitz
R=rsc, iant, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/72790043

[gobot, 2014-03-11 20:26:31]

This CL appears to have broken the darwin-386-cheney builder.