From a8b8f30d39e40383978258abc3b87e1f6b130141 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Mon, 8 Jul 2019 19:24:56 +0200 Subject: [PATCH] godwarf: support recursive types involving C qualifiers and typedefs (#1603) Backports debug/dwarf commit: 535741a69a1300d1fe2800778b99c8a1b75d7fdd CL: https://go-review.googlesource.com/18459 The x/debug/dwarf that we used for dwarf/godwarf/type.go was forked from debug/dwarf long before this commit. Original description: Currently readType simultaneously constructs a type graph and resolves the sizes of the types. However, these two operations are fundamentally at odds: the order we parse a cyclic structure in may be different than the order we need to resolve type sizes in. As a result, it's possible that when readType attempts to resolve the size of a typedef, it may dereference a nil Type field of another typedef retrieved from the type cache that's only partially constructed. To fix this, we delay resolving typedef sizes until the end of the readType recursion, when the full type graph is constructed. Fixes #1601 --- _fixtures/issue1601.go | 20 ++++++++++++++++++++ pkg/dwarf/godwarf/type.go | 37 ++++++++++++++++++++++++++++--------- pkg/proc/proc_test.go | 8 ++++++++ 3 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 _fixtures/issue1601.go diff --git a/_fixtures/issue1601.go b/_fixtures/issue1601.go new file mode 100644 index 00000000..ef8de1a8 --- /dev/null +++ b/_fixtures/issue1601.go @@ -0,0 +1,20 @@ +package main + +import ( + "runtime" +) + +/* +typedef struct Qst Q1; +typedef const Q1 Q; +struct Qst { + Q *q; +}; + +const Q1 globalq; +*/ +import "C" + +func main() { + runtime.Breakpoint() +} diff --git a/pkg/dwarf/godwarf/type.go b/pkg/dwarf/godwarf/type.go index 96f60b81..d83c190e 100644 --- a/pkg/dwarf/godwarf/type.go +++ b/pkg/dwarf/godwarf/type.go @@ -483,7 +483,7 @@ func (t *ChanType) stringIntl(recCheck recCheck) string { // Type reads the type at off in the DWARF ``info'' section. func ReadType(d *dwarf.Data, index int, off dwarf.Offset, typeCache map[dwarf.Offset]Type) (Type, error) { - typ, err := readType(d, "info", d.Reader(), off, typeCache) + typ, err := readType(d, "info", d.Reader(), off, typeCache, nil) if typ != nil { typ.Common().Index = index } @@ -495,9 +495,14 @@ func getKind(e *dwarf.Entry) reflect.Kind { return reflect.Kind(integer) } +type delayedSize struct { + ct *CommonType // type that needs its size computed from ut + ut Type // underlying type +} + // readType reads a type from r at off of name using and updating a -// type cache. -func readType(d *dwarf.Data, name string, r *dwarf.Reader, off dwarf.Offset, typeCache map[dwarf.Offset]Type) (Type, error) { +// type cache, callers sohuld pass nil to delayedSize, it is used for recursion. +func readType(d *dwarf.Data, name string, r *dwarf.Reader, off dwarf.Offset, typeCache map[dwarf.Offset]Type, delayedSizes *[]delayedSize) (Type, error) { if t, ok := typeCache[off]; ok { return t, nil } @@ -511,9 +516,23 @@ func readType(d *dwarf.Data, name string, r *dwarf.Reader, off dwarf.Offset, typ return nil, dwarf.DecodeError{name, off, "no type at offset"} } + // If this is the root of the recursion, prepare to resolve typedef sizes + // once the recursion is done. This must be done after the type graph is + // constructed because it may need to resolve cycles in a different order + // than readType encounters them. + if delayedSizes == nil { + var delayedSizeList []delayedSize + defer func() { + for _, ds := range delayedSizeList { + ds.ct.ByteSize = ds.ut.Size() + } + }() + delayedSizes = &delayedSizeList + } + // Parse type from dwarf.Entry. // Must always set typeCache[off] before calling - // d.Type recursively, to handle circular types correctly. + // d.readType recursively, to handle circular types correctly. var typ Type nextDepth := 0 @@ -558,7 +577,7 @@ func readType(d *dwarf.Data, name string, r *dwarf.Reader, off dwarf.Offset, typ var t Type switch toff := tval.(type) { case dwarf.Offset: - if t, err = readType(d, name, d.Reader(), toff, typeCache); err != nil { + if t, err = readType(d, name, d.Reader(), toff, typeCache, delayedSizes); err != nil { return nil } case uint64: @@ -971,13 +990,13 @@ func readType(d *dwarf.Data, name string, r *dwarf.Reader, off dwarf.Offset, typ b = -1 switch t := typ.(type) { case *TypedefType: - b = t.Type.Size() + *delayedSizes = append(*delayedSizes, delayedSize{typ.Common(), t.Type}) case *MapType: - b = t.Type.Size() + *delayedSizes = append(*delayedSizes, delayedSize{typ.Common(), t.Type}) case *ChanType: - b = t.Type.Size() + *delayedSizes = append(*delayedSizes, delayedSize{typ.Common(), t.Type}) case *InterfaceType: - b = t.Type.Size() + *delayedSizes = append(*delayedSizes, delayedSize{typ.Common(), t.Type}) case *PtrType: b = int64(addressSize) case *FuncType: diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 50fdf1f3..22fc4c64 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -4410,3 +4410,11 @@ func TestPluginStepping(t *testing.T) { {contNext, "plugin2.go:26"}, {contNext, "plugintest2.go:42"}}) } + +func TestIssue1601(t *testing.T) { + //Tests that recursive types involving C qualifiers and typedefs are parsed correctly + withTestProcess("issue1601", t, func(p proc.Process, fixture protest.Fixture) { + assertNoError(proc.Continue(p), t, "Continue") + evalVariable(p, t, "C.globalq") + }) +}