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

Fix symbolication on 32-bit userspace / 64-bit kernel #2869

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

jeez
Copy link
Contributor

@jeez jeez commented Dec 5, 2023

Symbolication is broken on platforms running a 64-bit kernel and a 32-bit userspace. For example, this simple test case can reproduce the issue: $ bpftrace -e 'profile:hz:99 { @[kstack] = count(); }' .

This is happening because bpftrace requests uint64_t addresses to the kernel here but then performs an implicit conversion to uintptr_t here when resolving ksyms, usyms and uids. Fix the issue by changing the signature of these functions so they receive a uint64_t address instead.

Ideally we should uncomment add_compile_options("-Wconversion") in bpftrace/CMakeLists.txt to avoid running into simple issues like this in the future. This patch is one small step towards fixing the current conversion warnings we have.

Issue 2854

Symbolication is broken on platforms running a 64-bit kernel and a
32-bit userspace. For example, this simple test case can reproduce the
issue: $ bpftrace -e 'profile:hz:99 { @[kstack] = count(); }' .

This is happening because bpftrace requests uint64_t addresses to the
kernel here but then performs an implicit conversion to uintptr_t here
when resolving ksyms, usyms and uids. Fix the issue by changing the
signature of these functions so they receive a uint64_t address instead.

Ideally we should uncomment add_compile_options("-Wconversion") in
bpftrace/CMakeLists.txt to avoid running into simple issues like this in
the future. This patch is one small step towards fixing the current
conversion warnings we have.

[Issue 2854](bpftrace#2854)

Signed-off-by: Jesus Sanchez-Palencia <jesussanp@google.com>
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.

lgtm, thanks!

wondering if we should add a -m32 testprog and add a runtime test to cover this case.

@danobi
Copy link
Member

danobi commented Dec 6, 2023

Nevermind - it's bpftrace that has to run in 32bit mode. Not the test prog.

@danobi danobi merged commit db73f5a into bpftrace:master Dec 6, 2023
18 checks passed
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

2 participants