-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
DWARF unwinding (CFI) fixes #3206
Comments
It's also used for Go but the subset used by Go is small and cgo testing is lacking. PRs for this would be welcome. I'm also curious what you are using this for if you can say. |
Sounds good! Will aim to send the PRs in a few weeks :). For context, I work on Parca Agent project, which is a continuous profiler written in Go which uses BPF to collect stack traces. As we speak I am working on a blog post that will be published tomorrow with all the details, but the gist of it is that currently there's no easy way to profile applications that were compiled without frame pointers. And as you are probably aware, this is a very common toggle that gets disabled by many distros and packagers. To allow profiling without frame pointers we've developed a BPF-based unwinder that uses information extracted from dwarf unwinding tables. We use quite a bit of Delve's code to do this. Unfortunately, we had to fork it for a variety of reasons, but we plan to upstream all the improvements that we might make, such as the correctness patches I mentioned 😄
Out of curiosity, do you happen to know when this is the case? I was under the impression that Go unwinding/stack walking could be done with |
It's always used, we could use the spdelta table but we never did, the standard library doesn't have a convenient interface for that and we didn't write one and it's better to minimize dependencies on internals of the go runtime. |
Just pushed #3218. Note that this code is dead. The current constants used by Delve are perfectly correct :) |
For the first issue, this is the current code we run, adapted to this codebase. Can open a PR in few days. We need to do this to follow the DWARF spec which specifies that a stack should be used as there could be a series of remember opcodes in a row |
Looks ok, except StateStack shouldn't be exported and prevRegs should be deleted. |
Good catch, missed pushing the change that removed |
Hi!! First of all, thanks so much for Delve, been using it since I've started using Go and works incredibly well and fast 🎉.
Long story short, we've used some of Delve's code, in particular, the DWARF CFI parser which has been working great for us. We've modified this code to suit our needs better, and while working on this we noticed that some of the DWARF opcodes might not be handled correctly, in particular:
I was wondering why this wasn't noticed before. As far as I understand Delve implements this to debug cgo code exclusively, is this correct?
I am more than happy to submit PRs for both of these issues in the next few weeks, but first I wanted to bring this up to see if this would be something you would be interested in.
Thanks!!
cc/ @aarzilli @kakkoyun
The text was updated successfully, but these errors were encountered: