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

Check a call function is available for a probe type #1646

Merged
merged 4 commits into from
Nov 24, 2020
Merged

Check a call function is available for a probe type #1646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db52afd
Add a function to see if the functin is available for that probe type
mmisono Nov 21, 2020
6376b24
Add test to check reg is not available for kfunc
mmisono Nov 21, 2020
39f7358
Update changelog
mmisono Nov 22, 2020
ca5a358
Call check_available at the begining of the analysis
mmisono Nov 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ and this project adheres to
- [#1623](https://github.com/iovisor/bpftrace/pull/1623)
- Fix tracing of usdt probes across namespaces
- [#1637](https://github.com/iovisor/bpftrace/pull/1637)
- Disable reg() for kfunc
- [#1646](https://github.com/iovisor/bpftrace/pull/1646)

#### Tools
- Hook up execsnoop.bt script onto `execveat` call
Expand Down
122 changes: 89 additions & 33 deletions src/ast/semantic_analyser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,15 @@ void SemanticAnalyser::visit(Call &call)
}
}

for (auto &ap : *probe_->attach_points)
{
if (!check_available(call, *ap))
{
LOG(ERROR, call.loc, err_) << call.func << " can not be used with \""
<< ap->provider << "\" probes";
}
}

if (call.func == "hist") {
check_assignment(call, true, false, false);
check_nargs(call, 1);
Expand Down Expand Up @@ -766,15 +775,6 @@ void SemanticAnalyser::visit(Call &call)
}
else if (call.func == "reg") {
if (check_nargs(call, 1)) {
for (auto &attach_point : *probe_->attach_points) {
ProbeType type = probetype(attach_point->provider);
if (type == ProbeType::tracepoint) {
LOG(ERROR, call.loc, err_)
<< "The reg function cannot be used with 'tracepoint' probes";
continue;
}
}

if (check_arg(call, Type::string, 0, true)) {
auto reg_name = bpftrace_.get_string_literal(call.vargs->at(0));
int offset = arch::offset(reg_name);;
Expand Down Expand Up @@ -809,15 +809,6 @@ void SemanticAnalyser::visit(Call &call)
auto name = bpftrace_.get_string_literal(call.vargs->at(0));
for (auto &ap : *probe_->attach_points)
{
ProbeType type = probetype(ap->provider);
if (type != ProbeType::usdt && type != ProbeType::uretprobe &&
type != ProbeType::uprobe)
{
LOG(ERROR, call.loc, err_)
<< "uaddr can only be used with u(ret)probes and usdt probes";
sizes.push_back(0);
continue;
}
struct symbol sym = {};
int err = bpftrace_.resolve_uname(name, &sym, ap->target);
if (err < 0 || sym.address == 0)
Expand Down Expand Up @@ -1074,21 +1065,6 @@ void SemanticAnalyser::visit(Call &call)
LOG(ERROR, call.loc, err_)
<< "signal only accepts string literals or integers";
}

for (auto &ap : *probe_->attach_points) {
ProbeType type = probetype(ap->provider);
if (ap->provider == "BEGIN" || ap->provider == "END") {
LOG(ERROR, call.loc, err_) << call.func << " can not be used with \""
<< ap->provider << "\" probes";
}
else if (type == ProbeType::interval
|| type == ProbeType::software
|| type == ProbeType::hardware
|| type == ProbeType::watchpoint) {
LOG(ERROR, call.loc, err_) << call.func << " can not be used with \""
<< ap->provider << "\" probes";
}
}
}
else if (call.func == "sizeof")
{
Expand Down Expand Up @@ -2820,6 +2796,86 @@ bool SemanticAnalyser::check_symbol(const Call &call, int arg_num __attribute__(
return true;
}

bool SemanticAnalyser::check_available(const Call &call, const AttachPoint &ap)
{
auto &func = call.func;
ProbeType type = probetype(ap.provider);

if (func == "reg")
{
switch (type)
{
case ProbeType::kprobe:
case ProbeType::kretprobe:
case ProbeType::uprobe:
case ProbeType::uretprobe:
case ProbeType::usdt:
case ProbeType::profile:
case ProbeType::interval:
case ProbeType::software:
case ProbeType::hardware:
case ProbeType::watchpoint:
return true;
case ProbeType::invalid:
case ProbeType::tracepoint:
case ProbeType::kfunc:
case ProbeType::kretfunc:
return false;
}
}
else if (func == "uaddr")
{
switch (type)
{
case ProbeType::usdt:
case ProbeType::uretprobe:
case ProbeType::uprobe:
return true;
case ProbeType::invalid:
case ProbeType::kprobe:
case ProbeType::kretprobe:
case ProbeType::tracepoint:
case ProbeType::profile:
case ProbeType::interval:
case ProbeType::software:
case ProbeType::hardware:
case ProbeType::watchpoint:
case ProbeType::kfunc:
case ProbeType::kretfunc:
return false;
}
}
else if (func == "signal")
{
if (ap.provider == "BEGIN" || ap.provider == "END")
return false;
switch (type)
{
case ProbeType::kprobe:
case ProbeType::kretprobe:
case ProbeType::uprobe:
case ProbeType::uretprobe:
case ProbeType::usdt:
case ProbeType::tracepoint:
case ProbeType::profile:
case ProbeType::kfunc:
case ProbeType::kretfunc:
return true;
case ProbeType::invalid:
case ProbeType::interval:
case ProbeType::software:
case ProbeType::hardware:
case ProbeType::watchpoint:
return false;
}
}

if (type == ProbeType::invalid)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move this to the top and then remove all the invalid cases from the switch statements? Or does that trigger a missing case warning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, -Wswitch emits warnings. warning: enumeration value 'invalid' not handled in switch [-Wswitch]

return false;

return true;
}

/*
* assign_map_type
*
Expand Down
1 change: 1 addition & 0 deletions src/ast/semantic_analyser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class SemanticAnalyser : public ASTVisitor
bool want_literal = false,
bool fail = true);
bool check_symbol(const Call &call, int arg_num);
bool check_available(const Call &call, const AttachPoint &ap);

void check_stack_call(Call &call, bool kernel);

Expand Down
2 changes: 1 addition & 1 deletion tests/mocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class MockBPFtrace : public BPFtrace {
{
return -1;
}
else if (name[0] > 'A' && name[0] < 'z')
else if (name[0] >= 'A' && name[0] <= 'z')
{
sym->address = 12345;
sym->size = 4;
Expand Down
5 changes: 5 additions & 0 deletions tests/semantic_analyser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2042,6 +2042,11 @@ TEST_F(semantic_analyser_btf, kfunc)
// func_* have different args, one of them
// is used in probe code, we can't continue -> FAIL
test("kfunc:func_* { $x = args->foo1; }", 0, true, false, 1);
// reg() is not available in kfunc
#ifdef ARCH_X86_64
test("kfunc:func_1 { reg(\"ip\") }", 1);
test("kretfunc:func_1 { reg(\"ip\") }", 1);
#endif
}

TEST_F(semantic_analyser_btf, short_name)
Expand Down