Skip to content

Commit

Permalink
Fix PR bpftrace#1416: unsigned int overflow bug and unstable tests
Browse files Browse the repository at this point in the history
The merged PR which enables `top` and `div` args of `print` for `avg` maps has
two issues that need addressing:

1. It came with subtractions of unsigned integers which can easily cause
negative overflows.
2. The runtime tests it came with are somehow unstable and fails randomly. This
patch brings more straightforward tests.
  • Loading branch information
suyuee committed Jul 24, 2020
1 parent 991c96c commit b536854
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 19 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ and this project adheres to
#### Removed

#### Fixed
- Fix negative overflow bug and unstable tests in PR #1416
- [#1416](https://github.com/iovisor/bpftrace/pull/1436)

#### Tools

Expand Down
16 changes: 6 additions & 10 deletions src/output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,11 +290,9 @@ void TextOutput::map_stats(
auto &key = key_count.first;
auto &value = values_by_key.at(key);

if (map.type_.IsAvgTy() && top)
{
if (i++ < (values_by_key.size() - top))
continue;
}
if (map.type_.IsAvgTy() && top && values_by_key.size() > top &&
i++ < (values_by_key.size() - top))
continue;

out_ << map.name_ << map.key_.argument_value_list_str(bpftrace, key) << ": ";

Expand Down Expand Up @@ -610,11 +608,9 @@ void JsonOutput::map_stats(
auto &key = key_count.first;
auto &value = values_by_key.at(key);

if (map.type_.IsAvgTy() && top)
{
if (j++ < (values_by_key.size() - top))
continue;
}
if (map.type_.IsAvgTy() && top && values_by_key.size() > top &&
j++ < (values_by_key.size() - top))
continue;

std::vector<std::string> args = map.key_.argument_value_list(bpftrace, key);
if (i > 0)
Expand Down
12 changes: 6 additions & 6 deletions tests/runtime/call
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,12 @@ RUN bpftrace -v -e 'BEGIN { @[nsecs] = strftime("%m/%d/%y", nsecs); exit();}'
EXPECT @\[[0-9]*\]: [0-9]{2}\/[0-9]{2}\/[0-9]{2}
TIMEOUT 5

NAME print_avg_map_top
RUN bpftrace -e 'i:ms:1 { @[nsecs/100000] = avg(1) } i:ms:10 { print(@, 3, 0); exit(); } END {clear(@)}' | grep -c -v "^ *$"
EXPECT ^4$
NAME print_avg_map_args
RUN bpftrace -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 2, 10); print("END"); clear(@); exit(); }'
EXPECT Attaching 1 probe\.\.\.\n@\[c\]: 3\n@\[d\]: 4\n\nEND
TIMEOUT 1

NAME print_avg_map_div
RUN bpftrace -e 'i:ms:1 { @[nsecs/1000000] = avg(10) } i:ms:10 { print(@, 1, 10) ;exit();} END {clear(@)}'
EXPECT ^@\[[0-9]+\]: 1$
NAME print_avg_map_with_large_top
RUN bpftrace -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 10, 10); print("END"); clear(@); exit(); }'
EXPECT Attaching 1 probe\.\.\.\n@\[a\]: 1\n@\[b\]: 2\n@\[c\]: 3\n@\[d\]: 4\n\nEND
TIMEOUT 1
11 changes: 8 additions & 3 deletions tests/runtime/json-output
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ EXPECT ^{"type": "value", "data": \[1,2,"string"\]}$
TIMEOUT 1

NAME print_avg_map_args
RUN bpftrace -f json -e 'i:ms:1 { @[nsecs/100000] = avg(10) } i:ms:10 { print(@, 1, 10); exit(); } END {clear(@)}'
EXPECT ^{"type": "stats", "data": {"@": { *"[0-9]*": 1}}}$
TIMEOUT 1
RUN bpftrace -f json -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 2, 10); clear(@); exit(); }'
EXPECT {"type": "stats", "data": {"@": { *"c": 3, *"d": 4}}}
TIMEOUT 1

NAME print_avg_map_with_large_top
RUN bpftrace -f json -e 'BEGIN { @["a"] = avg(10); @["b"] = avg(20); @["c"] = avg(30); @["d"] = avg(40); print(@, 10, 10); clear(@); exit(); }'
EXPECT {"type": "stats", "data": {"@": { *"a": 1, *"b": 2, *"c": 3, *"d": 4}}}
TIMEOUT 1

0 comments on commit b536854

Please sign in to comment.