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.

Sign up for GitHub

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

Set base address on OSX #313

Merged
merged 4 commits into from
Feb 9, 2018
Merged

Set base address on OSX #313

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5bdb9a9
Set base address on OSX
Timmmm Feb 6, 2018
98b3a8b
Simplify openMachO, add tests
Feb 7, 2018
ebd8f8c
Address various binutils issues in previous commits
Feb 8, 2018
25f4ba0
Fix nits & remove unused test binaries for now
Feb 9, 2018
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
14 changes: 12 additions & 2 deletions internal/binutils/binutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,20 @@ func (b *binrep) openMachO(name string, start, limit, offset uint64) (plugin.Obj
}
defer of.Close()

base := start
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am a bit hesitant whether a better value for the base in the absence of __TEXT is zero or start. But I guess it's not important since files without __TEXT should be impractical case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'll add a warning if it isn't found.


// Subtract the load address of the __TEXT section. Usually 0 for shared
// libraries or 0x100000000 for executables. You can check this value by
// running `objdump -private-headers <file>`.
textSegment := of.Segment("__TEXT")
if textSegment != nil {
base -= textSegment.Addr
}

if b.fast || (!b.addr2lineFound && !b.llvmSymbolizerFound) {
return &fileNM{file: file{b: b, name: name}}, nil
return &fileNM{file: file{b: b, name: name, base: base}}, nil
}
return &fileAddr2Line{file: file{b: b, name: name}}, nil
return &fileAddr2Line{file: file{b: b, name: name, base: base}}, nil
}

