Skip to content

Commit

Permalink
Strengthen tracepoint format parsing
Browse files Browse the repository at this point in the history
There's issue in current RHEL real time kernel with tracepoint format,
which makes bcc-tools to return wrong data.

Two new 'common_' fields were added and it causes 2 issues for tracepoint
format parser.

First issue
  - is the gap between common fields and other fields, which is not
    picked up by the parser, so the resulted struct is not aligned with
    the data.

Second issue
  - is the fact that current parser covers common fields with:
      u64 __do_not_use__
    so the new common fields are not accounted for.

    This issue is solved in the following patch. I kept both
    issues and fixes separated to make the change readable.

There's a 'not described gap' in the sched_wakeup's format file and
probably in other formats as well:

Having:
  # cat /sys/kernel/debug/tracing/events/sched/sched_wakeup/format
  name: sched_wakeup
  ID: 310
  format:
          field:unsigned short common_type;       offset:0;       size:2; signed:0;
          field:unsigned char common_flags;       offset:2;       size:1; signed:0;
          field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
          field:int common_pid;   offset:4;       size:4; signed:1;
          field:unsigned char common_migrate_disable;     offset:8;       size:1; signed:0;
          field:unsigned char common_preempt_lazy_count;  offset:9;       size:1; signed:0;

          field:char comm[16];    offset:12;      size:16;        signed:1;
          field:pid_t pid;        offset:28;      size:4; signed:1;
          field:int prio; offset:32;      size:4; signed:1;
          field:int success;      offset:36;      size:4; signed:1;
          field:int target_cpu;   offset:40;      size:4; signed:1;

There's "common_preempt_lazy_count" field on offset 9 with size 1:
        common_preempt_lazy_count;  offset:9;       size:1;

and it's followed by "comm" field on offset 12:
        field:char comm[16];    offset:12;      size:16;        signed:1;

which makes 2 bytes gap in between, that might confuse some applications
like bpftrace or bcc-tools library.

The tracepoint parser makes struct out of the field descriptions,
but does not account for such gaps.

I posted patch to fix this [1] in RT kernel, but that might take a while,
and we could easily fix our tracepoint parser to workaround this issue.

Adding code to detect this gaps and add 1 byte __pad_X fields, where X is
the offset number.

[1] https://lore.kernel.org/linux-rt-users/[email protected]/
Signed-off-by: Jiri Olsa <[email protected]>
  • Loading branch information
olsajiri authored and yonghong-song committed Mar 15, 2020
1 parent b9099c5 commit b8423e6
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 13 deletions.
49 changes: 36 additions & 13 deletions src/cc/common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ enum class field_kind_t {
common,
data_loc,
regular,
invalid
invalid,
pad,
};

static inline field_kind_t _get_field_kind(std::string const& line,
std::string& field_type,
std::string& field_name) {
std::string& field_name,
int *last_offset) {
auto field_pos = line.find("field:");
if (field_pos == std::string::npos)
return field_kind_t::invalid;
Expand All @@ -89,6 +91,10 @@ static inline field_kind_t _get_field_kind(std::string const& line,
if (semi_pos == std::string::npos)
return field_kind_t::invalid;

auto offset_str = line.substr(offset_pos + 7,
semi_pos - offset_pos - 7);
int offset = std::stoi(offset_str, nullptr);

auto size_pos = line.find("size:", semi_pos);
if (size_pos == std::string::npos)
return field_kind_t::invalid;
Expand All @@ -101,6 +107,13 @@ static inline field_kind_t _get_field_kind(std::string const& line,
semi_pos - size_pos - 5);
int size = std::stoi(size_str, nullptr);

if (*last_offset < offset) {
*last_offset += 1;
return field_kind_t::pad;
}

*last_offset = offset + size;

auto field = line.substr(field_pos + 6/*"field:"*/,
field_semi_pos - field_pos - 6);
auto pos = field.find_last_of("\t ");
Expand Down Expand Up @@ -152,19 +165,29 @@ std::string parse_tracepoint(std::istream &input, std::string const& category,
std::string const& event) {
std::string tp_struct = "struct tracepoint__" + category + "__" + event + " {\n";
tp_struct += "\tu64 __do_not_use__;\n";
int last_offset = 0;
for (std::string line; getline(input, line); ) {
std::string field_type, field_name;
switch (_get_field_kind(line, field_type, field_name)) {
case field_kind_t::invalid:
case field_kind_t::common:
continue;
case field_kind_t::data_loc:
tp_struct += "\tint data_loc_" + field_name + ";\n";
break;
case field_kind_t::regular:
tp_struct += "\t" + field_type + " " + field_name + ";\n";
break;
}
field_kind_t kind;

do {
kind = _get_field_kind(line, field_type, field_name, &last_offset);

switch (kind) {
case field_kind_t::invalid:
case field_kind_t::common:
continue;
case field_kind_t::data_loc:
tp_struct += "\tint data_loc_" + field_name + ";\n";
break;
case field_kind_t::regular:
tp_struct += "\t" + field_type + " " + field_name + ";\n";
break;
case field_kind_t::pad:
tp_struct += "\tchar __pad_" + std::to_string(last_offset - 1) + ";\n";
break;
}
} while (kind == field_kind_t::pad);
}

tp_struct += "};\n";
Expand Down
8 changes: 8 additions & 0 deletions tests/cc/test_parse_tracepoint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ TEST_CASE("test tracepoint parser", "[TracepointParser]") {
"struct tracepoint__syscalls__sys_enter_read {\n"
"\tu64 __do_not_use__;\n"
"\tint __syscall_nr;\n"
"\tchar __pad_12;\n"
"\tchar __pad_13;\n"
"\tchar __pad_14;\n"
"\tchar __pad_15;\n"
"\tu64 fd;\n"
"\tchar * buf;\n"
"\tsize_t count;\n"
Expand Down Expand Up @@ -57,6 +61,10 @@ TEST_CASE("test tracepoint parser", "[TracepointParser]") {
"\tint sig;\n"
"\tint errno;\n"
"\tint code;\n"
"\tchar __pad_20;\n"
"\tchar __pad_21;\n"
"\tchar __pad_22;\n"
"\tchar __pad_23;\n"
"\tunsigned long sa_handler;\n"
"\tunsigned long sa_flags;\n"
"};\n";
Expand Down

0 comments on commit b8423e6

Please sign in to comment.