Switch between __get_str and __get_rel_str within the print_fmt of user_events. Add unit test to ensure print_fmt is correct on known types. Signed-off-by: Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> --- kernel/trace/trace_events_user.c | 24 ++- .../selftests/user_events/ftrace_test.c | 166 ++++++++++++++++++ 2 files changed, 182 insertions(+), 8 deletions(-) diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c index b487d02acedf..2b47e7b11c3d 100644 --- a/kernel/trace/trace_events_user.c +++ b/kernel/trace/trace_events_user.c @@ -257,7 +257,7 @@ static int user_event_add_field(struct user_event *user, const char *type, goto add_field; add_validator: - if (strstr(type, "char[") != 0) + if (strstr(type, "char") != 0) validator_flags |= VALIDATOR_ENSURE_NULL; validator = kmalloc(sizeof(*validator), GFP_KERNEL); @@ -456,14 +456,21 @@ static const char *user_field_format(const char *type) return "%llu"; } -static bool user_field_is_dyn_string(const char *type) +static bool user_field_is_dyn_string(const char *type, const char **str_func) { - if (str_has_prefix(type, "__data_loc ") || - str_has_prefix(type, "__rel_loc ")) - if (strstr(type, "char[") != 0) - return true; + if (str_has_prefix(type, "__data_loc ")) { + *str_func = "__get_str"; + goto check; + } + + if (str_has_prefix(type, "__rel_loc ")) { + *str_func = "__get_rel_str"; + goto check; + } return false; +check: + return strstr(type, "char") != 0; } #define LEN_OR_ZERO (len ? len - pos : 0) @@ -472,6 +479,7 @@ static int user_event_set_print_fmt(struct user_event *user, char *buf, int len) struct ftrace_event_field *field, *next; struct list_head *head = &user->fields; int pos = 0, depth = 0; + const char *str_func; pos += snprintf(buf + pos, LEN_OR_ZERO, "\""); @@ -488,9 +496,9 @@ static int user_event_set_print_fmt(struct user_event *user, char *buf, int len) pos += snprintf(buf + pos, LEN_OR_ZERO, "\""); list_for_each_entry_safe_reverse(field, next, head, link) { - if (user_field_is_dyn_string(field->type)) + if (user_field_is_dyn_string(field->type, &str_func)) pos += snprintf(buf + pos, LEN_OR_ZERO, - ", __get_str(%s)", field->name); + ", %s(%s)", str_func, field->name); else pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s", field->name); diff --git a/tools/testing/selftests/user_events/ftrace_test.c b/tools/testing/selftests/user_events/ftrace_test.c index 16aff1fb295a..b2e5c0765a68 100644 --- a/tools/testing/selftests/user_events/ftrace_test.c +++ b/tools/testing/selftests/user_events/ftrace_test.c @@ -20,6 +20,7 @@ const char *data_file = "/sys/kernel/debug/tracing/user_events_data"; const char *status_file = "/sys/kernel/debug/tracing/user_events_status"; const char *enable_file = "/sys/kernel/debug/tracing/events/user_events/__test_event/enable"; const char *trace_file = "/sys/kernel/debug/tracing/trace"; +const char *fmt_file = "/sys/kernel/debug/tracing/events/user_events/__test_event/format"; static int trace_bytes(void) { @@ -47,6 +48,61 @@ static int trace_bytes(void) return bytes; } +static int get_print_fmt(char *buffer, int len) +{ + FILE *fp = fopen(fmt_file, "r"); + int c, index = 0, last = 0; + + if (!fp) + return -1; + + /* Read until empty line (Skip Common) */ + while (true) { + c = getc(fp); + + if (c == EOF) + break; + + if (last == '\n' && c == '\n') + break; + + last = c; + } + + last = 0; + + /* Read until empty line (Skip Properties) */ + while (true) { + c = getc(fp); + + if (c == EOF) + break; + + if (last == '\n' && c == '\n') + break; + + last = c; + } + + /* Read in print_fmt: */ + while (len > 1) { + c = getc(fp); + + if (c == EOF || c == '\n') + break; + + buffer[index++] = c; + + len--; + } + + buffer[index] = 0; + + fclose(fp); + + return 0; +} + static int clear(void) { int fd = open(data_file, O_RDWR); @@ -63,6 +119,44 @@ static int clear(void) return 0; } +static int check_print_fmt(const char *event, const char *expected) +{ + struct user_reg reg = {0}; + char print_fmt[256]; + int ret; + int fd; + + /* Ensure cleared */ + ret = clear(); + + if (ret != 0) + return ret; + + fd = open(data_file, O_RDWR); + + if (fd == -1) + return fd; + + reg.size = sizeof(reg); + reg.name_args = (__u64)event; + + /* Register should work */ + ret = ioctl(fd, DIAG_IOCSREG, ®); + + close(fd); + + if (ret != 0) + return ret; + + /* Ensure correct print_fmt */ + ret = get_print_fmt(print_fmt, sizeof(print_fmt)); + + if (ret != 0) + return ret; + + return strcmp(print_fmt, expected); +} + FIXTURE(user) { int status_fd; int data_fd; @@ -282,6 +376,78 @@ TEST_F(user, write_validator) { ASSERT_EQ(EFAULT, errno); } +TEST_F(user, print_fmt) { + int ret; + + ret = check_print_fmt("__test_event __rel_loc char[] data", + "print fmt: \"data=%s\", __get_rel_str(data)"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event __data_loc char[] data", + "print fmt: \"data=%s\", __get_str(data)"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event s64 data", + "print fmt: \"data=%lld\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event u64 data", + "print fmt: \"data=%llu\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event s32 data", + "print fmt: \"data=%d\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event u32 data", + "print fmt: \"data=%u\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event int data", + "print fmt: \"data=%d\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event unsigned int data", + "print fmt: \"data=%u\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event s16 data", + "print fmt: \"data=%d\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event u16 data", + "print fmt: \"data=%u\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event short data", + "print fmt: \"data=%d\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event unsigned short data", + "print fmt: \"data=%u\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event s8 data", + "print fmt: \"data=%d\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event u8 data", + "print fmt: \"data=%u\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event char data", + "print fmt: \"data=%d\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event unsigned char data", + "print fmt: \"data=%u\", REC->data"); + ASSERT_EQ(0, ret); + + ret = check_print_fmt("__test_event char[4] data", + "print fmt: \"data=%s\", REC->data"); + ASSERT_EQ(0, ret); +} + int main(int argc, char **argv) { return test_harness_run(argc, argv); -- 2.17.1