Skip to content

Conversation

@nnethercote
Copy link
Contributor

I am using symbolic_debuginfo in a symbolicator used for stack traces produced by Firefox.

On Mac, we don't build dSYMs by default because doing so is slow. So I want to emulate atos, which is able to read debuginfo from a binary by consulting the binary's symbol table and then reading debug info from the object files and archive files mentioned.

I have this working, but it requires a couple of small changes to symbolic_debuginfo, due to slight differences between .o files and executables/libraries.

Currently `raw_section` finds the given section by assuming that it's
within a segment named `__TEXT` or `__DWARF`, finding such a segment,
and then searching within that segment.

However, this fails for `.o` files, because they have a single nameless
segment. As
https://github.com/aidansteele/osx-abi-macho-file-format-reference says:

> For compactness, an intermediate object file contains only one
> segment. This segment has no name; it contains all the sections
> destined ultimately for different segments in the final object file.

This commit changes `raw_section` so it ignores segment names, and
simply searches through all segments.

As well as working with `.o` files, the resulting code is a little
shorter and simpler.
Because they occur in `.o` files.

Without the first change in this commit, debuginfo for entire `.o` files
can be missed. Without the second change, debuginfo for the first
function (which can start at 0x0) can be missed.
@nnethercote
Copy link
Contributor Author

This is my first PR for this project, so please let me know if I've overlooked anything w.r.t. contributing. I did run cargo test --all --all-features, and it passed.

Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @nnethercote! Once we get my question below addressed, I'm happy to merge.

This is my first PR for this project, so please let me know if I've overlooked anything w.r.t. contributing.

All good, thanks for asking!

@nnethercote
Copy link
Contributor Author

@jan-auer: Any new thoughts here? The project I'm working on currently uses a fork of Symbolic that contains this PR, because it needs to work directly with .o files on Mac. It's not an ideal situation, and it would be great if it could be resolved. Thanks.

@nnethercote
Copy link
Contributor Author

My tool is now being used in production for Firefox. It's still relying on the version of symbolic-debuginfo in this PR, because the release version doesn't do the right thing for my use case. This is sub-optimal.

@jan-auer
Copy link
Member

Sorry for the late reply, I took a closer look and ran some tests now. Performance checks out. It seems that this was an outdated concern, and I can frankly no longer find the cases where this would've hit us.

For some reason I remember this also being a correctness concern, but this might be false memory.

Thanks for this patch. Will look into merging all PRs now and then releasing a new version to crates.io as soon as we get rid of all the git dependencies.

@jan-auer jan-auer changed the title Improve .o handling on Mac fix(debuginfo): Improve .o handling on MachO (#173) Mar 17, 2020
@jan-auer jan-auer merged commit e4b3939 into getsentry:master Mar 17, 2020
@nnethercote nnethercote deleted the improve-o-handling-on-Mac branch March 17, 2020 21:57
@nnethercote nnethercote restored the improve-o-handling-on-Mac branch March 18, 2020 04:01
// TODO: *technically* there could be a relocatable section placed at VA 0
let low_pc = match low_pc {
Some(low_pc) if low_pc != 0 => low_pc,
Some(low_pc) => low_pc,
Copy link
Member

Choose a reason for hiding this comment

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

@nnethercote I'm afraid we'll need to make this check conditional, as you suggested. See #173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants