usdt: Fix attaching to probes in different binaries #1356
Conversation
|
cc @dalehamel |
|
Sorry for the delay, i'll take a look shortly danobi |
| @@ -25,6 +25,7 @@ add_executable(bpftrace | |||
| struct.cpp | |||
| tracepoint_format_parser.cpp | |||
| types.cpp | |||
| usdt.cpp | |||
There was a problem hiding this comment.
nice, much more idiomatic at this point to have it in its own file.
There was a problem hiding this comment.
This looks great, it's more similar to how BCC modularizes their USDT code too, about time we moved out of the util.cpp, which are always a bit of an anti-pattern.
I didn't read over the new usdt.cpp line by line, but it mosty looks like a refactor. Can you point me to the spot where you fixed the bug where attach to only one binary worked? I am curious what the issue was, but I see you added a test case that seems to be passing, so looks like the reorganized code works correctly.
Ah there it is! Thanks for the explanation. LGTM |
This commit just cut and pastes code around. It's nicer to have small, orthogonal modules.
Keeping unused code around is asking for bit rot and bugs. Also hide some methods that are used for internal caching.
USDTHelper caching logic was previously incorrect. It would mark the cache as loaded if any pid or path was processed. This would cause scripts that place usdt probes on different binaries to silently fail (because a default usdt_probe_entry would be returned). This commit breaks up the cache by pid or by path. Also switch usdt_provider_cache to an unordered_map b/c we don't need ordering and unordered data structures are faster. Next commit will get rid of the silent failure behavior.
It's good to have an error reporting mechanism. Returning a default value caused a lot of headache when trying to debug usdt lookup failures that were fixed in the previous commit.
Before:
$ sudo ./build/src/bpftrace -e 'usdt:/usr/bin/python3:python:asdf { 1 }'
terminate called after throwing an instance of 'std::runtime_error'
what(): Failed to find usdt probe: usdt:/usr/bin/python3:python:asdf
[1] 865191 abort sudo ./build/src/bpftrace -e
After:
$ sudo ./build/src/bpftrace -e 'usdt:/usr/bin/python3:python:asdf { 1 }'
Failed to compile: Failed to find usdt probe:
usdt:/usr/bin/python3:python:asdf
danobi
force-pushed
the
usdt_multi_module
branch
from
cf1b8e0 to
70eec47
Compare
USDTHelper caching logic was previously incorrect. It would mark the
cache as loaded if any pid or path was processed. This would cause
scripts that place usdt probes on different binaries to silently fail
(because a default usdt_probe_entry would be returned).
See the runtime test for an example of how this can occur.
Checklist
docs/reference_guide.mdCHANGELOG.md