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

Allow uprobe placement on arbitrary addresses when --unsafe is used. #1388

Merged
merged 1 commit into from
Jul 3, 2020

Conversation

mzr
Copy link
Contributor

@mzr mzr commented Jun 18, 2020

Follow-up to #1282.
When there is no symbol found for requested address of uprobe placement and --unsafe is set, warning is emitted, and uprobe is placed as normally.

Checklist
  • Language changes are updated in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md

@fbs
Copy link
Member

fbs commented Jun 23, 2020

nice, thanks for working on this!

Unsafe uprobe:bin:4377 not matching any symbol at address 0x1119

I think the message can be improved a bit. Matching a symbol isn't a requirement for bpftrace. All it wants to know is that it is not inserting a probe in the middle of an instruction and then crashing the tracee. Maybe something along the lines of WARNING: could not determine instruction boundary (binary appears stripped), misaligned probes can lead to tracee crashes

@mzr
Copy link
Contributor Author

mzr commented Jun 23, 2020

I don't think that's the correct warning either. At least it doesn't fit into the control flow in the source code. Originally it failed because there was no symbol defined at that address (if (!sym.start)).
Also as far as I understand, alignment within an instruction is checked later by calling the check_alignment.

How about

WARNING: Unsafe uprobe:bin:4377 not matching any symbol at address 0x1119. Skipping all alignment checks such as instruction boundary. Misaligned probes can lead to tracee crashes.

?

@fbs
Copy link
Member

fbs commented Jun 24, 2020

I don't think that's the correct warning either. At least it doesn't fit into the control flow in the source code. Originally it failed because there was no symbol defined at that address (if (!sym.start)).

Having a sym.start doesn't mean that there exists a symbol at that specific address. What it does, through the sym_address_cb, is finding a "function" that contains that address (func_start <= address <= func_end). That way it can use func_start as a starting point for checking instruction boundaries as func_start will always be aligned properly.

!sym.start can mean that the file is (partly) stripped from the required symbols or that the address is actually outside of function boundaries the current code doesn't really know which it is.

Maybe we could add some checks that at least verify that the address is not completely out of bounds, but that can be done in a later pr too.

@fbs
Copy link
Member

fbs commented Jun 29, 2020

Can you squash the commits into one? :)

@fbs
Copy link
Member

fbs commented Jul 2, 2020

@danobi what do you think of the warning message?

@fbs fbs added this to the 0.11.0 milestone Jul 2, 2020
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 to me. But I think documentation needs updating

docs/reference_guide.md Outdated Show resolved Hide resolved
@danobi
Copy link
Member

danobi commented Jul 3, 2020

Thanks!

@danobi danobi merged commit 24205c9 into bpftrace:master Jul 3, 2020
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.

3 participants