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

Gotrace #1719

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Gotrace #1719

wants to merge 5 commits into from

Conversation

nyarly
Copy link

@nyarly nyarly commented Apr 30, 2018

This touches on #934 - it adds a rough gotrace.py tool (which still would require documentation etc), but handles Go gc compiled code by using stack offsets, and manages Go's Pascal-style strings. I'm still not sure how to handle slices or concrete structs, but it seemed reasonable to present this for comment now that it will work for some cases.

The appropriate comparison is to trace.py - reviewing what's changed, I think it would be possible to merge most of the changes back into trace.py with a few caveats -

  • to format Go strings properly, we need to capture the length, and then trim them in Python (the BPF validator doesn't like variable length string copies.) That behavior obviously isn't consistent with C-style null-terminated strings.
  • future work should address slices, which also transmit a capacity argument; if anything, they'll need to be formatted differently
  • I'm not certain, but I suspect that signatures in the probe definition won't work with Go programs at all.

I think this all could be handled by a flag and conditionals within trace.py

@nyarly
Copy link
Author

nyarly commented May 2, 2018

Does this make sense, @brendangregg @goldshtn I'd like to make progress on this, but I'd like some sense of what y'all think about this direction.

If it were to go back into trace.py, it occurred to me that, basically, it would need an expansion to the trace definition language - since afaict it doesn't make sense to allow signatures for Go gc traces.

@nyarly
Copy link
Author

nyarly commented May 13, 2018

Just a reminder on this PR - @goldshtn @brendangregg

@nyarly
Copy link
Author

nyarly commented Aug 17, 2018

Renewing my request for review, @goldshtn & @brendangregg - I got totally distracted myself, but I'd like to make progress on this

@bobrik
Copy link
Contributor

bobrik commented Aug 21, 2018

FYI: golang/go#27077

@nyarly
Copy link
Author

nyarly commented Aug 21, 2018

@bobrik I don't follow that issue completely. How does unwinding the stack relate? Is it peculiar to Go?

@bobrik
Copy link
Contributor

bobrik commented Aug 21, 2018

I was playing with funclatency and it broke with my test go program. Tracing of uretprobes can get into the same issues.

@nyarly
Copy link
Author

nyarly commented Aug 21, 2018

Do you understand the issue with unwinding the stack? Are uretprobes just fundamentally incompatible with Go programs - seems to me that they sometimes work, but I'm unclear on when or why. At the least, should we consider if BCC tools can determine if a particular probe will be incompatible?

@bobrik
Copy link
Contributor

bobrik commented Aug 22, 2018

I don't have enough knowledge to understand the issue with stack unwinding, sorry. My hope is that Go developers can figure this out.

@nyarly
Copy link
Author

nyarly commented Aug 23, 2018

@bobrik This comment seems to summarize the whole thing: #1320 (comment)

I'm not entirely sure what to do about this for this PR, which is kind of languishing anyway.

@Julio-Guerra
Copy link

What about making BCC tools ABI-aware?

@nyarly
Copy link
Author

nyarly commented Jan 15, 2019

@Julio-Guerra I'm not sure I understand what you mean?

@Julio-Guerra
Copy link

@Julio-Guerra I'm not sure I understand what you mean?

Since, I guess, trace.py expects C conventions that are for example false with Go, one way to merge your work into trace.py would be to abstract this out so that trace.py no longer makes assumptions on the binary interfaces and can therefore be ported to other languages.

@nyarly
Copy link
Author

nyarly commented Jan 21, 2019

The C conventions are pretty universal in Linux, though, as I understand it. Go's "everything goes on the stack directly" is a significant outlier, which also foils uretprobes, unfortunately. It's natural to want to say "report the 3rd argument when called", but that means something totally different to Go than it does to everything else. It's different enough that the Go trace version needs not only needs to handle it differently, but also present a different user interface.

@brendangregg
Copy link
Member

How much delta is there between gotrace and trace? If it's somewhat significant, then I'd go ahead with accepting gotrace into bcc.

I was just rereading Gianluca Borello's experiments with using uprobes on the return points (#1320 (comment)), and that's something that could be added to gotrace. With such a change, it'd definitely be a significant divergence from trace.

@bobek
Copy link

bobek commented Aug 19, 2020

This may be of interest -- golang/go#40724 -- they are proposing to switch to register-based calling for go 1.16.

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.

5 participants