Skip to content

Commit 7cfb563

Browse files
Jordan Romejordalgo
Jordan Rome
authored and
jordalgo
committed
Fix crash using some per-cpu maps as map keys
'count', 'sum', 'avg', 'min', 'max' are all castable per-cpu map aggregation types so they should be valid as map keys when printing. Also add semantic analyser checks for invalid per-cpu map aggregation types. Issue: #3591
1 parent a18d8bb commit 7cfb563

File tree

6 files changed

+49
-5
lines changed

6 files changed

+49
-5
lines changed

‎.github/include/aot_skip.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ aot.other.no symbolize enum after arithmetic
9191
aot.other.no symbolize enum after arithmetic mixed
9292
aot.other.per_cpu_map_arithmetic
9393
aot.other.per_cpu_map_avg_if
94+
aot.other.per_cpu_map_as_map_key
9495
# Enum metadata is not being serialized
9596
aot.other.symbolize enum in map key
9697
# Enum metadata is not being serialized

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@ and this project adheres to
112112
- [#3542](https://github.com/bpftrace/bpftrace/pull/3542)
113113
- Fix field access and offsetof for strings that are builtin types
114114
- [#3565](https://github.com/bpftrace/bpftrace/pull/3565)
115+
- Fix crash when using castable per-cpu map types as map keys
116+
- [#3594](https://github.com/bpftrace/bpftrace/pull/3594)
115117
#### Security
116118
#### Docs
117119
- Remove mention of unsupported character literals

‎src/ast/passes/semantic_analyser.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,6 +1672,10 @@ void SemanticAnalyser::validate_map_key(const SizedType &key,
16721672
LOG(ERROR, loc, err_) << "context cannot be used as a map key";
16731673
}
16741674

1675+
if (key.IsHistTy() || key.IsLhistTy() || key.IsStatsTy()) {
1676+
LOG(ERROR, loc, err_) << key << " cannot be used as a map key";
1677+
}
1678+
16751679
if (is_final_pass() && key.IsNoneTy()) {
16761680
LOG(ERROR, loc, err_) << "Invalid expression for assignment: ";
16771681
}

‎src/output.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,11 @@ std::string Output::map_key_str(BPFtrace &bpftrace,
452452
case Type::array:
453453
case Type::mac_address:
454454
case Type::record:
455+
case Type::count_t:
456+
case Type::avg_t:
457+
case Type::max_t:
458+
case Type::min_t:
459+
case Type::sum_t:
455460
return value_to_str(bpftrace, arg, data, false, 1, true);
456461
case Type::tuple: {
457462
std::vector<std::string> elems;
@@ -466,17 +471,12 @@ std::string Output::map_key_str(BPFtrace &bpftrace,
466471
}
467472
case Type::cgroup_path_t:
468473
case Type::strerror_t:
469-
case Type::avg_t:
470-
case Type::count_t:
471474
case Type::hist_t:
472475
case Type::lhist_t:
473-
case Type::max_t:
474-
case Type::min_t:
475476
case Type::none:
476477
case Type::reference:
477478
case Type::stack_mode:
478479
case Type::stats_t:
479-
case Type::sum_t:
480480
case Type::timestamp_mode:
481481
case Type::voidtype:
482482
LOG(BUG) << "Invalid mapkey argument type: " << arg;

‎tests/runtime/other‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,10 @@ NAME per_cpu_map_cast
275275
PROG BEGIN { @a = count(); @b = sum(10); printf("%d-%d\n", (uint64)@a, (int64)@b); exit();}
276276
EXPECT 1-10
277277

278+
NAME per_cpu_map_as_map_key
279+
PROG BEGIN {@a = count(); @b = sum(5); @c = min(1); @d = max(10); @e = avg(7); @[@a, @b, @c, @d, @e] = 1; exit(); }
280+
EXPECT @[1, 5, 1, 10, 7]: 1
281+
278282
NAME dry run empty output
279283
RUN {{BPFTRACE}} --dry-run -e 'BEGIN { printf("hello\n"); @ = 0; }'
280284
EXPECT_NONE hello

‎tests/semantic_analyser.cpp‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,9 @@ kprobe:f { $x = hist(1); }
692692
)");
693693
test_error("kprobe:f { @x[hist(1)] = 1; }", R"(
694694
stdin:1:12-22: ERROR: hist() should be directly assigned to a map
695+
kprobe:f { @x[hist(1)] = 1; }
696+
~~~~~~~~~~
697+
stdin:1:12-22: ERROR: hist_t cannot be used as a map key
695698
kprobe:f { @x[hist(1)] = 1; }
696699
~~~~~~~~~~
697700
)");
@@ -764,6 +767,9 @@ stdin:1:12-21: ERROR: lhist() should be directly assigned to a map
764767
kprobe:f { @[lhist()] = 1; }
765768
~~~~~~~~~
766769
stdin:1:12-21: ERROR: lhist() requires 4 arguments (0 provided)
770+
kprobe:f { @[lhist()] = 1; }
771+
~~~~~~~~~
772+
stdin:1:12-21: ERROR: lhist_t cannot be used as a map key
767773
kprobe:f { @[lhist()] = 1; }
768774
~~~~~~~~~
769775
)");
@@ -2556,6 +2562,33 @@ stdin:4:9-30: ERROR: Argument mismatch for @x: trying to access with arguments:
25562562
)");
25572563
}
25582564

2565+
TEST(semantic_analyser, per_cpu_map_as_map_key)
2566+
{
2567+
test("BEGIN { @x = count(); @y[@x] = 1; }");
2568+
test("BEGIN { @x = sum(10); @y[@x] = 1; }");
2569+
test("BEGIN { @x = min(1); @y[@x] = 1; }");
2570+
test("BEGIN { @x = max(1); @y[@x] = 1; }");
2571+
test("BEGIN { @x = avg(1); @y[@x] = 1; }");
2572+
2573+
test_error("BEGIN { @x = hist(10); @y[@x] = 1; }", R"(
2574+
stdin:1:24-29: ERROR: hist_t cannot be used as a map key
2575+
BEGIN { @x = hist(10); @y[@x] = 1; }
2576+
~~~~~
2577+
)");
2578+
2579+
test_error("BEGIN { @x = lhist(10, 0, 10, 1); @y[@x] = 1; }", R"(
2580+
stdin:1:35-40: ERROR: lhist_t cannot be used as a map key
2581+
BEGIN { @x = lhist(10, 0, 10, 1); @y[@x] = 1; }
2582+
~~~~~
2583+
)");
2584+
2585+
test_error("BEGIN { @x = stats(10); @y[@x] = 1; }", R"(
2586+
stdin:1:25-30: ERROR: stats_t cannot be used as a map key
2587+
BEGIN { @x = stats(10); @y[@x] = 1; }
2588+
~~~~~
2589+
)");
2590+
}
2591+
25592592
TEST(semantic_analyser, probe_short_name)
25602593
{
25612594
test("t:a:b { args }");

0 commit comments

Comments
 (0)