code review 5792048: runtime: inline calls to notok

runtime: inline calls to notok

When a very low-level system call that should never fail
does fail, we call notok, which crashes the program.
Often, we are then left with only the program counter as
information about the crash, and it is in notok.
Instead, inline calls to notok (it is just one instruction
on most systems) so that the program counter will
tell us which system call is unhappy.

[rsc, 2012-03-08 18:35:05]

Hello golang-dev@googlegroups.com,

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

[gri, 2012-03-08 18:43:59]

LGTM

- some // crash comments are not aligned (don't care too much)
- isn't there a panic or the like opcode on x86? (0xcc opcode)
- I would add a panic pseudo assembly instruction to the compiler which could do
the right thing for each architecture

http://codereview.appspot.com/5792048/diff/2013/src/pkg/runtime/asm_amd64.s
File src/pkg/runtime/asm_amd64.s (right):

http://codereview.appspot.com/5792048/diff/2013/src/pkg/runtime/asm_amd64.s#n...
src/pkg/runtime/asm_amd64.s:67: PUSHQ	$runtime·main(SB)		// entry
align comment ?

http://codereview.appspot.com/5792048/diff/2013/src/pkg/runtime/asm_amd64.s#n...
src/pkg/runtime/asm_amd64.s:76: MOVL	$0xf1, 0xf1  // crash
align comment?

[minux1, 2012-03-08 18:46:06]

LGTM.

[rsc, 2012-03-08 18:55:56]

Thanks.

On Thu, Mar 8, 2012 at 13:43,  <gri@golang.org> wrote:
> - isn't there a panic or the like opcode on x86? (0xcc opcode)
> - I would add a panic pseudo assembly instruction to the compiler which
> could do the right thing for each architecture

0xcc is breakpoint.  It's not clear if we want that or not.
For now I am happy with commented instructions.
I'm just trying to make bug reports a little easier to read.
We could make more sweeping changes after Go 1.

Russ

[minux1, 2012-03-08 18:57:25]

On 2012/03/08 18:43:59, gri wrote:
> - isn't there a panic or the like opcode on x86? (0xcc opcode)
0xcc (int 3) will trigger SIGTRAP and its default behavior is SigThrow, not
SigPanic.
> - I would add a panic pseudo assembly instruction to the compiler which could
do
> the right thing for each architecture
This is a good idea.

[bradfitz, 2012-03-08 18:59:00]

I assume all the 2(PC) etc are still the right sizes?

On Thu, Mar 8, 2012 at 10:35 AM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> runtime: inline calls to notok
>
> When a very low-level system call that should never fail
> does fail, we call notok, which crashes the program.
> Often, we are then left with only the program counter as
> information about the crash, and it is in notok.
> Instead, inline calls to notok (it is just one instruction
> on most systems) so that the program counter will
> tell us which system call is unhappy.
>
> Please review this at
http://codereview.appspot.com/**5792048/<http://codereview.appspot.com/5792048/>
>
> Affected files:
>  M src/pkg/runtime/asm_amd64.s
>  M src/pkg/runtime/sys_darwin_**386.s
>  M src/pkg/runtime/sys_darwin_**amd64.s
>  M src/pkg/runtime/sys_freebsd_**386.s
>  M src/pkg/runtime/sys_freebsd_**amd64.s
>  M src/pkg/runtime/sys_linux_**amd64.s
>  M src/pkg/runtime/sys_linux_arm.**s
>  M src/pkg/runtime/sys_netbsd_**386.s
>  M src/pkg/runtime/sys_netbsd_**amd64.s
>  M src/pkg/runtime/sys_openbsd_**386.s
>  M src/pkg/runtime/sys_openbsd_**amd64.s
>  M src/pkg/runtime/sys_windows_**amd64.s
>
>
>

[rsc, 2012-03-08 18:59:32]

On Thu, Mar 8, 2012 at 13:58, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> I assume all the 2(PC) etc are still the right sizes?

Yes, 2(PC) means 2 instructions after the current one.

[bradfitz, 2012-03-08 19:02:46]

On Thu, Mar 8, 2012 at 10:59 AM, Russ Cox <rsc@golang.org> wrote:

> On Thu, Mar 8, 2012 at 13:58, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> > I assume all the 2(PC) etc are still the right sizes?
>
> Yes, 2(PC) means 2 instructions after the current one.
>

Ah. Thought it was size of instructions. If you need me, I'll be up in http
land.

[rsc, 2012-03-08 19:03:59]

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

runtime: inline calls to notok

When a very low-level system call that should never fail
does fail, we call notok, which crashes the program.
Often, we are then left with only the program counter as
information about the crash, and it is in notok.
Instead, inline calls to notok (it is just one instruction
on most systems) so that the program counter will
tell us which system call is unhappy.

R=golang-dev, gri, minux.ma, bradfitz
CC=golang-dev
http://codereview.appspot.com/5792048