code review 3309041: [68]l: generate debug info for builtin structured types...

[68]l: generate debug info for builtin structured types.  prettyprinting in gdb.

[lvd, 2010-12-01 01:07:39]

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

I'd like you to review this change.

[lvd, 2010-12-01 21:52:16]

sample session:

6g ~/g3/test/chan/fifo.go
6l fifo.6
gdb 6.out
---------------------------

(gdb) b 16
Breakpoint 1 at 0x400c17: file /home/lvd/g3/test/chan/fifo.go, line 16.
(gdb) r
Starting program: /home/lvd/play/6.out

Breakpoint 1, main.AsynchFifo () at /home/lvd/g3/test/chan/fifo.go:16
16              ch := make(chan int, N)
(gdb) p ch
$1 = (chan int) 0x43bd20
(gdb) p *ch
$2 = struct hchan<int>
(gdb) n
17              for i := 0; i < N; i++ {
(gdb)
18                      ch <- i
(gdb)
17              for i := 0; i < N; i++ {
(gdb)
18                      ch <- i
(gdb)
17              for i := 0; i < N; i++ {
(gdb) p ch
$3 = (chan int) 0x7ffff7eed000
(gdb) p *ch
$4 = struct hchan<int> = {0, 1}
(gdb) p $len(*ch)
$5 = 2
(gdb) p $cap(*ch)
$6 = 10




On Wed, Dec 1, 2010 at 02:07, <lvd@google.com> wrote:

> Reviewers: rsc,
>
> Message:
> Hello rsc (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> [68]l: generate debug info for builtin structured types.  prettyprinting
> in gdb.
>
> Please review this at http://codereview.appspot.com/3309041/
>
> Affected files:
>  M src/cmd/ld/dwarf.c
>  A src/pkg/runtime/runtime-gdb.py
>
>
>

[rsc1, 2010-12-02 16:50:15]

This is very cool.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c
File src/cmd/ld/dwarf.c (right):

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode902
src/cmd/ld/dwarf.c:902: decodetype_funcintype(Sym *s, int i) {
brace on next line
and the next function too

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode985
src/cmd/ld/dwarf.c:985: if (1 && debug['v'] > 2) {
change back to 0
?

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1070
src/cmd/ld/dwarf.c:1070: for (i = 0; i < nfields; ++i) {
i++
throughout

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1263
src/cmd/ld/dwarf.c:1263: hash		= defgotype(lookup("type.runtime.hash",0));
hash = defgotype is fine.  no need for the internal tabs

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1267
src/cmd/ld/dwarf.c:1267: if (!hash || !hash_subtable || !hash_entry)
please use ptr == nil instead of !ptr
throughout

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1303
src/cmd/ld/dwarf.c:1303: mkinternaltypename("hash_entry",
wrap with one extra tab.  all the spaces will look terrible
in any other tab width or in a variable-width font.

dwhe = newdie...,
\tmkinternaltypename("...",
\t\tgetattr...

throughout

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1958
src/cmd/ld/dwarf.c:1958: if (nn) n = nn + 1;
multiple lines

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2235
src/cmd/ld/dwarf.c:2235: writegdbscript()
(void)

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2275
src/cmd/ld/dwarf.c:2275: DW_AT_type, find(&dwtypes, "void"));
look for "\t " in your program and fix them please

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2318
src/cmd/ld/dwarf.c:2318: pubnameso  = writepub(ispubname);
don't bother lining these up

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2446
src/cmd/ld/dwarf.c:2446: if (pubnamessize  > 0) nsect++;
multiple lines

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb.py
File src/pkg/runtime/runtime-gdb.py (right):

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:21: "Pretty print Go strings."
please use tabs for indentation, even in Python,
for consistency with the other python we have
to maintain (lib/codereview/codereview.py)

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:28: def display_hint(self): return 'string'
please use multiple lines for all these
grep for ": "

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:33: class SliceTypePrinter:
should have a class ArrayTypePrinter too
and len and cap on arrays
they're rare but still happen

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:58: #    pattern = re.compile(r'^map\[.*?\].*$')
delete?

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:64: def display_hint (self): return 'map'
s/ (/(/

[lvd, 2010-12-02 20:46:55]

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c
File src/cmd/ld/dwarf.c (right):

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode902
src/cmd/ld/dwarf.c:902: decodetype_funcintype(Sym *s, int i) {
On 2010/12/02 16:50:15, rsc1 wrote:
> brace on next line
> and the next function too

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode985
src/cmd/ld/dwarf.c:985: if (1 && debug['v'] > 2) {
On 2010/12/02 16:50:15, rsc1 wrote:
> change back to 0
> ?

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1070
src/cmd/ld/dwarf.c:1070: for (i = 0; i < nfields; ++i) {
On 2010/12/02 16:50:15, rsc1 wrote:
> i++
> throughout
> 

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1263
src/cmd/ld/dwarf.c:1263: hash		= defgotype(lookup("type.runtime.hash",0));
On 2010/12/02 16:50:15, rsc1 wrote:
> hash = defgotype is fine.  no need for the internal tabs

the Anal Alignment helps me catch cut & paste errors

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1267
src/cmd/ld/dwarf.c:1267: if (!hash || !hash_subtable || !hash_entry)
On 2010/12/02 16:50:15, rsc1 wrote:
> please use ptr == nil instead of !ptr
> throughout

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1303
src/cmd/ld/dwarf.c:1303: mkinternaltypename("hash_entry",
On 2010/12/02 16:50:15, rsc1 wrote:
> wrap with one extra tab.  all the spaces will look terrible
> in any other tab width or in a variable-width font.
> 
> dwhe = newdie...,
> \tmkinternaltypename("...",
> \t\tgetattr...
> 
> throughout

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode1958
src/cmd/ld/dwarf.c:1958: if (nn) n = nn + 1;
On 2010/12/02 16:50:15, rsc1 wrote:
> multiple lines

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2235
src/cmd/ld/dwarf.c:2235: writegdbscript()
On 2010/12/02 16:50:15, rsc1 wrote:
> (void)

Done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2275
src/cmd/ld/dwarf.c:2275: DW_AT_type, find(&dwtypes, "void"));
On 2010/12/02 16:50:15, rsc1 wrote:
> look for "\t " in your program and fix them please

emacs put them in.  done.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2318
src/cmd/ld/dwarf.c:2318: pubnameso  = writepub(ispubname);
On 2010/12/02 16:50:15, rsc1 wrote:
> don't bother lining these up

again, this really helps me.  if you want i'll make a separate CL in a few weeks
unaligning them all.

http://codereview.appspot.com/3309041/diff/19001/src/cmd/ld/dwarf.c#newcode2446
src/cmd/ld/dwarf.c:2446: if (pubnamessize  > 0) nsect++;
On 2010/12/02 16:50:15, rsc1 wrote:
> multiple lines

Done.

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb.py
File src/pkg/runtime/runtime-gdb.py (right):

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:21: "Pretty print Go strings."
On 2010/12/02 16:50:15, rsc1 wrote:
> please use tabs for indentation, even in Python,
> for consistency with the other python we have
> to maintain (lib/codereview/codereview.py)

Done.

Couldnt get emacs to ignor the google.el settings for python, so i just run the
whole thing through unexpand -t4.  i'll try to remember doing that before
submitting.

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:28: def display_hint(self): return 'string'
On 2010/12/02 16:50:15, rsc1 wrote:
> please use multiple lines for all these
> grep for ": "
> 

Done.

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:33: class SliceTypePrinter:
On 2010/12/02 16:50:15, rsc1 wrote:
> should have a class ArrayTypePrinter too
> and len and cap on arrays
> they're rare but still happen

arrays are printed correctly natively by gdb by virtue of the completeness of
the corresponding dwarf.  if you type whatis 'main.foo' you get the type, who's
name includes len==cap.  so pretty printing and $len/$cap are a bit superfluous.

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:58: #    pattern = re.compile(r'^map\[.*?\].*$')
On 2010/12/02 16:50:15, rsc1 wrote:
> delete?

this is how i quickly switch them on or off while developing.  # is gone

http://codereview.appspot.com/3309041/diff/19001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:64: def display_hint (self): return 'map'
On 2010/12/02 16:50:15, rsc1 wrote:
> s/ (/(/

Done.

[rsc1, 2010-12-03 17:52:35]

LGTM

http://codereview.appspot.com/3309041/diff/30001/src/cmd/ld/dwarf.c
File src/cmd/ld/dwarf.c (right):

http://codereview.appspot.com/3309041/diff/30001/src/cmd/ld/dwarf.c#newcode1418
src/cmd/ld/dwarf.c:1418: if (strncmp(s, "go.string.", 10) == 0) return;
multiple lines

http://codereview.appspot.com/3309041/diff/30001/src/pkg/runtime/runtime-gdb.py
File src/pkg/runtime/runtime-gdb.py (right):

http://codereview.appspot.com/3309041/diff/30001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:77: yield ("[%d]" %  i,		 v['key'])
single space is fine after comma

http://codereview.appspot.com/3309041/diff/30001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:145: if
StringTypePrinter.pattern.match(typename): return obj['len']
multiple lines
here and next 10 or so

http://codereview.appspot.com/3309041/diff/30001/src/pkg/runtime/runtime-gdb....
src/pkg/runtime/runtime-gdb.py:165: if hasattr(k, 'invoke'): k()
multiple lines

[lvd, 2010-12-03 18:11:43]

all done and submitting

[lvd, 2010-12-03 18:19:41]

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

[68]l: generate debug info for builtin structured types.  prettyprinting in gdb.

R=rsc
CC=golang-dev
http://codereview.appspot.com/3309041