Bcc: trace.py support for args with Go's gc compiler

Created on 2 Feb 2017  路  10Comments  路  Source: iovisor/bcc

trace's arg# aliases work with Go when compiled with gccgo ("gccgo -o hello hello.go"). But Go's gc compiler ("go build hello.go") uses a different function calling convention, based on the Plan 9 compiler.

Is it important to fix this? Does everyone run gccgo binaries (where trace already works), because they are expected to be faster, and not Go's compiler? I don't know. I've added a prio:low tag for now.

I wrote some quick docs on the issues here: http://www.brendangregg.com/blog/2017-01-31/golang-bcc-bpf-function-tracing.html

And included a proof of concept:

# trace '/home/bgregg/functions:main.add "%d %d" go1, go2'
PID    TID    COMM         FUNC             -
17555  17555  functions    main.add         42 13

The change I made was a dirty hack, and needs to be rewritten properly. I'd done this:

# diff -u trace.py trace-go.py 
--- trace.py    2017-02-01 19:34:01.477727396 +0000
+++ trace-go.py 2017-02-02 09:28:23.796606244 +0000
@@ -209,6 +209,8 @@

         aliases = {
                 "retval": "PT_REGS_RC(ctx)",
+                "go1": "1;bpf_probe_read(&__data.v0,sizeof(__data.v0),(void *)ctx->sp+8)",
+                "go2": "1;bpf_probe_read(&__data.v1,sizeof(__data.v1),(void *)ctx->sp+16)",
                 "arg1": "PT_REGS_PARM1(ctx)",
                 "arg2": "PT_REGS_PARM2(ctx)",
                 "arg3": "PT_REGS_PARM3(ctx)",

They should be "goarg1", "goarg1", etc. I suspect this also needs to be done differently so that they can be used in tests, eg, "(goarg1 == 42)". And this should use macros instead of ctx->sp.

I'd also check the following resources to confirm that this approach is valid:

enhancement low

Most helpful comment

Implemented #1719 as a first cut of this.

All 10 comments

If you're looking for datapoints, I used to (go 1.0) rely on gccgo when I needed to link in external C libraries, but nowadays native go is my go-to.

I'd go for a helper syntax like $ptr(x) which would do the bpf_probe_read automatically, and then the goargN aliases can be made equivalent to $ptr(PT_REGS_SP(ctx)+N*8), at least on 64-bit platforms. But that way we also gain the generic ability to do memory reads, which might prove useful. (E.g. in a retprobe when we know rax points to a value we want, we'd be able to do $ptr(ctx->ax).)

But there's also always the general remark that we keep pushing the trace/argdist syntax further and further, and there has to be a point where we recommend a dedicated script. @brendangregg

We also use gc binaries, so I've been doing something similar to this "dirty hack".

I'd be happy to do the work to get this functionality added to the trace script, but I'm not sure exactly what solution you'd be most happy to see. If we go for the $ptr() approach, for example, it seems like we'll need to generalize the paren-matching logic in _parse_probe. Would changes that extensive be welcome?

Someone just told me about this, possible for go 1.8:

https://go-review.googlesource.com/#/c/28832/

This is work in progress for passing arguments (parameters?
what is the proper word?) and results in registers.

Maybe Go is going to switch to the x86_84 ABI?

@kelleyk I think having $ptr would be nice in general. All the parsing work is pretty hacky right now, so if you feel like cleaning that up or generalizing, it would be very welcome. With the direction we're currently heading, it looks like we could be adding more helpers like this in the future.

I think that the details of the new Go ABI are still up in the air, and from https://github.com/golang/go/issues/18597 it sounds like the change is motivated by performance and not by standardization. There is some mention of making the new ABI a superset of the platform ABI, but also not a lot of hesitation to stray when useful (by, for example, skipping the 160-byte save area on s390x). It also seems like it would land in go1.9 at the earliest, since the go1.8 tree was feature-frozen at the end of October.

@goldshtn Thanks for the guidance! Let me see if I can find a minute to take a stab at $ptr and the supporting parsing improvements, then.

I ran across this and started toying with a PR. What I'm finding is that the changes required to trace.py are more extensive - at least the assignment templating needs to change to accomodate strings or multiple arguments. I'm tempted, therefore, to copy trace.py to gotrace.py and hack that up until it can manage the gc style of parameter handling. Would that be acceptable as a PR?

@nyarly Anything that works would be better than having no solution at all, but I think having a separate tool in the long run would not be great. If you're so inclined, you could hack on gotrace.py temporarily, just to see if the approach works, but I think we should have a single tool that can support both use cases. And if it means a massive refactoring of trace.py's (admittedly already hacky) code, I'm afraid that's just a prerequisite for this work. Merging gotrace.py risks diverging in a way that's detrimental to our goals here, in my opinion.

I agree that separate tooling is sub-optimal. Candidly, I think refactoring trace.py such that it can handle both C and Go is more than I'd want to take on. I'll put together gotrace.py and revisit.

Implemented #1719 as a first cut of this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

anisse picture anisse  路  8Comments

avgsg picture avgsg  路  6Comments

axot picture axot  路  3Comments

alvenwong picture alvenwong  路  7Comments

brendangregg picture brendangregg  路  4Comments