-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Access to uprobe arguments by name #1994
Conversation
9be3cfc
to
bce952c
Compare
tests/data/dwarf_data.h
Outdated
// } | ||
|
||
// generated by: | ||
// % cat a.c |
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.
would be nice to have cmake do this
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.
Good idea, I tried to do so in the last commit.
Would be nice if we could do the same for BTF data in tests/data/bft_data.h
. I actually have it prepared, the problem is that it requires pahole version >=1.17 which is not available in our CI environment.
Nice! Need to find the time to dive into this and test it. But it looks good |
bce952c
to
992982e
Compare
Create a new function that generates LLVM code for reading registers from context (used for "argN", "retval", and "func") builtins.
Many variables and fields related to named probe arguments are now kfunc-specific. As they will be extended to other probe types in future, we must rename them accordingly. This commit does multiple things: - rename "kfarg" to "funcarg" - rename "btf_ap_args" to "ap_args" - introduce a new typedef ProbeArgs
Postpone resolution of kfunc arguments to the point when usage of "args" or "retval" is detected in the program. This way, the arguments are resolved only if they will be really needed. In addition, this makes the FieldAnalyser code simpler and prepares it for extension with uprobe argument resolution.
Missing some of the flags may cause unexpected and hard to discover errors.
992982e
to
f658772
Compare
add_custom_target(testing_debuginfo_data | ||
COMMAND | ||
${CMAKE_COMMAND} | ||
-DDATA_SOURCE_DIR="${CMAKE_SOURCE_DIR}/tests/data" | ||
-P ${CMAKE_SOURCE_DIR}/tests/data/generate_debuginfo_data.cmake) |
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.
I'm not sure this is correct. Will this regen the final output if the data_source.c
is changed?
Instead, how about using add_custom_command() with a shell script? The output file is the generated header. The input files are the shell script and data_source.c
. And then the following line add_dependencies
stays the same and I think it will "just work". Should be a littler easier to grok/program too.
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.
I'm not sure this is correct. Will this regen the final output if the
data_source.c
is changed?
Yes, it will. CMake will handle that.
Instead, how about using add_custom_command() with a shell script? The output file is the generated header. The input files are the shell script and
data_source.c
. And then the following lineadd_dependencies
stays the same and I think it will "just work". Should be a littler easier to grok/program too.
In fact, using add_custom_command() was my original approach, but I wasn't able to make it work. I just couldn't make CMake create the correct rule for the target file.
Using a shell script instead of the CMake script is possible, but CMake allows to use configure_file() which gives quite a nice control of the contents of the generated header.
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.
Any chance this example might help? https://github.com/libbpf/libbpf-bootstrap/blob/67a29e511cc9d0a570d4d3b9797827f3a08ccdb5/tools/cmake/FindBpfObject.cmake#L163-L170
I'm ok to merge this as is too
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.
The example suggests to use target_sources() to create the dependency between the target (bpftrace_test
) and the generated header, but unfortunately this doesn't rebuild bpftrace_test
when data_source.c
is changed.
It's probably possible to make it work somehow, but likely it will be more complicated than the current approach.
db8a495
to
2b23724
Compare
If a binary has DWARF available, allow to access arguments by name. Uses "args" builtin for this (the same way as kfunc/tracepoin do). Supports only arguments of integer and pointer types for now.
Uses mocking of DWARF data similar to what we use for BTF - creating a temporary ELF file from a prepared byte array. To keep it minimal, the ELF contains only the DWARF sections. Currently used for testing access to uprobe arguments by name.
2b23724
to
a366850
Compare
a366850
to
a47b9b2
Compare
Unit tests for DWARF-based features rely on mocked DWARF data that is stored in tests/data/dwarf_data.h in the form of a byte array. Instead of creating the header manually, we now generate it by CMake. This requires the `xxd` tool - add it to Docker images.
a47b9b2
to
ea4e7fb
Compare
Allows to reference uprobe arguments by name, with automatic type resolution:
Arguments are accessed via the
args
builtin (same as with tracepoints and kfuncs). This feature relies on DWARF to obtain argument names and types, so the traced binary must be compiled with DWARF.Currently supports only basic types (integers, chars, enums) and pointers to them. Support for C structs will follow, but it'll require more extensive changes, so I'd prefer doing it in a separate PR.
Related issue #1742 (will close it once at least all C types are fully supported).
This PR also introduces unit testing for DWARF-based features.
Checklist
man/adoc/bpftrace.adoc
and if needed indocs/reference_guide.md
CHANGELOG.md