Skip to content
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

Merged
merged 7 commits into from
Oct 6, 2021

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Sep 7, 2021

Allows to reference uprobe arguments by name, with automatic type resolution:

# bpftrace -lv 'uprobe:tests/testprogs/uprobe_test:function1'
uprobe:tests/testprogs/uprobe_test:function1
    int* n
    char c

# bpftrace -e 'uprobe:tests/testprogs/uprobe_test:function1 { printf("%d\n", *(args->n)); }'
Attaching 1 probe...
13

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
  • Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

// }

// generated by:
// % cat a.c
Copy link
Contributor

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

Copy link
Contributor Author

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.

tests/dwarf_common.h Outdated Show resolved Hide resolved
@fbs
Copy link
Contributor

fbs commented Sep 10, 2021

Nice! Need to find the time to dive into this and test it. But it looks good

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.
src/ast/passes/semantic_analyser.cpp Show resolved Hide resolved
src/dwarf_parser.h Outdated Show resolved Hide resolved
Comment on lines +47 to +51
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)
Copy link
Member

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.

Copy link
Contributor Author

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 line add_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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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.

@viktormalik viktormalik force-pushed the uprobe-named-args branch 2 times, most recently from db8a495 to 2b23724 Compare October 1, 2021 05:52
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.
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.
@viktormalik viktormalik merged commit 3f8dd63 into bpftrace:master Oct 6, 2021
@viktormalik viktormalik deleted the uprobe-named-args branch November 24, 2021 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants