code review 117280043: debug/dwarf: fix Reader panic on DW_TAG_unspecified_type

debug/dwarf: fix Reader panic on DW_TAG_unspecified_type

The linker currently produces the DWARF 3 DW_TAG_unspecified_type tag, however the Reader in debug/dwarf will panic whenever that tag is encountered.

Fixes issue 8437.

[dparker, 2014-07-28 20:14:06]

Hello golang-codereviews@googlegroups.com,

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

[bradfitz, 2014-07-31 17:40:15]

R=iant

But my non-dwarf-specific comment: no test?



On Mon, Jul 28, 2014 at 1:14 PM, <parkerderek86@gmail.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> debug/dwarf: fix Reader panic on DW_TAG_unspecified_type
>
> The linker currently produces the DWARF 3 DW_TAG_unspecified_type tag,
> however the Reader in debug/dwarf will panic whenever that tag is
> encountered.
>
> Fixes issue 8437.
>
> Please review this at https://codereview.appspot.com/117280043/
>
> Affected files (+13, -0 lines):
>   M src/pkg/debug/dwarf/type.go
>
>
> Index: src/pkg/debug/dwarf/type.go
> ===================================================================
> --- a/src/pkg/debug/dwarf/type.go
> +++ b/src/pkg/debug/dwarf/type.go
> @@ -88,6 +88,10 @@
>         BasicType
>  }
>
> +type UnspecifiedType struct {
> +       BasicType
> +}
> +
>  // qualifiers
>
>  // A QualType represents a type that has the C/C++ "const", "restrict",
> or "volatile" qualifier.
> @@ -630,6 +634,15 @@
>                 typeCache[off] = t
>                 t.Name, _ = e.Val(AttrName).(string)
>                 t.Type = typeOf(e)
> +
> +       case TagUnspecifiedType:
> +               // Unspecified type (DWARF v3 §5.2)
> +               // Attributes:
> +               //      AttrName: name
> +               t := new(UnspecifiedType)
> +               typ = t
> +               typeCache[off] = t
> +               t.Name, _ = e.Val(AttrName).(string)
>         }
>
>         if err != nil {
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[dparker, 2014-07-31 17:54:27]

On 2014/07/31 17:40:15, bradfitz wrote:
> R=iant
> 
> But my non-dwarf-specific comment: no test?
> 
> 
> 
> On Mon, Jul 28, 2014 at 1:14 PM, <mailto:parkerderek86@gmail.com> wrote:
> 
> > Reviewers: golang-codereviews,
> >
> > Message:
> > Hello mailto:golang-codereviews@googlegroups.com,
> >
> > I'd like you to review this change to
> > https://code.google.com/p/go
> >
> >
> > Description:
> > debug/dwarf: fix Reader panic on DW_TAG_unspecified_type
> >
> > The linker currently produces the DWARF 3 DW_TAG_unspecified_type tag,
> > however the Reader in debug/dwarf will panic whenever that tag is
> > encountered.
> >
> > Fixes issue 8437.
> >
> > Please review this at https://codereview.appspot.com/117280043/
> >
> > Affected files (+13, -0 lines):
> >   M src/pkg/debug/dwarf/type.go
> >
> >
> > Index: src/pkg/debug/dwarf/type.go
> > ===================================================================
> > --- a/src/pkg/debug/dwarf/type.go
> > +++ b/src/pkg/debug/dwarf/type.go
> > @@ -88,6 +88,10 @@
> >         BasicType
> >  }
> >
> > +type UnspecifiedType struct {
> > +       BasicType
> > +}
> > +
> >  // qualifiers
> >
> >  // A QualType represents a type that has the C/C++ "const", "restrict",
> > or "volatile" qualifier.
> > @@ -630,6 +634,15 @@
> >                 typeCache[off] = t
> >                 t.Name, _ = e.Val(AttrName).(string)
> >                 t.Type = typeOf(e)
> > +
> > +       case TagUnspecifiedType:
> > +               // Unspecified type (DWARF v3 §5.2)
> > +               // Attributes:
> > +               //      AttrName: name
> > +               t := new(UnspecifiedType)
> > +               typ = t
> > +               typeCache[off] = t
> > +               t.Name, _ = e.Val(AttrName).(string)
> >         }
> >
> >         if err != nil {
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "golang-codereviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to mailto:golang-codereviews+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
> >

Gah, I know, it felt really sloppy pushing this up without a test. It seems like
everything in debug/dwarf/testdata is generated via the following gcc command:
cc -gdwarf-2 -m64 -c typedef.c && gcc -gdwarf-2 -m64 -o typedef.elf typedef.o

and then there is the type_test.go file which runs through and tests all of the
typedefs in debug/dwarf/testdata/typedef.c. I could try to see if I can get the
linker to emit a DW_TAG_unspecified_type tag somehow and assert that in the map
it maps to <unspecified type> when `String()`'ed. Any direction on how to get
the linker to emit that tag so I can assert about it? I can dig through the
linker and figure it out probably otherwise.

[iant, 2014-07-31 22:48:19]

https://codereview.appspot.com/117280043/diff/30002/src/pkg/debug/dwarf/type.go
File src/pkg/debug/dwarf/type.go (right):

https://codereview.appspot.com/117280043/diff/30002/src/pkg/debug/dwarf/type....
src/pkg/debug/dwarf/type.go:91: type UnspecifiedType struct {
This new exported type needs a doc comment, like the preceding ones.

[dparker, 2014-08-01 14:30:24]

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

Please take another look.

[dparker, 2014-08-01 14:31:42]

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

Please take another look.

[rsc, 2014-08-06 18:34:41]

LGTM

[rsc, 2014-08-06 18:35:39]

Please complete a CLA as described at golang.org/doc/contribute.html#copyright

Thanks.

[dparker, 2014-08-06 18:50:14]

On 2014/08/06 18:35:39, rsc wrote:
> Please complete a CLA as described at golang.org/doc/contribute.html#copyright
> 
> Thanks.

Thanks! I went ahead and completed it online and submitted it.

[bradfitz, 2014-08-06 19:11:39]

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

debug/dwarf: fix Reader panic on DW_TAG_unspecified_type

The linker currently produces the DWARF 3 DW_TAG_unspecified_type tag, however
the Reader in debug/dwarf will panic whenever that tag is encountered.

Fixes issue 8437.

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

Committer: Brad Fitzpatrick <bradfitz@golang.org>

[r, 2014-08-06 20:25:14]

https://codereview.appspot.com/117280043/diff/60001/src/pkg/debug/dwarf/type.go
File src/pkg/debug/dwarf/type.go (right):

https://codereview.appspot.com/117280043/diff/60001/src/pkg/debug/dwarf/type....
src/pkg/debug/dwarf/type.go:91: // A UnspecifiedType represents implicit,
unknown, ambiguous or nonexistent type.
s/A/An/

[r, 2014-08-06 20:35:45]

https://codereview.appspot.com/117280043/diff/60001/src/pkg/debug/dwarf/type.go
File src/pkg/debug/dwarf/type.go (right):

https://codereview.appspot.com/117280043/diff/60001/src/pkg/debug/dwarf/type....
src/pkg/debug/dwarf/type.go:91: // A UnspecifiedType represents implicit,
unknown, ambiguous or nonexistent type.
s/implicit/an implicit/