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(symcache): Use same instruction address for full lookup #226

Merged
merged 1 commit into from
Apr 22, 2020

Conversation

jan-auer
Copy link
Member

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

Ensures that for a lookup with inline frames the same instruction address is yielded for every frame instead of the caller function address. This allows Sentry to display proper inline markers.

When this was implemented originally, this changed address was supposed to show the instruction of the call in the parent. However, this can also be observed using symbol_addr on the inlinee. Having the same instruction address is technically more correct.

Example

Notice the difference in instruction addresses. Inline frames are marked with a blue dashed underline.

Before
Screen Shot 2020-04-22 at 20 43 30

After
Screen Shot 2020-04-22 at 20 41 18

@jan-auer jan-auer requested review from mitsuhiko and a team April 22, 2020 18:47
@jan-auer jan-auer self-assigned this Apr 22, 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.

Makes sense

@jan-auer jan-auer merged commit 8ba9426 into master Apr 22, 2020
@jan-auer jan-auer deleted the fix/symcache-instr-addr branch April 22, 2020 19:10
jan-auer added a commit that referenced this pull request Apr 22, 2020
* master:
  fix(symcache): Use same instruction address for full lookup (#226)
  feat(examples): Add a naive addr2line example (#222)
  fix(build): Broken compilation with -Clink-dead-code (#225)
  fix(debuginfo): Patch all parent line records of inlinees in DWARF (#223)
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.

None yet

2 participants