perf probe: Fix error message if get_real_path() failed

Perf probe -L shows incorrect error message (Dwarf error) if it fails to find source file. This can confuse users.

# ./perf probe -s /nowhere -L vfs_read
Debuginfo analysis failed. (-2)
  Error: Failed to show lines. (-2)

With this patch, it shows correct message.

# ./perf probe -s /nowhere -L vfs_read
Failed to find source file. (-2)
  Error: Failed to show lines. (-2)

Signed-off-by Masami Hiramatsu <[email protected]>

Cc Chase Douglas <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Ingo Molnar <[email protected]>

---

 tools/perf/util/probe-event.c  |   59 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-finder.c |   61 +++-------------------------------------
 2 files changed, 64 insertions(+), 56 deletions(-)

tools/perf/util/probe-event.c +59 -0

index 09cf546..8d08e75 100644
@@ -195,6 +195,55 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
 	return ntevs;
 }
 
+/*
+ * Find a src file from a DWARF tag path. Prepend optional source path prefix
+ * and chop off leading directories that do not exist. Result is passed back as
+ * a newly allocated path on success.
+ * Return 0 if file was found and readable, -errno otherwise.
+ */
+static int get_real_path(const char *raw_path, char **new_path)
+{
+	if (!symbol_conf.source_prefix) {
+		if (access(raw_path, R_OK) == 0) {
+			*new_path = strdup(raw_path);
+			return 0;
+		} else
+			return -errno;
+	}
+
+	*new_path = malloc((strlen(symbol_conf.source_prefix) +
+			    strlen(raw_path) + 2));
+	if (!*new_path)
+		return -ENOMEM;
+
+	for (;;) {
+		sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
+			raw_path);
+
+		if (access(*new_path, R_OK) == 0)
+			return 0;
+
+		switch (errno) {
+		case ENAMETOOLONG:
+		case ENOENT:
+		case EROFS:
+		case EFAULT:
+			raw_path = strchr(++raw_path, '/');
+			if (!raw_path) {
+				free(*new_path);
+				*new_path = NULL;
+				return -ENOENT;
+			}
+			continue;
+
+		default:
+			free(*new_path);
+			*new_path = NULL;
+			return -errno;
+		}
+	}
+}
+
 #define LINEBUF_SIZE 256
 #define NR_ADDITIONAL_LINES 2
 

@@ -244,6 +293,7 @@ int show_line_range(struct line_range *lr)
 	struct line_node *ln;
 	FILE *fp;
 	int fd, ret;
+	char *tmp;
 
 	/* Search a line range */
 	ret = init_vmlinux();

@@ -266,6 +316,15 @@ int show_line_range(struct line_range *lr)
 		return ret;
 	}
 
+	/* Convert source file path */
+	tmp = lr->path;
+	ret = get_real_path(tmp, &lr->path);
+	free(tmp);	/* Free old path */
+	if (ret < 0) {
+		pr_warning("Failed to find source file. (%d)\n", ret);
+		return ret;
+	}
+
 	setup_pager();
 
 	if (lr->function)

tools/perf/util/probe-finder.c +4 -55

index 3e64e1f..a934a36 100644
@@ -58,55 +58,6 @@ static int strtailcmp(const char *s1, const char *s2)
 	return 0;
 }
 
-/*
- * Find a src file from a DWARF tag path. Prepend optional source path prefix
- * and chop off leading directories that do not exist. Result is passed back as
- * a newly allocated path on success.
- * Return 0 if file was found and readable, -errno otherwise.
- */
-static int get_real_path(const char *raw_path, char **new_path)
-{
-	if (!symbol_conf.source_prefix) {
-		if (access(raw_path, R_OK) == 0) {
-			*new_path = strdup(raw_path);
-			return 0;
-		} else
-			return -errno;
-	}
-
-	*new_path = malloc((strlen(symbol_conf.source_prefix) +
-			    strlen(raw_path) + 2));
-	if (!*new_path)
-		return -ENOMEM;
-
-	for (;;) {
-		sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
-			raw_path);
-
-		if (access(*new_path, R_OK) == 0)
-			return 0;
-
-		switch (errno) {
-		case ENAMETOOLONG:
-		case ENOENT:
-		case EROFS:
-		case EFAULT:
-			raw_path = strchr(++raw_path, '/');
-			if (!raw_path) {
-				free(*new_path);
-				*new_path = NULL;
-				return -ENOENT;
-			}
-			continue;
-
-		default:
-			free(*new_path);
-			*new_path = NULL;
-			return -errno;
-		}
-	}
-}
-
 /* Line number list operations */
 
 /* Add a line to line number list */

@@ -1256,13 +1207,11 @@ end:
 static int line_range_add_line(const char *src, unsigned int lineno,
 			       struct line_range *lr)
 {
-	int ret;
-
-	/* Copy real path */
+	/* Copy source path */
 	if (!lr->path) {
-		ret = get_real_path(src, &lr->path);
-		if (ret != 0)
-			return ret;
+		lr->path = strdup(src);
+		if (lr->path == NULL)
+			return -ENOMEM;
 	}
 	return line_list__add_line(&lr->line_list, lineno);
 }

Quoted 186 lines · Masami Hiramatsu

Perf probe -L shows incorrect error message (Dwarf error) if it fails to find source file. This can confuse users.

# ./perf probe -s /nowhere -L vfs_read
Debuginfo analysis failed. (-2)
  Error: Failed to show lines. (-2)

With this patch, it shows correct message.

# ./perf probe -s /nowhere -L vfs_read
Failed to find source file. (-2)
  Error: Failed to show lines. (-2)

Signed-off-by Masami Hiramatsu <[email protected]>

Cc Chase Douglas <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Ingo Molnar <[email protected]>

---

 tools/perf/util/probe-event.c  |   59 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-finder.c |   61 +++-------------------------------------
 2 files changed, 64 insertions(+), 56 deletions(-)

tools/perf/util/probe-event.c +59 -0

index 09cf546..8d08e75 100644
@@ -195,6 +195,55 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
 	return ntevs;
 }
 
+/*
+ * Find a src file from a DWARF tag path. Prepend optional source path prefix
+ * and chop off leading directories that do not exist. Result is passed back as
+ * a newly allocated path on success.
+ * Return 0 if file was found and readable, -errno otherwise.
+ */
+static int get_real_path(const char *raw_path, char **new_path)
+{
+	if (!symbol_conf.source_prefix) {
+		if (access(raw_path, R_OK) == 0) {
+			*new_path = strdup(raw_path);
+			return 0;
+		} else
+			return -errno;
+	}
+
+	*new_path = malloc((strlen(symbol_conf.source_prefix) +
+			    strlen(raw_path) + 2));
+	if (!*new_path)
+		return -ENOMEM;
+
+	for (;;) {
+		sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
+			raw_path);
+
+		if (access(*new_path, R_OK) == 0)
+			return 0;
+
+		switch (errno) {
+		case ENAMETOOLONG:
+		case ENOENT:
+		case EROFS:
+		case EFAULT:
+			raw_path = strchr(++raw_path, '/');
+			if (!raw_path) {
+				free(*new_path);
+				*new_path = NULL;
+				return -ENOENT;
+			}
+			continue;
+
+		default:
+			free(*new_path);
+			*new_path = NULL;
+			return -errno;
+		}
+	}
+}
+
 #define LINEBUF_SIZE 256
 #define NR_ADDITIONAL_LINES 2
 

@@ -244,6 +293,7 @@ int show_line_range(struct line_range *lr)
 	struct line_node *ln;
 	FILE *fp;
 	int fd, ret;
+	char *tmp;
 
 	/* Search a line range */
 	ret = init_vmlinux();

@@ -266,6 +316,15 @@ int show_line_range(struct line_range *lr)
 		return ret;
 	}
 
+	/* Convert source file path */
+	tmp = lr->path;
+	ret = get_real_path(tmp, &lr->path);
+	free(tmp);	/* Free old path */
+	if (ret < 0) {
+		pr_warning("Failed to find source file. (%d)\n", ret);
+		return ret;
+	}
+
 	setup_pager();
 
 	if (lr->function)

tools/perf/util/probe-finder.c +4 -55

index 3e64e1f..a934a36 100644
@@ -58,55 +58,6 @@ static int strtailcmp(const char *s1, const char *s2)
 	return 0;
 }
 
-/*
- * Find a src file from a DWARF tag path. Prepend optional source path prefix
- * and chop off leading directories that do not exist. Result is passed back as
- * a newly allocated path on success.
- * Return 0 if file was found and readable, -errno otherwise.
- */
-static int get_real_path(const char *raw_path, char **new_path)
-{
-	if (!symbol_conf.source_prefix) {
-		if (access(raw_path, R_OK) == 0) {
-			*new_path = strdup(raw_path);
-			return 0;
-		} else
-			return -errno;
-	}
-
-	*new_path = malloc((strlen(symbol_conf.source_prefix) +
-			    strlen(raw_path) + 2));
-	if (!*new_path)
-		return -ENOMEM;
-
-	for (;;) {
-		sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
-			raw_path);
-
-		if (access(*new_path, R_OK) == 0)
-			return 0;
-
-		switch (errno) {
-		case ENAMETOOLONG:
-		case ENOENT:
-		case EROFS:
-		case EFAULT:
-			raw_path = strchr(++raw_path, '/');
-			if (!raw_path) {
-				free(*new_path);
-				*new_path = NULL;
-				return -ENOENT;
-			}
-			continue;
-
-		default:
-			free(*new_path);
-			*new_path = NULL;
-			return -errno;
-		}
-	}
-}
-
 /* Line number list operations */
 
 /* Add a line to line number list */

@@ -1256,13 +1207,11 @@ end:
 static int line_range_add_line(const char *src, unsigned int lineno,
 			       struct line_range *lr)
 {
-	int ret;
-
-	/* Copy real path */
+	/* Copy source path */
 	if (!lr->path) {
-		ret = get_real_path(src, &lr->path);
-		if (ret != 0)
-			return ret;
+		lr->path = strdup(src);
+		if (lr->path == NULL)
+			return -ENOMEM;
 	}
 	return line_list__add_line(&lr->line_list, lineno);
 }

tools/perf/util/probe-finder.c int find_line_range(int fd, struct line_range *lr) · line 1415

Quoted 8 lines

tools/perf/util/probe-finder.c +1 -1

@@ -1460,7 +1409,7 @@ int find_line_range(int fd, struct line_range *lr)
 		}
 		off = noff;
 	}
-	pr_debug("path: %lx\n", (unsigned long)lr->path);
+	pr_debug("path: %s\n", lr->path);
 	dwarf_end(dbg);
 
 	return (ret < 0) ? ret : lf.found;

This seems fine to me. I don't think any functionality has really changed, just moving the function into probe-event.c and printing out an accurate error message.

Acked-by Chase Douglas <[email protected]>

Thanks!

Hi Arnaldo,

Could you please merge this series, because this series fixes some bugs which people easily hit ?

Thank you,

Quoted 204 lines · Chase Douglas

Quoted 186 lines · Masami Hiramatsu

Perf probe -L shows incorrect error message (Dwarf error) if it fails to find source file. This can confuse users.

# ./perf probe -s /nowhere -L vfs_read
Debuginfo analysis failed. (-2)
  Error: Failed to show lines. (-2)

With this patch, it shows correct message.

# ./perf probe -s /nowhere -L vfs_read
Failed to find source file. (-2)
  Error: Failed to show lines. (-2)

Signed-off-by Masami Hiramatsu <[email protected]>

Cc Chase Douglas <[email protected]>

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Ingo Molnar <[email protected]>

---

 tools/perf/util/probe-event.c  |   59 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-finder.c |   61 +++-------------------------------------
 2 files changed, 64 insertions(+), 56 deletions(-)

tools/perf/util/probe-event.c +59 -0

index 09cf546..8d08e75 100644
@@ -195,6 +195,55 @@ static int try_to_find_kprobe_trace_events(struct perf_probe_event *pev,
 	return ntevs;
 }
 
+/*
+ * Find a src file from a DWARF tag path. Prepend optional source path prefix
+ * and chop off leading directories that do not exist. Result is passed back as
+ * a newly allocated path on success.
+ * Return 0 if file was found and readable, -errno otherwise.
+ */
+static int get_real_path(const char *raw_path, char **new_path)
+{
+	if (!symbol_conf.source_prefix) {
+		if (access(raw_path, R_OK) == 0) {
+			*new_path = strdup(raw_path);
+			return 0;
+		} else
+			return -errno;
+	}
+
+	*new_path = malloc((strlen(symbol_conf.source_prefix) +
+			    strlen(raw_path) + 2));
+	if (!*new_path)
+		return -ENOMEM;
+
+	for (;;) {
+		sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
+			raw_path);
+
+		if (access(*new_path, R_OK) == 0)
+			return 0;
+
+		switch (errno) {
+		case ENAMETOOLONG:
+		case ENOENT:
+		case EROFS:
+		case EFAULT:
+			raw_path = strchr(++raw_path, '/');
+			if (!raw_path) {
+				free(*new_path);
+				*new_path = NULL;
+				return -ENOENT;
+			}
+			continue;
+
+		default:
+			free(*new_path);
+			*new_path = NULL;
+			return -errno;
+		}
+	}
+}
+
 #define LINEBUF_SIZE 256
 #define NR_ADDITIONAL_LINES 2
 

