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

[Deploy preview] Inline callstacks demo #2556

Closed
wants to merge 13 commits into from

Conversation

mstange
Copy link
Contributor

@mstange mstange commented May 20, 2020

Deploy preview
Deploy preview

Same profile: before after

Another comparison: before after

It's not working as great as I hoped it would. There are lots of bad frames in the call stacks. I think the problem is that the return addresses, which are collected during stackwalking, point to the instruction after the call instruction.
I'll try to find out how many goats I'd have to sacrifice to have the stackwalkers give me the addresses of the actual call instructions, for all caller frames upstack.

Edit: This is working great now. I just needed to adjust caller addresses by 1 byte upwards. Now all line information and inline stacks in caller functions look correct.

@mstange
Copy link
Contributor Author

mstange commented May 20, 2020

It's not working as great as I hoped it would. There are lots of bad frames in the call stacks. I think the problem is that the return addresses, which are collected during stackwalking, point to the instruction after the call instruction.
I'll try to find out how many goats I'd have to sacrifice to have the stackwalkers give me the addresses of the actual call instructions, for all caller frames upstack.

Turns out it's not that hard to work around this: When looking up symbols for callers, we can just subtract one byte from the return address, and we'll fall into the call instruction.

Deploy preview

@gregtatum gregtatum closed this Jun 30, 2020
@gregtatum gregtatum reopened this Jun 30, 2020
@gregtatum gregtatum changed the base branch from master to main June 30, 2020 19:27
@mstange
Copy link
Contributor Author

mstange commented Dec 7, 2021

This is superseded by #3556 and #3727.

@mstange mstange closed this Dec 7, 2021
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