Skip to content

Commit

Permalink
Semantic Analyser: Update map key types on the first pass
Browse files Browse the repository at this point in the history 
This is more pre-work for for-each loops over maps, as it is necessary
to know map key types when looping over elements.

This change required also setting map.skip_key_validation before the map
is first visited.

The tests needed to be updated as the returned error code from
SemanticAnalyser changed. Took the opportunity to check the error
messages for them too.
  • Loading branch information
@ajor
ajor committed Feb 20, 2024
1 parent 8f513d0 commit 5d5c461
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 28 deletions.
23 changes: 16 additions & 7 deletions src/ast/passes/semantic_analyser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,14 @@ void SemanticAnalyser::visit(Builtin &builtin)
}
}

namespace {
bool skip_key_validation(const Call &call)
{
return call.func == "print" || call.func == "clear" || call.func == "zero" ||
call.func == "len";
}
} // namespace

void SemanticAnalyser::visit(Call &call)
{
// Check for unsafe-ness first. It is likely the most pertinent issue
Expand Down Expand Up @@ -462,10 +470,15 @@ void SemanticAnalyser::visit(Call &call)

if (call.vargs) {
for (size_t i = 0; i < call.vargs->size(); ++i) {
auto &expr = (*call.vargs)[i];
auto &expr = *(*call.vargs)[i];
func_arg_idx_ = i;

expr->accept(*this);
if (expr.is_map && skip_key_validation(call)) {
Map &map = static_cast<Map &>(expr);
map.skip_key_validation = true;
}

expr.accept(*this);
}
}

Expand Down Expand Up @@ -917,7 +930,6 @@ void SemanticAnalyser::visit(Call &call)
auto &arg = *call.vargs->at(0);
if (arg.is_map) {
Map &map = static_cast<Map &>(arg);
map.skip_key_validation = true;
if (map.vargs != nullptr) {
LOG(ERROR, call.loc, err_)
<< "The map passed to " << call.func << "() should not be "
Expand Down Expand Up @@ -969,7 +981,6 @@ void SemanticAnalyser::visit(Call &call)
LOG(ERROR, call.loc, err_) << "clear() expects a map to be provided";
else {
Map &map = static_cast<Map &>(arg);
map.skip_key_validation = true;
if (map.vargs != nullptr) {
LOG(ERROR, call.loc, err_)
<< "The map passed to " << call.func << "() should not be "
Expand All @@ -985,7 +996,6 @@ void SemanticAnalyser::visit(Call &call)
LOG(ERROR, call.loc, err_) << "zero() expects a map to be provided";
else {
Map &map = static_cast<Map &>(arg);
map.skip_key_validation = true;
if (map.vargs != nullptr) {
LOG(ERROR, call.loc, err_)
<< "The map passed to " << call.func << "() should not be "
Expand All @@ -1000,7 +1010,6 @@ void SemanticAnalyser::visit(Call &call)
LOG(ERROR, call.loc, err_) << "len() expects a map to be provided";
else {
Map &map = static_cast<Map &>(arg);
map.skip_key_validation = true;
if (map.vargs != nullptr) {
LOG(ERROR, call.loc, err_)
<< "The map passed to " << call.func << "() should not be "
Expand Down Expand Up @@ -1373,7 +1382,7 @@ void SemanticAnalyser::visit(Map &map)
}
}

if (pass_ > 1 && !map.skip_key_validation)
if (!map.skip_key_validation)
update_key_type(map, key);

auto search_val = map_val_.find(map.ident);
Expand Down
79 changes: 58 additions & 21 deletions tests/semantic_analyser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,18 +280,45 @@ TEST(semantic_analyser, consistent_map_values)

TEST(semantic_analyser, consistent_map_keys)
{
test("kprobe:f { @x = 0; @x; }", 0);
test("kprobe:f { @x[1] = 0; @x[2]; }", 0);
test("BEGIN { @x = 0; @x; }");
test("BEGIN { @x[1] = 0; @x[2]; }");

test("kprobe:f { @x = 0; @x[1]; }", 2);
test("kprobe:f { @x[1] = 0; @x; }", 2);
test_error("BEGIN { @x = 0; @x[1]; }", R"(
stdin:1:17-22: ERROR: Argument mismatch for @x: trying to access with arguments: [unsigned int64] when map expects arguments: []
BEGIN { @x = 0; @x[1]; }
~~~~~
)");
test_error("BEGIN { @x[1] = 0; @x; }", R"(
stdin:1:20-22: ERROR: Argument mismatch for @x: trying to access with arguments: [] when map expects arguments: [unsigned int64]
BEGIN { @x[1] = 0; @x; }
~~
)");

test("kprobe:f { @x[1,2] = 0; @x[3,4]; }", 0);
test("kprobe:f { @x[1,2] = 0; @x[3]; }", 2);
test("kprobe:f { @x[1] = 0; @x[2,3]; }", 2);
test("BEGIN { @x[1,2] = 0; @x[3,4]; }");

test("kprobe:f { @x[1,\"a\",kstack] = 0; @x[2,\"b\", kstack]; }", 0);
test("kprobe:f { @x[1,\"a\",kstack] = 0; @x[\"b\", 2, kstack]; }", 2);
test_error("BEGIN { @x[1,2] = 0; @x[3]; }", R"(
stdin:1:22-27: ERROR: Argument mismatch for @x: trying to access with arguments: [unsigned int64] when map expects arguments: [unsigned int64, unsigned int64]
BEGIN { @x[1,2] = 0; @x[3]; }
~~~~~
)");
test_error("BEGIN { @x[1] = 0; @x[2,3]; }", R"(
stdin:1:20-27: ERROR: Argument mismatch for @x: trying to access with arguments: [unsigned int64, unsigned int64] when map expects arguments: [unsigned int64]
BEGIN { @x[1] = 0; @x[2,3]; }
~~~~~~~
)");

test("BEGIN { @x[1,\"a\",kstack] = 0; @x[2,\"b\", kstack]; }");

test_error(R"(
BEGIN {
@x[1,"a",kstack] = 0;
@x["b", 2, kstack];
})",
R"(
stdin:3:7-25: ERROR: Argument mismatch for @x: trying to access with arguments: [string[2], unsigned int64, kstack] when map expects arguments: [unsigned int64, string[2], kstack]
@x["b", 2, kstack];
~~~~~~~~~~~~~~~~~~
)");
}

TEST(semantic_analyser, if_statements)
Expand Down Expand Up @@ -1151,12 +1178,17 @@ TEST(semantic_analyser, array_as_map_key)
0);

// Mismatched key types
test("struct MyStruct { int x[2]; int y[4]; }"
"kprobe:f { "
" @x[((struct MyStruct *)arg0)->x] = 0; "
" @x[((struct MyStruct *)arg0)->y] = 1; "
"}",
2);
test_error(R"(
struct MyStruct { int x[2]; int y[4]; }
BEGIN {
@x[((struct MyStruct *)0)->x] = 0;
@x[((struct MyStruct *)0)->y] = 1;
})",
R"(
stdin:4:7-37: ERROR: Argument mismatch for @x: trying to access with arguments: [int32[4]] when map expects arguments: [int32[2]]
@x[((struct MyStruct *)0)->y] = 1;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
)");
}

TEST(semantic_analyser, array_compare)
Expand Down Expand Up @@ -1821,12 +1853,17 @@ TEST(semantic_analyser, struct_as_map_key)
0);

// Mismatched key types
test("struct A { int x; } struct B { char x; } "
"kprobe:f { "
" @x[*((struct A *)arg0)] = 0; "
" @x[*((struct B *)arg1)] = 1; "
"}",
2);
test_error(R"(
struct A { int x; } struct B { char x; }
BEGIN {
@x[*((struct A *)0)] = 0;
@x[*((struct B *)0)] = 1;
})",
R"(
stdin:4:9-30: ERROR: Argument mismatch for @x: trying to access with arguments: [struct B] when map expects arguments: [struct A]
@x[*((struct B *)0)] = 1;
~~~~~~~~~~~~~~~~~~~~~
)");
}

TEST(semantic_analyser, probe_short_name)
Expand Down

0 comments on commit 5d5c461

Please sign in to comment.