func (b *binrep) openELF(name string, start, limit, offset uint64) (plugin.ObjFile, error) {
Expand Down
79 changes: 76 additions & 3 deletions internal/binutils/binutils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,20 @@ func TestSetFastSymbolization(t *testing.T) {

func skipUnlessLinuxAmd64(t *testing.T) {
if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
t.Skip("Disasm only tested on x86-64 linux")
t.Skip("This test only works on x86-64 Linux")
}
}

func skipUnlessDarwinAmd64(t *testing.T) {
if runtime.GOOS != "darwin" || runtime.GOARCH != "amd64" {
t.Skip("This test only works on x86-64 Mac")
}
}

func TestDisasm(t *testing.T) {
skipUnlessLinuxAmd64(t)
bu := &Binutils{}
insts, err := bu.Disasm(filepath.Join("testdata", "hello"), 0, math.MaxUint64)
insts, err := bu.Disasm(filepath.Join("testdata", "exe_linux_64"), 0, math.MaxUint64)
if err != nil {
t.Fatalf("Disasm: unexpected error %v", err)
}
Expand Down Expand Up @@ -215,7 +221,7 @@ func TestObjFile(t *testing.T) {
} {
t.Run(tc.desc, func(t *testing.T) {
bu := &Binutils{}
f, err := bu.Open(filepath.Join("testdata", "hello"), tc.start, tc.limit, tc.offset)
f, err := bu.Open(filepath.Join("testdata", "exe_linux_64"), tc.start, tc.limit, tc.offset)
if err != nil {
t.Fatalf("Open: unexpected error %v", err)
}
Expand Down Expand Up @@ -255,6 +261,73 @@ func TestObjFile(t *testing.T) {
}
}

func TestMachoFiles(t *testing.T) {
skipUnlessDarwinAmd64(t)

// Load `file`, pretending it was mapped at `start`. Then get the symbol
// table. Check that it contains the symbol `sym` and that the address
// `addr` gives the `expected` stack trace.
for _, tc := range []struct {
desc string
file string
start, limit, offset uint64
addr uint64
sym string
expected []plugin.Frame
}{
{"normal mapping", "exe_mac_64", 0x100000000, math.MaxUint64, 0,
0x100000f50, "_main",
[]plugin.Frame{
{Func: "main", File: "/tmp/hello.c", Line: 3},
}},
{"other mapping", "exe_mac_64", 0x200000000, math.MaxUint64, 0,
0x200000f50, "_main",
[]plugin.Frame{
{Func: "main", File: "/tmp/hello.c", Line: 3},
}},
{"lib normal mapping", "lib_mac_64", 0, math.MaxUint64, 0,
0xfa0, "_bar",
[]plugin.Frame{
{Func: "bar", File: "/tmp/lib.c", Line: 6},
}},
} {
t.Run(tc.desc, func(t *testing.T) {
bu := &Binutils{}
f, err := bu.Open(filepath.Join("testdata", tc.file), tc.start, tc.limit, tc.offset)
if err != nil {
t.Fatalf("Open: unexpected error %v", err)
}
defer f.Close()
syms, err := f.Symbols(nil, 0)
if err != nil {
t.Fatalf("Symbols: unexpected error %v", err)
}

find := func(name string) *plugin.Sym {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: could move this piece out to an outer function to reuse with the other test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

for _, s := range syms {
for _, n := range s.Name {
if n == name {
return s
}
}
}
return nil
}
m := find(tc.sym)
if m == nil {
t.Fatalf("Symbols: could not find symbol %v", tc.sym)
}
gotFrames, err := f.SourceLine(tc.addr)
if err != nil {
t.Fatalf("SourceLine: unexpected error %v", err)
}
if !reflect.DeepEqual(gotFrames, tc.expected) {
t.Fatalf("SourceLine for main: got %v; want %v\n", gotFrames, tc.expected)
}
})
}
}

func TestLLVMSymbolizer(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("testtdata/llvm-symbolizer has only been tested on linux")
Expand Down
Binary file added internal/binutils/debug.test
View file
Open in desktop
Binary file not shown.
9 changes: 8 additions & 1 deletion internal/binutils/disasm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func findSymbols(syms []byte, file string, r *regexp.Regexp, address uint64) ([]
var symbols []*plugin.Sym
names, start := []string{}, uint64(0)
buf := bytes.NewBuffer(syms)
symAddr := uint64(0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this symAddr will get shadowed by the assignment in the loop header, so it's probably not what the code after the loop expects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well spotted!

for symAddr, name, err := nextSymbol(buf); err == nil; symAddr, name, err = nextSymbol(buf) {
if err != nil {
return nil, err
Expand All @@ -51,6 +52,12 @@ func findSymbols(syms []byte, file string, r *regexp.Regexp, address uint64) ([]
names, start = []string{name}, symAddr
}

if len(names) != 0 {
if match := matchSymbol(names, start, symAddr-1, r, address); match != nil {
symbols = append(symbols, &plugin.Sym{Name: match, File: file, Start: start, End: symAddr - 1})
}
}

return symbols, nil
}

Expand All @@ -62,7 +69,7 @@ func matchSymbol(names []string, start, end uint64, r *regexp.Regexp, address ui
return names
}
for _, name := range names {
if r.MatchString(name) {
if r == nil || r.MatchString(name) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we pass r as nil from anywhere? Ah, you do know in f.Symbols(nil, 0) in the test. I think you can just do f.Symbols(regexp.MustCompile(tc.sym), 0)?.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No but the comments for the Symbols() interface say it can be nil.

return []string{name}
}

Expand Down
0 internal/binutils/testdata/hello → internal/binutils/testdata/exe_linux_64
View file
Open in desktop
File renamed without changes.
Binary file added internal/binutils/testdata/exe_mac_64
View file
Open in desktop
Binary file not shown.
20 changes: 20 additions & 0 deletions internal/binutils/testdata/exe_mac_64.dSYM/Contents/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CFBundleDevelopmentRegion</key>
<string>English</string>
<key>CFBundleIdentifier</key>
<string>com.apple.xcode.dsym.exe_mac_64</string>
<key>CFBundleInfoDictionaryVersion</key>
<string>6.0</string>
<key>CFBundlePackageType</key>
<string>dSYM</string>
<key>CFBundleSignature</key>
<string>????</string>
<key>CFBundleShortVersionString</key>
<string>1.0</string>
<key>CFBundleVersion</key>
<string>1</string>
</dict>
</plist>
Binary file added internal/binutils/testdata/exe_mac_64.dSYM/Contents/Resources/DWARF/exe_mac_64
View file
Open in desktop
Binary file not shown.
Binary file added internal/binutils/testdata/lib_mac_64
View file
Open in desktop
Binary file not shown.
20 changes: 20 additions & 0 deletions internal/binutils/testdata/lib_mac_64.dSYM/Contents/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reading https://stackoverflow.com/questions/8058497/can-clang-build-executables-with-dsym-debug-information, it appears to me that building the test binaries from the command line it should be possible to have the debug information as part of those, and these additional dSYM files and directory would not be needed. I can look into doing that myself, can you include the source code of the executable and library test binaries that you used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep you can build it from source too. I just copied how the Linux test worked.

The only weird thing is if you have linking as a separate step (i.e. clang++ -c foo.cpp -o foo.o; clang++ foo.o -o foo rather than just doing it all in one step you need to run dysmutil on the binary. Shouldn't matter in this case because it is just one file.

<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>CFBundleDevelopmentRegion</key>
<string>English</string>
<key>CFBundleIdentifier</key>
<string>com.apple.xcode.dsym.lib_mac_64</string>
<key>CFBundleInfoDictionaryVersion</key>
<string>6.0</string>
<key>CFBundlePackageType</key>
<string>dSYM</string>
<key>CFBundleSignature</key>
<string>????</string>
<key>CFBundleShortVersionString</key>
<string>1.0</string>
<key>CFBundleVersion</key>
<string>1</string>
</dict>
</plist>
Binary file added internal/binutils/testdata/lib_mac_64.dSYM/Contents/Resources/DWARF/lib_mac_64
View file
Open in desktop
Binary file not shown.