-
-
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
Migrate from objdump and grep to library calls #410
Migrate from objdump and grep to library calls #410
Conversation
Thanks! I'd merge it but we need to figure out how to handle this:
I guess I have an older bcc version installed. |
It looks like bcc_elf.h was separated from bcc_helpers.h around bcc 0.2 or so. I haven't been able to build that version locally, but hopefully the cmake workaround that I just pushed will work. I'm no cmake expert, though, so suggestions are welcome. |
Now get:
|
Hmm, okay. I'll investigate this more. What version of bcc do you have installed? |
This seems similar to the issue reported in this comment from #335, but I don't think it's a bcc version problem. The libbpfcc-dev package on Ubuntu 18.04 doesn't include bcc_elf.h despite its being present in bcc 0.5.0. The upstream Debian package has it, and the libbcc package from the iovisor apt repo also has it. I can see the symbols in the library:
How can we fix this? Should I open an issue on the bcc repo? |
@brendangregg ping. Any recommendations here? |
6b91c04
to
8664160
Compare
I need to setup a fresh environment and retest to see if it was just a problem with my dev environment (my dev environments get messy since I'm always building and testing things). |
I ended up doing the same. It was working for me on Ubuntu 16.04, but I ran into the missing header issue on 18.04. Seems like a bug with the libbpfcc-dev package. |
@acj it should be working now with bcc Also, should we try to keep it backwards compatible (keep using objdump for older bcc versions)? |
I would like to merge this since it will bring a big improvement on startup time once #557 is merged as well. |
Sorry for the delay. Great to hear about the startup time improvements! I'll test this against bcc master tomorrow morning. If we want to merge soon, then I agree that backwards compatibility is a good move. Do we have a reliable way to get the installed bcc version? If not, I can probably branch based on the presence of bcc_elf.h. |
We tend to use feature detection for compatibility with multiple bcc versions. Here's one example: c03c39f |
8664160
to
fdd02ec
Compare
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.
LGTM
Looks good, thanks! |
Fixes #128.