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.

Sign up for GitHub

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

Jump to bottom

Support parsing bitfields from BTF/DWARF #2505

Merged
merged 5 commits into from
Mar 29, 2023
Merged

Support parsing bitfields from BTF/DWARF #2505

merged 5 commits into from
Mar 29, 2023

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Feb 13, 2023 •
edited

Since #2315, we do not run Clang parser if we can resolve the necessary types from BTF/DWARF in field analyser. Until now, this was not possible for structs that contained bitfields, this PR adds support for that.

The implementation is quite straightforward, reusing the bitfield parsing algorithm from ClangParser, only filling it with correct values extracted from BTF/DWARF. The only problem is that DWARF uses different approaches for representing bitfield offsets in versions <4 and >=4 while some existing systems (such as the one in CI) use the older approach. Therefore, I implemented support for both. See the code comments for implementation details.

A part of this PR is also generation of testing BTF data automatically during the build (in the same way that is already used for DWARF data). This requires several additional tools, mainly pahole which I had to build manually on Alpine.

Fixes #2212, fixes #2500, closes #2501. Since the 0.17.0 release caused a regression (see #2500) which is resolved by this PR, we should consider releasing this into 0.17.1 (@fbs wdyt?).

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

@viktormalik viktormalik mentioned this pull request Feb 13, 2023
Closed
danobi
danobi approved these changes Mar 22, 2023
View reviewed changes
Copy link
Member

@danobi danobi left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Just one comment about cmake stuff though.

tests/CMakeLists.txt Show resolved Hide resolved
@viktormalik viktormalik force-pushed the bitfields branch 2 times, most recently from 468534b to 44bf210 Compare March 29, 2023 05:21
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 29, 2023
View reviewed changes
tests/data/data_source.c Fixed Show fixed Hide fixed
tests/data/data_source.c Fixed Show fixed Hide fixed
tests/data/data_source.c Fixed Show fixed Hide fixed
tests/data/data_source.c Fixed Show fixed Hide fixed
tests/data/data_source.c Fixed Show fixed Hide fixed
tests/data/data_source.c Fixed Show fixed Hide fixed
@viktormalik
Let CMake generate BTF data for unit tests
6f7ffd0 
This is done similarly to generating mocked DWARF data, using the same
source data.

Requires several utilities: pahole, llvm-objcopy, xxd, nm, awk.

Especially llvm-objcopy may be troublesome as it is required by pahole
but it is often named llvm-objcopy-XX or llvmXX-objcopy where XX is the
LLVM version.  Hence, we set the LLVM_OBJCOPY env variable which pahole
is able to process.

Pahole is not packaged in Alpine 3.12 which is used in embedded CI
tests. Since I was not able to bump Alpine to any newer version that
would have pahole, we build it from source for now.
@viktormalik
Refactor work with bitfields
7696121 
Two important refactorings were done:

- Bitfield info is now stored inside Field as std::optional<Bitfield>
  (instead of bool is_bitfield + Bitfield). This is more convenient to
  work with but required some changes, especially to serialization and
  tests.

- Moved algorithm for computing bitfield information from ClangParser to
  the Bitfield class since the same algorithm will be used for
  generating bitfields from BTF and DWARF.

Requires libcereal version >= 1.3.0 (support for std::optional), install
it manually in the Docker image for embedded builds (focal has 1.2.1).
@viktormalik
Parse bitfields from BTF
3c0a239 
Re-using the existing bitfield parsing algorithm (originally from Clang
parser). Add a unit test for bitfield parsing (FieldAnalyser). The test
required to expand the testing BTF data.
@viktormalik
Parse bitfields from DWARF
8ba34f2 
Re-using the existing bitfield parsing algorithm.

The problem is that DWARF versions >=4 use a different approach to
represent bitfield offsets from the one used in versions <4 (in
addition, some systems, such as the one in the CI, use the old approach
even for DWARF 4). Hence, we support both approaches. See code comments
for implementation details.

A new unit test for bitfield parsing is added to FieldAnalyser,
supporting both variants of bitfield representation.
@viktormalik
Update CHANGELOG (bitfield parsing from BTF/DWARF)
822589a
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 29, 2023
View reviewed changes
tests/data/data_source.c
int main(void)
{
struct bpf_iter__task iter_task;

Check notice

Code scanning / CodeQL

Unused local variable

Variable iter_task is not used.
tests/data/data_source.c
int main(void)
{
struct bpf_iter__task iter_task;
struct bpf_iter__task_file iter_task_file;

Check notice

Code scanning / CodeQL

Unused local variable

Variable iter_task_file is not used.
tests/data/data_source.c
int main(void)
{
struct bpf_iter__task iter_task;
struct bpf_iter__task_file iter_task_file;
struct bpf_iter__task_vma iter_task_vma;

Check notice

Code scanning / CodeQL

Unused local variable

Variable iter_task_vma is not used.
tests/data/data_source.c
int a : 8;
int b : 1;
int c : 3;
int d : 20;

Check warning

Code scanning / CodeQL

Ambiguously signed bit-field member

Bit field d of type int should have explicitly unsigned integral, explicitly signed integral, or enumeration type.
tests/data/data_source.c
int : 12; // padding
int a : 8;
int b : 1;
int c : 3;

Check warning

Code scanning / CodeQL

Ambiguously signed bit-field member

Bit field c of type int should have explicitly unsigned integral, explicitly signed integral, or enumeration type.
tests/data/data_source.c
int pgid;
int : 12; // padding
int a : 8;
int b : 1;

Check warning

Code scanning / CodeQL

Ambiguously signed bit-field member

Bit field b of type int should have explicitly unsigned integral, explicitly signed integral, or enumeration type.
tests/data/data_source.c
int pid;
int pgid;
int : 12; // padding
int a : 8;

Check warning

Code scanning / CodeQL

Ambiguously signed bit-field member

Bit field a of type int should have explicitly unsigned integral, explicitly signed integral, or enumeration type.
@viktormalik viktormalik merged commit a2c2308 into bpftrace:master Mar 29, 2023
@viktormalik viktormalik deleted the bitfields branch March 29, 2023 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danobi danobi danobi approved these changes

@ajor ajor Awaiting requested review from ajor ajor is a code owner

@fbs fbs Awaiting requested review from fbs fbs is a code owner

@BurntBrunch BurntBrunch Awaiting requested review from BurntBrunch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@viktormalik @danobi