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

fix(debuginfo): Skip eliminated functions in linked objects #216

Merged
merged 4 commits into from
Apr 8, 2020

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Apr 7, 2020

Fixes an issue that indirectly lead to "inline parent offset too large for symcache file format" errors by reintroducing checks for functions with a DW_AT_low_pc of 0.

In regular shared objects or executables, such functions have been eliminated by the linker. As the primary use case of this library is to symbolicate stack traces, yielding such functions from the function iterator is not desirable. Also, the virtual address of the functions is no longer valid and implicitly overlaps with other eliminated functions. At least in the default behavior, they should not be included.

Removing the checks introduced a 10% performance regression on large ELF symbols. I did not notice this on my initial tests, since I missed the fact that this case is only triggered by the GNU linker. If it weren't for the symcache bug, the performance hit alone would be bearable.

Now, for relocatable objects it may make sense to include the functions. This means, there are two options:

  1. Adding a method on DwarfFunctionIterator that allows to override the check. This has the disadvantage that it would not be exposed on the trait or on ObjectFunctionIterator. A possible interface could be:
for function in debug_session.functions().include_eliminated(true) { ... }
  1. The DWARF implementation could detect that this is a Relocatable, and include the function by default and hide them otherwise. This would mean that the iterator magically does the right thing. Also, this comes closest to the original intention of the previously removed code comment.

Since I am gravitating to option 2, this is what is implemented in this patch.

Partially reverts #173
cc @nnethercote

@jan-auer jan-auer added the bug label Apr 7, 2020
@jan-auer jan-auer requested review from mitsuhiko and a team April 7, 2020 23:13
@jan-auer jan-auer self-assigned this Apr 7, 2020
Copy link
Member

@mitsuhiko mitsuhiko left a comment

Choose a reason for hiding this comment

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

Just the bool thing. Logic seems sensible.

debuginfo/src/elf.rs Outdated Show resolved Hide resolved
debuginfo/src/dwarf.rs Outdated Show resolved Hide resolved
@nnethercote
Copy link
Contributor

I think this will preserve the behaviour I need for my use case involving .o files. Thanks for keeping it in mind!

@jan-auer jan-auer merged commit 4d509d1 into master Apr 8, 2020
@jan-auer jan-auer deleted the fix/skip-optimized-functions branch April 8, 2020 08:06
jan-auer added a commit that referenced this pull request Apr 8, 2020
* master:
  fix(debuginfo): Skip eliminated functions in linked objects (#216)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants