-
Notifications
You must be signed in to change notification settings - Fork 616
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
Set base address on OSX #313
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -179,10 +179,25 @@ func (b *binrep) openMachO(name string, start, limit, offset uint64) (plugin.Obj | |
} | ||
defer of.Close() | ||
|
||
// 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 { | ||
return nil, fmt.Errorf("Parsing %s: no __TEXT segment", name) | ||
} | ||
if textSegment.Addr > start { | ||
return nil, fmt.Errorf("Parsing %s: __TEXT segment address (0x%x) > mapping start address (0x%x)", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error messages should start with lowercase: https://github.com/golang/go/wiki/CodeReviewComments#error-strings Suggesting By the way, could There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I wasn't sure about that but even if it is you'd probably want to know about it - i.e. get a bug report here, rather than just having it underflow and fail silently (or get lucky with the unsigned arithmetic). |
||
name, textSegment.Addr, start) | ||
} | ||
|
||
base := start - 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) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,24 +34,48 @@ var ( | |
func findSymbols(syms []byte, file string, r *regexp.Regexp, address uint64) ([]*plugin.Sym, error) { | ||
// Collect all symbols from the nm output, grouping names mapped to | ||
// the same address into a single symbol. | ||
|
||
// The symbols to return. | ||
var symbols []*plugin.Sym | ||
|
||
// The current group of symbol names, and the address they are all at. | ||
names, start := []string{}, uint64(0) | ||
|
||
buf := bytes.NewBuffer(syms) | ||
for symAddr, name, err := nextSymbol(buf); err == nil; symAddr, name, err = nextSymbol(buf) { | ||
|
||
for { | ||
symAddr, name, err := nextSymbol(buf) | ||
if err == io.EOF { | ||
// Done! If there was an unfinished group, append it. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit / opiniated: remove the exclamation mark, those are overly emotional for comments and log messages to my taste... |
||
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}) | ||
} | ||
} | ||
|
||
// And return the symbols. | ||
return symbols, nil | ||
} | ||
|
||
if err != nil { | ||
// There was some kind of serious error reading nm's output. | ||
return nil, err | ||
} | ||
if start == symAddr { | ||
|
||
// If this symbol is at the same address as the current group, add it to the group. | ||
if symAddr == start { | ||
names = append(names, name) | ||
continue | ||
} | ||
|
||
// Otherwise append the current group to the list of symbols. | ||
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}) | ||
} | ||
|
||
// And start a new group. | ||
names, start = []string{name}, symAddr | ||
} | ||
|
||
return symbols, nil | ||
} | ||
|
||
// matchSymbol checks if a symbol is to be selected by checking its | ||
|
@@ -62,7 +86,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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No but the comments for the |
||
return []string{name} | ||
} | ||
|
||
|
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> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
<!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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error messages should start with lowercase: https://github.com/golang/go/wiki/CodeReviewComments#error-strings
Suggesting
fmt.Errorf("not a mach executable: no __TEXT segment: %s", name)
, to be somewhat consistent withfmt.Errorf("unrecognized binary: %s", name)
above in this file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied the one above but ok.