-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow '.' in idents for golang #166
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd definitely be good to get in support for Go symbols. As it is, this change breaks a few other things in the language though due to "." being used for struct field access. We really need to get the CI working again...
It might be enough to just update the definition of wildcard
in parser.yy with an extra "." rule (not sure though). Also, please add a parser test once this is working!
Thanks for looking. I'm not surprised that it broke other things. I'll see if I can update the parser properly and also run the tests! |
0a0f16e
to
7387a7d
Compare
I pushed an update to this branch implementing this as suggested by @ajor. I also added a test. |
This is actually still insufficient for go. This fixes package level functions but methods on structs are more complicated. Given the following definition:
The symbol shows up as Supporting this seems like it would be more complicated. Would you want to support escaping ( |
Thanks @psanford for working on this! Using your change, I was able to attach to some of my functions using uprobes. I'm hoping we can get Golang to be first class with bpftrace. Another consideration that is relevant to this change is that Golang allows for any unicode letter or digit to be used in an identifier. For example:
This compiles to:
However this doesn't appear to parse well:
Incidentally, I think developing a grammar for escapes might not be the only option we should consider. Another solution would be to just have a literal syntax. Something like this:
This would allow for whatever bytes you want with 0 ambiguity. You would only have to escape the Wildcards could just exist outside the literal:
Or they could exist inside the literal with special meaning and just have an escape for the byte value. Outside of golang support, it should be noted that filenames can have unicode characters as well and that also appears to currently not work. What do you think? |
Thats a good point about unicode that I hadn't considered. Quoting does seem like a better option than trying to do escaping. |
Quoting sounds good. Could probably just reuse the string type to do this |
7387a7d
to
707de90
Compare
Allow quoted string attach points so that you can match more than [a-Z0-9]. This specific issue that this fixes is attaching to go functions. These symbols are in the format '<package>.<function>' and can contain unicode letters and digits.
When specifying a string literal attach point do not try to expand wildcards if they are present. This specifically is required for Go methods on pointer receivers, which have a '*' in their symbol name.
707de90
to
80792a5
Compare
I pushed the change to support quoted strings. This fixes both the unicode issue and the pointer receiver issue. In addition to updating the parser.yy I updated the AttachPoint class to have an explicit bool I left the other attach_point form the same as before (need_expansion=true). |
This looks good to me. Note that there's follow-up work to get uretprobes to work with go sanely; that's been discussed before ( iovisor/bcc#1320 ). |
Thanks! |
@psanford Thanks a lot for doing this! |
Go symbols are in the format 'package.function'. Allow '.' in
ident so that you can attach uprobes to go functions.
Prior to this patch, the following would error:
With this patch applied I am able to trace this function in a go binary.