Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for parsing multiple filename tables #218

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions _fixtures/cgotest.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package main

/*
#include <stdio.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add this include?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't reproduce until we have some C includes. Even new test will pass without this.
I have tested with all combinations between [oldTest, newTest, oldcgo, newcgo, cgo-with-only-runtime, cgo-with-only-header, cgo-with-header-runtime]
Tested with 1 variable at a time. newTest+cgo-with-header-with-runtime is required for fail test.

char* foo(void) { return "hello, world!"; }
*/
import "C"

import "fmt"
import "runtime"

func main() {
runtime.GOMAXPROCS(runtime.NumCPU())
fmt.Println(C.GoString(C.foo()))
}
63 changes: 51 additions & 12 deletions dwarf/line/line_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
)

type DebugLinePrologue struct {
Length uint32
UnitLength uint32
Version uint16
PrologueLength uint32
Length uint32
MinInstrLength uint8
InitialIsStmt uint8
LineBase int8
Expand All @@ -24,6 +24,7 @@ type DebugLineInfo struct {
IncludeDirs []string
FileNames []*FileEntry
Instructions []byte
Lookup map[string]*FileEntry
}

type FileEntry struct {
Expand All @@ -33,26 +34,63 @@ type FileEntry struct {
Length uint64
}

func Parse(data []byte) *DebugLineInfo {
type DebugLines []*DebugLineInfo

func (d *DebugLines) GetLineInfo(name string) *DebugLineInfo {
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer line comments, i.e.: //.

Find in which table file exists.
Return that table
*/
for _, l := range *d {
if fe := l.GetFileEntry(name); fe != nil {
return l
}
}

return nil
}

func (d *DebugLineInfo) GetFileEntry(name string) *FileEntry {
return d.Lookup[name]
}

func Parse(data []byte) DebugLines {
var (
dbl = new(DebugLineInfo)
buf = bytes.NewBuffer(data)
lines = make(DebugLines, 0)
buf = bytes.NewBuffer(data)
)

parseDebugLinePrologue(dbl, buf)
parseIncludeDirs(dbl, buf)
parseFileEntries(dbl, buf)
dbl.Instructions = buf.Bytes()
// we have to scan multiple file name tables here
for buf.Len() > 0 {
dbl := new(DebugLineInfo)
dbl.Lookup = make(map[string]*FileEntry)

parseDebugLinePrologue(dbl, buf)
parseIncludeDirs(dbl, buf)
parseFileEntries(dbl, buf)

/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as comment above regarding prefer line comments. Also, although much appreciated, you can remove the shout out at the end of the calculation description :).

Instructions size calculation breakdown:

- dbl.Prologue.UnitLength is the length of the entire unit, not including the 4 bytes to represent that length.
- dbl.Prologue.Length is the length of the prologue not including unit length, version or the prologue length itself.
- So you have UnitLength - PrologueLength - (version_length_bytes(2) + prologue_length_bytes(4)).
- thanks to Derek for ^
*/
dbl.Instructions = buf.Next(int(dbl.Prologue.UnitLength - dbl.Prologue.Length - 6))

return dbl
lines = append(lines, dbl)
}

return lines
}

func parseDebugLinePrologue(dbl *DebugLineInfo, buf *bytes.Buffer) {
p := new(DebugLinePrologue)

p.Length = binary.LittleEndian.Uint32(buf.Next(4))
p.UnitLength = binary.LittleEndian.Uint32(buf.Next(4))
p.Version = binary.LittleEndian.Uint16(buf.Next(2))
p.PrologueLength = binary.LittleEndian.Uint32(buf.Next(4))
p.Length = binary.LittleEndian.Uint32(buf.Next(4))
p.MinInstrLength = uint8(buf.Next(1)[0])
p.InitialIsStmt = uint8(buf.Next(1)[0])
p.LineBase = int8(buf.Next(1)[0])
Expand Down Expand Up @@ -91,5 +129,6 @@ func parseFileEntries(info *DebugLineInfo, buf *bytes.Buffer) {
entry.Length, _ = util.DecodeULEB128(buf)

info.FileNames = append(info.FileNames, entry)
info.Lookup[name] = entry
}
}
3 changes: 2 additions & 1 deletion dwarf/line/line_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ func TestDebugLinePrologueParser(t *testing.T) {
}
defer os.Remove(p)
data := grabDebugLineSection(p, t)
dbl := Parse(data)
debugLines := Parse(data)
dbl := debugLines[0]
prologue := dbl.Prologue

if prologue.Version != uint16(2) {
Expand Down
16 changes: 10 additions & 6 deletions dwarf/line/state_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,15 @@ func newStateMachine(dbl *DebugLineInfo) *StateMachine {

// Returns all PCs for a given file/line. Useful for loops where the 'for' line
// could be split amongst 2 PCs.
func (dbl *DebugLineInfo) AllPCsForFileLine(f string, l int) (pcs []uint64) {
func (dbl *DebugLines) AllPCsForFileLine(f string, l int) (pcs []uint64) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kill leading whitespace.

lineInfo := dbl.GetLineInfo(f)

var (
sm = newStateMachine(lineInfo)
buf = bytes.NewBuffer(lineInfo.Instructions)
foundFile bool
lastAddr uint64
sm = newStateMachine(dbl)
buf = bytes.NewBuffer(dbl.Instructions)
)

for b, err := buf.ReadByte(); err == nil; b, err = buf.ReadByte() {
Expand All @@ -105,11 +108,12 @@ func (dbl *DebugLineInfo) AllPCsForFileLine(f string, l int) (pcs []uint64) {
return
}

func (dbl *DebugLineInfo) AllPCsBetween(begin, end uint64) []uint64 {
func (dbl *DebugLines) AllPCsBetween(begin, end uint64, filename string) []uint64 {
lineInfo := dbl.GetLineInfo(filename)
var (
pcs []uint64
sm = newStateMachine(dbl)
buf = bytes.NewBuffer(dbl.Instructions)
sm = newStateMachine(lineInfo)
buf = bytes.NewBuffer(lineInfo.Instructions)
)

for b, err := buf.ReadByte(); err == nil; b, err = buf.ReadByte() {
Expand Down
2 changes: 1 addition & 1 deletion proc/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Process struct {
dwarf *dwarf.Data
goSymTable *gosym.Table
frameEntries frame.FrameDescriptionEntries
lineInfo *line.DebugLineInfo
lineInfo line.DebugLines
firstStart bool
os *OSProcessDetails
arch Arch
Expand Down
28 changes: 28 additions & 0 deletions proc/proc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,34 @@ func TestSwitchThread(t *testing.T) {
})
}

func TestCGONext(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this test fail without your patch? I think there is already a CGO next test (I know it was my suggestion to write this test), but I want to make sure we have a test that triggers the behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This test fails without my patch with exact same error. (with updated cgotest fixture only).
Another cgo test passes (with/without extra C header).

//test if one can do 'next' in a cgo binary

// On OSX with Go < 1.5 CGO is not supported due to: https://github.com/golang/go/issues/8973
if runtime.GOOS == "darwin" && strings.Contains(runtime.Version(), "1.4") {
return
}

withTestProcess("cgotest", t, func(p *Process, fixture protest.Fixture) {
pc, err := p.FindFunctionLocation("main.main", true, 0)
if err != nil {
t.Fatal(err)
}
_, err = p.SetBreakpoint(pc)
if err != nil {
t.Fatal(err)
}
err = p.Continue()
if err != nil {
t.Fatal(err)
}
err = p.Next()
if err != nil {
t.Fatal(err)
}
})
}

type loc struct {
line int
fn string
Expand Down
6 changes: 3 additions & 3 deletions proc/threads.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (thread *Thread) setNextBreakpoints() (err error) {
if filepath.Ext(loc.File) == ".go" {
err = thread.next(curpc, fde, loc.File, loc.Line)
} else {
err = thread.cnext(curpc, fde)
err = thread.cnext(curpc, fde, loc.File)
}
return err
}
Expand Down Expand Up @@ -226,8 +226,8 @@ func (thread *Thread) next(curpc uint64, fde *frame.FrameDescriptionEntry, file
// Set a breakpoint at every reachable location, as well as the return address. Without
// the benefit of an AST we can't be sure we're not at a branching statement and thus
// cannot accurately predict where we may end up.
func (thread *Thread) cnext(curpc uint64, fde *frame.FrameDescriptionEntry) error {
pcs := thread.dbp.lineInfo.AllPCsBetween(fde.Begin(), fde.End())
func (thread *Thread) cnext(curpc uint64, fde *frame.FrameDescriptionEntry, file string) error {
pcs := thread.dbp.lineInfo.AllPCsBetween(fde.Begin(), fde.End(), file)
ret, err := thread.ReturnAddress()
if err != nil {
return err
Expand Down