code review 4907048: runtime: simplify stack traces

runtime: simplify stack traces

Make the stack traces more readable for new
Go programmers while preserving their utility for old hands.

- Change status number [4] to string.
- Elide frames in runtime package (internal details).
- Swap file:line and arguments.
- Drop 'created by' for main goroutine.
- Show goroutines in order of allocation:
  implies main goroutine first if nothing else.

There is no option to get the extra frames back.
Uncomment 'return 1' at the bottom of symtab.c.

$ 6.out
throw: all goroutines are asleep - deadlock!

goroutine 1 [chan send]:
main.main()
       /Users/rsc/g/go/src/pkg/runtime/x.go:22 +0x8a

goroutine 2 [select (no cases)]:
main.sel()
       /Users/rsc/g/go/src/pkg/runtime/x.go:11 +0x18
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:19 +0x23

goroutine 3 [chan receive]:
main.recv(0xf8400010a0, 0x0)
       /Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:20 +0x50

goroutine 4 [chan receive (nil chan)]:
main.recv(0x0, 0x0)
       /Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:21 +0x66
$

$ 6.out index
panic: runtime error: index out of range

goroutine 1 [running]:
main.main()
        /Users/rsc/g/go/src/pkg/runtime/x.go:25 +0xb9
$

$ 6.out nil
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x22ca]

goroutine 1 [running]:
main.main()
        /Users/rsc/g/go/src/pkg/runtime/x.go:28 +0x211
$

$ 6.out panic
panic: panic

goroutine 1 [running]:
main.main()
        /Users/rsc/g/go/src/pkg/runtime/x.go:30 +0x101
$

[rsc, 2011-08-18 16:18:27]

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

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

[zhai, 2011-08-18 16:32:51]

On Fri, Aug 19, 2011 at 12:18 AM, <rsc@golang.org> wrote:

> Description:
> runtime: simplify stack traces
>
> Make the stack traces more readable for new
> Go programmers while preserving their utility for old hands.
>
>
> - Elide frames in runtime package (internal details).

 - Drop 'created by' for main goroutine.
>

Ohh, I miss the prev precise stack traces.

[rsc, 2011-08-18 16:36:29]

> Ohh, I miss the prev precise stack traces.

Why?  Can you give an example of a failure
where the new stack trace does not contain
the information you need to debug?

Russ

[zhai, 2011-08-18 16:46:42]

On Fri, Aug 19, 2011 at 12:36 AM, Russ Cox <rsc@golang.org> wrote:

> > Ohh, I miss the prev precise stack traces.
>
> Why?  Can you give an example of a failure
> where the new stack trace does not contain
> the information you need to debug?
>
> Russ
>


 - Elide frames in runtime package (internal details).
 - Drop 'created by' for main goroutine.

when I  looking at the stack traces, I only interest in the .go files
generally
but the information above ( .s file ) is very useful when need inspecting
inside the runtime.

[rsc, 2011-08-18 16:56:01]

>  - Elide frames in runtime package (internal details).
>  - Drop 'created by' for main goroutine.
> when I  looking at the stack traces, I only interest in the .go files
> generally
> but the information above ( .s file ) is very useful when need inspecting
> inside the runtime.

If you are actively working on package runtime
(definitely not the common case), then the fix
is to uncomment the 'return 1' at the bottom of
symtab.c, as it says in the CL description.

Russ

[niemeyer, 2011-08-18 16:57:25]

I've been following the thread and the debate about more easily identifying
goroutine boundaries. I like the current style, but it also feels like we could
add a trivial marker to facilitate visual scanning of goroutine boundaries
without compromising on the alignment.

Here is a suggestion, reformatting the CL desc:

 $ 6.out
throw: all goroutines are asleep - deadlock!

goroutine 1 [chan send]:
main.main()
       /Users/rsc/g/go/src/pkg/runtime/x.go:22 +0x8a
-----
goroutine 2 [select (no cases)]:
main.sel()
       /Users/rsc/g/go/src/pkg/runtime/x.go:11 +0x18
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:19 +0x23
-----
goroutine 3 [chan receive]:
main.recv(0xf8400010a0, 0x0)
       /Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:20 +0x50
-----
goroutine 4 [chan receive (nil chan)]:
main.recv(0x0, 0x0)
       /Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:21 +0x66

[rsc, 2011-08-18 17:00:38]

I'd rather not bikeshed this.  There are already blank lines.

Russ

[niemeyer, 2011-08-18 17:36:12]

> I'd rather not bikeshed this.  There are already blank lines.

Suggesting a different idea to address an aspect of
the problem is not necessarily bikesheding.

I won't argue about that either.. but I do suggest
asking a few people around you to compare the two
approaches and whether they are more easily able to
spot the boundary.

If they are, there's no reason not to add it.

</argument>

[rsc, 2011-08-18 17:40:17]

>> I'd rather not bikeshed this.  There are already blank lines.
>
> Suggesting a different idea to address an aspect of
> the problem is not necessarily bikesheding.

Agreed.  But ultimately this comes down to
what you expect to see vs what you see.
I have no doubt that whichever we choose
people will be able to read at a glance after
seeing one or two.

Russ

[r, 2011-08-19 04:24:15]

LGTM
it would be nice to clean up runtime/debug.Stack too but that can wait

[rsc, 2011-08-23 03:26:43]

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

runtime: simplify stack traces

Make the stack traces more readable for new
Go programmers while preserving their utility for old hands.

- Change status number [4] to string.
- Elide frames in runtime package (internal details).
- Swap file:line and arguments.
- Drop 'created by' for main goroutine.
- Show goroutines in order of allocation:
  implies main goroutine first if nothing else.

There is no option to get the extra frames back.
Uncomment 'return 1' at the bottom of symtab.c.

$ 6.out
throw: all goroutines are asleep - deadlock!

goroutine 1 [chan send]:
main.main()
       /Users/rsc/g/go/src/pkg/runtime/x.go:22 +0x8a

goroutine 2 [select (no cases)]:
main.sel()
       /Users/rsc/g/go/src/pkg/runtime/x.go:11 +0x18
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:19 +0x23

goroutine 3 [chan receive]:
main.recv(0xf8400010a0, 0x0)
       /Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:20 +0x50

goroutine 4 [chan receive (nil chan)]:
main.recv(0x0, 0x0)
       /Users/rsc/g/go/src/pkg/runtime/x.go:15 +0x2e
created by main.main
       /Users/rsc/g/go/src/pkg/runtime/x.go:21 +0x66
$

$ 6.out index
panic: runtime error: index out of range

goroutine 1 [running]:
main.main()
        /Users/rsc/g/go/src/pkg/runtime/x.go:25 +0xb9
$

$ 6.out nil
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x22ca]

goroutine 1 [running]:
main.main()
        /Users/rsc/g/go/src/pkg/runtime/x.go:28 +0x211
$

$ 6.out panic
panic: panic

goroutine 1 [running]:
main.main()
        /Users/rsc/g/go/src/pkg/runtime/x.go:30 +0x101
$

R=golang-dev, qyzhai, n13m3y3r, r
CC=golang-dev
http://codereview.appspot.com/4907048

[rsc, 2011-08-23 14:44:50]

My fix for runtime.Callers slipped into this CL unintentionally.
http://codereview.appspot.com/4907048/diff2/7001:17002/src/pkg/runtime/runtime.c