diff mbox series

[v6,13/13] user_events: Use __get_rel_str for relative string fields

Message ID 20211201182515.2446-14-beaub@linux.microsoft.com (mailing list archive)
State Superseded
Headers show
Series user_events: Enable user processes to create and write to trace events | expand

Commit Message

Beau Belgrave Dec. 1, 2021, 6:25 p.m. UTC 
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@linux.microsoft.com>
---
 kernel/trace/trace_events_user.c              |  24 ++-
 .../selftests/user_events/ftrace_test.c       | 166 ++++++++++++++++++
 2 files changed, 182 insertions(+), 8 deletions(-)
diff mbox series

Patch

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, &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);