@@ -244,6 +293,7 @@ int show_line_range(struct line_range *lr)
 	struct line_node *ln;
 	FILE *fp;
 	int fd, ret;
+	char *tmp;
 
 	/* Search a line range */
 	ret = init_vmlinux();

@@ -266,6 +316,15 @@ int show_line_range(struct line_range *lr)
 		return ret;
 	}
 
+	/* Convert source file path */
+	tmp = lr->path;
+	ret = get_real_path(tmp, &lr->path);
+	free(tmp);	/* Free old path */
+	if (ret < 0) {
+		pr_warning("Failed to find source file. (%d)\n", ret);
+		return ret;
+	}
+
 	setup_pager();
 
 	if (lr->function)

tools/perf/util/probe-finder.c +4 -55

index 3e64e1f..a934a36 100644
@@ -58,55 +58,6 @@ static int strtailcmp(const char *s1, const char *s2)
 	return 0;
 }
 
-/*
- * Find a src file from a DWARF tag path. Prepend optional source path prefix
- * and chop off leading directories that do not exist. Result is passed back as
- * a newly allocated path on success.
- * Return 0 if file was found and readable, -errno otherwise.
- */
-static int get_real_path(const char *raw_path, char **new_path)
-{
-	if (!symbol_conf.source_prefix) {
-		if (access(raw_path, R_OK) == 0) {
-			*new_path = strdup(raw_path);
-			return 0;
-		} else
-			return -errno;
-	}
-
-	*new_path = malloc((strlen(symbol_conf.source_prefix) +
-			    strlen(raw_path) + 2));
-	if (!*new_path)
-		return -ENOMEM;
-
-	for (;;) {
-		sprintf(*new_path, "%s/%s", symbol_conf.source_prefix,
-			raw_path);
-
-		if (access(*new_path, R_OK) == 0)
-			return 0;
-
-		switch (errno) {
-		case ENAMETOOLONG:
-		case ENOENT:
-		case EROFS:
-		case EFAULT:
-			raw_path = strchr(++raw_path, '/');
-			if (!raw_path) {
-				free(*new_path);
-				*new_path = NULL;
-				return -ENOENT;
-			}
-			continue;
-
-		default:
-			free(*new_path);
-			*new_path = NULL;
-			return -errno;
-		}
-	}
-}
-
 /* Line number list operations */
 
 /* Add a line to line number list */

@@ -1256,13 +1207,11 @@ end:
 static int line_range_add_line(const char *src, unsigned int lineno,
 			       struct line_range *lr)
 {
-	int ret;
-
-	/* Copy real path */
+	/* Copy source path */
 	if (!lr->path) {
-		ret = get_real_path(src, &lr->path);
-		if (ret != 0)
-			return ret;
+		lr->path = strdup(src);
+		if (lr->path == NULL)
+			return -ENOMEM;
 	}
 	return line_list__add_line(&lr->line_list, lineno);
 }

tools/perf/util/probe-finder.c int find_line_range(int fd, struct line_range *lr) · line 1415

Quoted 8 lines

tools/perf/util/probe-finder.c +1 -1

@@ -1460,7 +1409,7 @@ int find_line_range(int fd, struct line_range *lr)
 		}
 		off = noff;
 	}
-	pr_debug("path: %lx\n", (unsigned long)lr->path);
+	pr_debug("path: %s\n", lr->path);
 	dwarf_end(dbg);
 
 	return (ret < 0) ? ret : lf.found;

This seems fine to me. I don't think any functionality has really changed, just moving the function into probe-event.c and printing out an accurate error message.

Acked-by Chase Douglas <[email protected]>

Thanks!