[PATCH v7 13/13] user_events: Use __get_rel_str for relative string fields

From: Beau Belgrave
Date: Thu Dec 09 2021 - 17:33:08 EST


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 56eb58ddb4cf..3779fa2ca14a 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, &reg);
+
+ 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