[PATCH -tip 0/4][BUGFIX] Perf probe bugfixes

[PATCH -tip 0/4][BUGFIX] Perf probe bugfixes

[Masami Hiramatsu]

Hi,

This series fixes several bugs and cleanup messages
(reported by Arnaldo) on perf-tools.

Thank you,

---

Masami Hiramatsu (4):
      [BUGFIX]perf probe: Fix to support libdwfl older than 0.148
      [CLEANUP]perf probe: cleanup messages
      [BUGFIX]perf: Fix strlist__parse_list to handle const string
      [BUGFIX]perf: Fix lazy wildcard matching


 tools/perf/util/probe-event.c  |   30 +++++----
 tools/perf/util/probe-finder.c |  127 ++++++++++++++++++++++++----------------
 tools/perf/util/string.c       |    2 -
 tools/perf/util/strlist.c      |   14 +++-
 4 files changed, 104 insertions(+), 69 deletions(-)

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH -tip 1/4] [BUGFIX]perf: Fix lazy wildcard matching

[Masami Hiramatsu]

Fix lazy wildcard matching to ignore space after
wild card.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: linux-kernel@vger.kernel.org
---

 tools/perf/util/string.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 0409fc7..8fc0bd3 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -259,7 +259,7 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
 		if (!*pat)	/* Tail wild card matches all */
 			return true;
 		while (*str)
-			if (strglobmatch(str++, pat))
+			if (__match_glob(str++, pat, ignore_space))
 				return true;
 	}
 	return !*str && !*pat;

[PATCH -tip 2/4] [BUGFIX]perf: Fix strlist__parse_list to handle const string

[Masami Hiramatsu]

Fix strlist__parse_list to handle const string. Without
this patch, strlist__parse_list() causes SEGV when
caller passes a constant string.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: linux-kernel@vger.kernel.org
---

 tools/perf/util/strlist.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/strlist.c b/tools/perf/util/strlist.c
index 6783a20..caa7884 100644
--- a/tools/perf/util/strlist.c
+++ b/tools/perf/util/strlist.c
@@ -136,13 +136,19 @@ static int strlist__parse_list_entry(struct strlist *self, const char *s)
 
 int strlist__parse_list(struct strlist *self, const char *s)
 {
-	char *sep;
+	char *sep, *tmp;
 	int err;
 
+	/* This method requires strdup, because this changes given string */
+	if (!self->dupstr)
+		return -EINVAL;
+
 	while ((sep = strchr(s, ',')) != NULL) {
-		*sep = '\0';
-		err = strlist__parse_list_entry(self, s);
-		*sep = ',';
+		tmp = strndup(s, sep - s);
+		if (tmp == NULL)
+			return -ENOMEM;
+		err = strlist__parse_list_entry(self, tmp);
+		free(tmp);
 		if (err != 0)
 			return err;
 		s = sep + 1;

[PATCH -tip 3/4] [CLEANUP]perf probe: cleanup messages

[Masami Hiramatsu]

Add new lines for error or debug messages, change dwarf related
words to more generic words (or just removed)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: linux-kernel@vger.kernel.org
---

 tools/perf/util/probe-event.c  |   30 +++++++++++++++--------------
 tools/perf/util/probe-finder.c |   42 +++++++++++++++++++++-------------------
 2 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3b6a529..089d78e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -95,7 +95,7 @@ static int init_vmlinux(void)
 		goto out;
 
 	if (machine__create_kernel_maps(&machine) < 0) {
-		pr_debug("machine__create_kernel_maps ");
+		pr_debug("machine__create_kernel_maps() failed.\n");
 		goto out;
 	}
 out:
@@ -140,7 +140,8 @@ static int open_vmlinux(const char *module)
 {
 	const char *path = kernel_get_module_path(module);
 	if (!path) {
-		pr_err("Failed to find path of %s module", module ?: "kernel");
+		pr_err("Failed to find path of %s module.\n",
+		       module ?: "kernel");
 		return -ENOENT;
 	}
 	pr_debug("Try to open %s\n", path);
@@ -217,7 +218,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 		pr_warning("Warning: No dwarf info found in the vmlinux - "
 			"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
 		if (!need_dwarf) {
-			pr_debug("Trying to use symbols.\nn");
+			pr_debug("Trying to use symbols.\n");
 			return 0;
 		}
 	}
@@ -457,7 +458,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 
 	fd = open_vmlinux(module);
 	if (fd < 0) {
-		pr_warning("Failed to open debuginfo file.\n");
+		pr_warning("Failed to open debug information file.\n");
 		return fd;
 	}
 
@@ -690,39 +691,40 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 
 	/* Exclusion check */
 	if (pp->lazy_line && pp->line) {
-		semantic_error("Lazy pattern can't be used with line number.");
+		semantic_error("Lazy pattern can't be used with"
+			       " line number.\n");
 		return -EINVAL;
 	}
 
 	if (pp->lazy_line && pp->offset) {
-		semantic_error("Lazy pattern can't be used with offset.");
+		semantic_error("Lazy pattern can't be used with offset.\n");
 		return -EINVAL;
 	}
 
 	if (pp->line && pp->offset) {
-		semantic_error("Offset can't be used with line number.");
+		semantic_error("Offset can't be used with line number.\n");
 		return -EINVAL;
 	}
 
 	if (!pp->line && !pp->lazy_line && pp->file && !pp->function) {
 		semantic_error("File always requires line number or "
-			       "lazy pattern.");
+			       "lazy pattern.\n");
 		return -EINVAL;
 	}
 
 	if (pp->offset && !pp->function) {
-		semantic_error("Offset requires an entry function.");
+		semantic_error("Offset requires an entry function.\n");
 		return -EINVAL;
 	}
 
 	if (pp->retprobe && !pp->function) {
-		semantic_error("Return probe requires an entry function.");
+		semantic_error("Return probe requires an entry function.\n");
 		return -EINVAL;
 	}
 
 	if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
 		semantic_error("Offset/Line/Lazy pattern can't be used with "
-			       "return probe.");
+			       "return probe.\n");
 		return -EINVAL;
 	}
 
@@ -996,7 +998,7 @@ int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
 
 	return tmp - buf;
 error:
-	pr_debug("Failed to synthesize perf probe argument: %s",
+	pr_debug("Failed to synthesize perf probe argument: %s\n",
 		 strerror(-ret));
 	return ret;
 }
@@ -1046,7 +1048,7 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
 
 	return buf;
 error:
-	pr_debug("Failed to synthesize perf probe point: %s",
+	pr_debug("Failed to synthesize perf probe point: %s\n",
 		 strerror(-ret));
 	if (buf)
 		free(buf);
@@ -1787,7 +1789,7 @@ static int del_trace_probe_event(int fd, const char *group,
 
 	ret = e_snprintf(buf, 128, "%s:%s", group, event);
 	if (ret < 0) {
-		pr_err("Failed to copy event.");
+		pr_err("Failed to copy event.\n");
 		return ret;
 	}
 
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 3991d73..90b6292 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -627,8 +627,8 @@ static_var:
 	regs = get_arch_regstr(regn);
 	if (!regs) {
 		/* This should be a bug in DWARF or this tool */
-		pr_warning("Mapping for DWARF register number %u "
-			   "missing on this architecture.", regn);
+		pr_warning("Mapping for the register number %u "
+			   "missing on this architecture.\n", regn);
 		return -ERANGE;
 	}
 
@@ -674,13 +674,14 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		if (ret != DW_TAG_pointer_type &&
 		    ret != DW_TAG_array_type) {
 			pr_warning("Failed to cast into string: "
-				   "%s(%s) is not a pointer nor array.",
+				   "%s(%s) is not a pointer nor array.\n",
 				   dwarf_diename(vr_die), dwarf_diename(&type));
 			return -EINVAL;
 		}
 		if (ret == DW_TAG_pointer_type) {
 			if (die_get_real_type(&type, &type) == NULL) {
-				pr_warning("Failed to get a type information.");
+				pr_warning("Failed to get a type"
+					   " information.\n");
 				return -ENOENT;
 			}
 			while (*ref_ptr)
@@ -695,7 +696,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		if (!die_compare_name(&type, "char") &&
 		    !die_compare_name(&type, "unsigned char")) {
 			pr_warning("Failed to cast into string: "
-				   "%s is not (unsigned) char *.",
+				   "%s is not (unsigned) char *.\n",
 				   dwarf_diename(vr_die));
 			return -EINVAL;
 		}
@@ -805,8 +806,8 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 			return -EINVAL;
 		}
 		if (field->name[0] == '[') {
-			pr_err("Semantic error: %s is not a pointor nor array.",
-			       varname);
+			pr_err("Semantic error: %s is not a pointor"
+			       " nor array.\n", varname);
 			return -EINVAL;
 		}
 		if (field->ref) {
@@ -953,7 +954,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwarf_Addr paddr,
 	name = dwarf_diename(sp_die);
 	if (name) {
 		if (dwarf_entrypc(sp_die, &eaddr) != 0) {
-			pr_warning("Failed to get entry pc of %s\n",
+			pr_warning("Failed to get entry address of %s\n",
 				   dwarf_diename(sp_die));
 			return -ENOENT;
 		}
@@ -969,7 +970,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwarf_Addr paddr,
 	if (retprobe) {
 		if (eaddr != paddr) {
 			pr_warning("Return probe must be on the head of"
-				   " a real function\n");
+				   " a real function.\n");
 			return -EINVAL;
 		}
 		tp->retprobe = true;
@@ -1008,7 +1009,7 @@ static int call_probe_finder(Dwarf_Die *sp_die, struct probe_finder *pf)
 		Dwarf_Frame *frame;
 		if (dwarf_cfi_addrframe(pf->cfi, pf->addr, &frame) != 0 ||
 		    dwarf_frame_cfa(frame, &pf->fb_ops, &nops) != 0) {
-			pr_warning("Failed to get CFA on 0x%jx\n",
+			pr_warning("Failed to get call frame on 0x%jx\n",
 				   (uintmax_t)pf->addr);
 			return -ENOENT;
 		}
@@ -1035,7 +1036,7 @@ static int find_probe_point_by_line(struct probe_finder *pf)
 	int ret = 0;
 
 	if (dwarf_getsrclines(&pf->cu_die, &lines, &nlines) != 0) {
-		pr_warning("No source lines found in this CU.\n");
+		pr_warning("No source lines found.\n");
 		return -ENOENT;
 	}
 
@@ -1137,7 +1138,7 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	}
 
 	if (dwarf_getsrclines(&pf->cu_die, &lines, &nlines) != 0) {
-		pr_warning("No source lines found in this CU.\n");
+		pr_warning("No source lines found.\n");
 		return -ENOENT;
 	}
 
@@ -1195,7 +1196,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
 	else {
 		/* Get probe address */
 		if (dwarf_entrypc(in_die, &addr) != 0) {
-			pr_warning("Failed to get entry pc of %s.\n",
+			pr_warning("Failed to get entry address of %s.\n",
 				   dwarf_diename(in_die));
 			param->retval = -ENOENT;
 			return DWARF_CB_ABORT;
@@ -1236,8 +1237,8 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
 			param->retval = find_probe_point_lazy(sp_die, pf);
 		else {
 			if (dwarf_entrypc(sp_die, &pf->addr) != 0) {
-				pr_warning("Failed to get entry pc of %s.\n",
-					   dwarf_diename(sp_die));
+				pr_warning("Failed to get entry address of "
+					   "%s.\n", dwarf_diename(sp_die));
 				param->retval = -ENOENT;
 				return DWARF_CB_ABORT;
 			}
@@ -1279,7 +1280,7 @@ static int find_probes(int fd, struct probe_finder *pf)
 
 	dbg = dwfl_init_offline_dwarf(fd, &dwfl, &bias);
 	if (!dbg) {
-		pr_warning("No dwarf info found in the vmlinux - "
+		pr_warning("No debug information found in the vmlinux - "
 			"please rebuild with CONFIG_DEBUG_INFO=y.\n");
 		return -EBADF;
 	}
@@ -1524,7 +1525,7 @@ int find_perf_probe_point(unsigned long addr, struct perf_probe_point *ppt)
 	/* Open the live linux kernel */
 	dbg = dwfl_init_live_kernel_dwarf(addr, &dwfl, &bias);
 	if (!dbg) {
-		pr_warning("No dwarf info found in the vmlinux - "
+		pr_warning("No debug information found in the vmlinux - "
 			"please rebuild with CONFIG_DEBUG_INFO=y.\n");
 		ret = -EINVAL;
 		goto end;
@@ -1534,7 +1535,8 @@ int find_perf_probe_point(unsigned long addr, struct perf_probe_point *ppt)
 	addr += bias;
 	/* Find cu die */
 	if (!dwarf_addrdie(dbg, (Dwarf_Addr)addr - bias, &cudie)) {
-		pr_warning("No CU DIE is found at %lx\n", addr);
+		pr_warning("Failed to find debug information for address %lx\n",
+			   addr);
 		ret = -EINVAL;
 		goto end;
 	}
@@ -1659,7 +1661,7 @@ static int find_line_range_by_line(Dwarf_Die *sp_die, struct line_finder *lf)
 
 	line_list__init(&lf->lr->line_list);
 	if (dwarf_getsrclines(&lf->cu_die, &lines, &nlines) != 0) {
-		pr_warning("No source lines found in this CU.\n");
+		pr_warning("No source lines found.\n");
 		return -ENOENT;
 	}
 
@@ -1784,7 +1786,7 @@ int find_line_range(int fd, struct line_range *lr)
 
 	dbg = dwfl_init_offline_dwarf(fd, &dwfl, &bias);
 	if (!dbg) {
-		pr_warning("No dwarf info found in the vmlinux - "
+		pr_warning("No debug information found in the vmlinux - "
 			"please rebuild with CONFIG_DEBUG_INFO=y.\n");
 		return -EBADF;
 	}

[PATCH -tip 4/4] [BUGFIX]perf probe: Fix to support libdwfl older than 0.148

[Masami Hiramatsu]

Since the libdwfl before 0.148 fails to analyze live
kernel debuginfo, 'perf probe --list' compiled with those
old libdwfl sometime crashes.

To avoid that bug, perf probe does not use libdwfl's live
kernel analysis routine when it is compiled with older libdwfl.

Side effect: perf with older libdwfl doesn't support
listing probe in modules with source code line. Those
could be shown by symbol+offset.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: linux-kernel@vger.kernel.org
---

 tools/perf/util/probe-finder.c |   85 ++++++++++++++++++++++++++--------------
 1 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 90b6292..ab83b6a 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -117,28 +117,6 @@ static void line_list__free(struct list_head *head)
 }
 
 /* Dwarf FL wrappers */
-
-static int __linux_kernel_find_elf(Dwfl_Module *mod,
-				   void **userdata,
-				   const char *module_name,
-				   Dwarf_Addr base,
-				   char **file_name, Elf **elfp)
-{
-	int fd;
-	const char *path = kernel_get_module_path(module_name);
-
-	if (path) {
-		fd = open(path, O_RDONLY);
-		if (fd >= 0) {
-			*file_name = strdup(path);
-			return fd;
-		}
-	}
-	/* If failed, try to call standard method */
-	return dwfl_linux_kernel_find_elf(mod, userdata, module_name, base,
-					  file_name, elfp);
-}
-
 static char *debuginfo_path;	/* Currently dummy */
 
 static const Dwfl_Callbacks offline_callbacks = {
@@ -151,14 +129,6 @@ static const Dwfl_Callbacks offline_callbacks = {
 	.find_elf = dwfl_build_id_find_elf,
 };
 
-static const Dwfl_Callbacks kernel_callbacks = {
-	.find_debuginfo = dwfl_standard_find_debuginfo,
-	.debuginfo_path = &debuginfo_path,
-
-	.find_elf = __linux_kernel_find_elf,
-	.section_address = dwfl_linux_kernel_module_section_address,
-};
-
 /* Get a Dwarf from offline image */
 static Dwarf *dwfl_init_offline_dwarf(int fd, Dwfl **dwflp, Dwarf_Addr *bias)
 {
@@ -185,6 +155,38 @@ error:
 	return dbg;
 }
 
+#if _ELFUTILS_PREREQ(0, 148)
+/* This method is buggy if elfutils is older than 0.148 */
+static int __linux_kernel_find_elf(Dwfl_Module *mod,
+				   void **userdata,
+				   const char *module_name,
+				   Dwarf_Addr base,
+				   char **file_name, Elf **elfp)
+{
+	int fd;
+	const char *path = kernel_get_module_path(module_name);
+
+	pr_debug2("Use file %s for %s\n", path, module_name);
+	if (path) {
+		fd = open(path, O_RDONLY);
+		if (fd >= 0) {
+			*file_name = strdup(path);
+			return fd;
+		}
+	}
+	/* If failed, try to call standard method */
+	return dwfl_linux_kernel_find_elf(mod, userdata, module_name, base,
+					  file_name, elfp);
+}
+
+static const Dwfl_Callbacks kernel_callbacks = {
+	.find_debuginfo = dwfl_standard_find_debuginfo,
+	.debuginfo_path = &debuginfo_path,
+
+	.find_elf = __linux_kernel_find_elf,
+	.section_address = dwfl_linux_kernel_module_section_address,
+};
+
 /* Get a Dwarf from live kernel image */
 static Dwarf *dwfl_init_live_kernel_dwarf(Dwarf_Addr addr, Dwfl **dwflp,
 					  Dwarf_Addr *bias)
@@ -205,11 +207,34 @@ static Dwarf *dwfl_init_live_kernel_dwarf(Dwarf_Addr addr, Dwfl **dwflp,
 	dbg = dwfl_addrdwarf(*dwflp, addr, bias);
 	/* Here, check whether we could get a real dwarf */
 	if (!dbg) {
+		pr_debug("Failed to find kernel dwarf at %lx\n",
+			 (unsigned long)addr);
 		dwfl_end(*dwflp);
 		*dwflp = NULL;
 	}
 	return dbg;
 }
+#else
+/* With older elfutils, this just support kernel module... */
+static Dwarf *dwfl_init_live_kernel_dwarf(Dwarf_Addr addr __used, Dwfl **dwflp,
+					  Dwarf_Addr *bias)
+{
+	int fd;
+	const char *path = kernel_get_module_path("kernel");
+
+	if (!path) {
+		pr_err("Failed to find vmlinux path\n");
+		return NULL;
+	}
+
+	pr_debug2("Use file %s for debuginfo\n", path);
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	return dwfl_init_offline_dwarf(fd, dwflp, bias);
+}
+#endif
 
 /* Dwarf wrappers */
 

Re: [PATCH -tip 2/4] [BUGFIX]perf: Fix strlist__parse_list to handle const string

[Arnaldo Carvalho de Melo]

Em Fri, Dec 17, 2010 at 10:12:06PM +0900, Masami Hiramatsu escreveu:
> Fix strlist__parse_list to handle const string. Without this patch,
> strlist__parse_list() causes SEGV when caller passes a constant
> string.
 
> +++ b/tools/perf/util/strlist.c
> @@ -136,13 +136,19 @@ static int strlist__parse_list_entry(struct strlist *self, const char *s)
>  
>  int strlist__parse_list(struct strlist *self, const char *s)
>  {
> -	char *sep;
> +	char *sep, *tmp;
>  	int err;
>  
> +	/* This method requires strdup, because this changes given string */
> +	if (!self->dupstr)
> +		return -EINVAL;
> +

Why is the above check needed if you solved the problem by strnduping at
each separator?

Wouldn't be better to just stop changing the string by passing the
length to strlist__parse_list_entry, etc?

>  	while ((sep = strchr(s, ',')) != NULL) {
> -		*sep = '\0';
> -		err = strlist__parse_list_entry(self, s);
> -		*sep = ',';
> +		tmp = strndup(s, sep - s);
> +		if (tmp == NULL)
> +			return -ENOMEM;
> +		err = strlist__parse_list_entry(self, tmp);
> +		free(tmp);
>  		if (err != 0)
>  			return err;
>  		s = sep + 1;

Re: [PATCH -tip 2/4] [BUGFIX]perf: Fix strlist__parse_list to handle const string

[Masami Hiramatsu]

(2010/12/22 3:23), Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 17, 2010 at 10:12:06PM +0900, Masami Hiramatsu escreveu:
>> Fix strlist__parse_list to handle const string. Without this patch,
>> strlist__parse_list() causes SEGV when caller passes a constant
>> string.
>  
>> +++ b/tools/perf/util/strlist.c
>> @@ -136,13 +136,19 @@ static int strlist__parse_list_entry(struct strlist *self, const char *s)
>>  
>>  int strlist__parse_list(struct strlist *self, const char *s)
>>  {
>> -	char *sep;
>> +	char *sep, *tmp;
>>  	int err;
>>  
>> +	/* This method requires strdup, because this changes given string */
>> +	if (!self->dupstr)
>> +		return -EINVAL;
>> +
> 
> Why is the above check needed if you solved the problem by strnduping at
> each separator?

Without dupstr, strlist(str_node__delete) will not release
allocated buffer... And we have to release it if the parsed
string started with "file://" after loading file.

(Ah, and there is a same problem in strlist__load() too...)

> Wouldn't be better to just stop changing the string by passing the
> length to strlist__parse_list_entry, etc?

Yeah, it could be, but in that case, we need changing code
wider.
I think this is the simplest solution. :)

(Of course, we have to check dupstr flag on top of strlist__load too.)

Thank you,

>>  	while ((sep = strchr(s, ',')) != NULL) {
>> -		*sep = '\0';
>> -		err = strlist__parse_list_entry(self, s);
>> -		*sep = ',';
>> +		tmp = strndup(s, sep - s);
>> +		if (tmp == NULL)
>> +			return -ENOMEM;
>> +		err = strlist__parse_list_entry(self, tmp);
>> +		free(tmp);
>>  		if (err != 0)
>>  			return err;
>>  		s = sep + 1;

-- 
Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[tip:perf/core] perf probe: Cleanup messages

[tip-bot for Masami Hiramatsu]

Commit-ID:  0e43e5d222095ca2d1d825dd2e4fa158bdc4cc9b
Gitweb:     http://git.kernel.org/tip/0e43e5d222095ca2d1d825dd2e4fa158bdc4cc9b
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 17 Dec 2010 22:12:11 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 20:20:58 -0200

perf probe: Cleanup messages

Add new lines for error or debug messages, change dwarf related words to more
generic words (or just removed).

Cc: 2nddept-manager@sdl.hitachi.co.jp
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20101217131211.24123.40437.stgit@ltc236.sdl.hitachi.co.jp>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c  |   30 +++++++++++++++-------------
 tools/perf/util/probe-finder.c |   42 ++++++++++++++++++++-------------------
 2 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3b6a529..089d78e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -95,7 +95,7 @@ static int init_vmlinux(void)
 		goto out;
 
 	if (machine__create_kernel_maps(&machine) < 0) {
-		pr_debug("machine__create_kernel_maps ");
+		pr_debug("machine__create_kernel_maps() failed.\n");
 		goto out;
 	}
 out:
@@ -140,7 +140,8 @@ static int open_vmlinux(const char *module)
 {
 	const char *path = kernel_get_module_path(module);
 	if (!path) {
-		pr_err("Failed to find path of %s module", module ?: "kernel");
+		pr_err("Failed to find path of %s module.\n",
+		       module ?: "kernel");
 		return -ENOENT;
 	}
 	pr_debug("Try to open %s\n", path);
@@ -217,7 +218,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
 		pr_warning("Warning: No dwarf info found in the vmlinux - "
 			"please rebuild kernel with CONFIG_DEBUG_INFO=y.\n");
 		if (!need_dwarf) {
-			pr_debug("Trying to use symbols.\nn");
+			pr_debug("Trying to use symbols.\n");
 			return 0;
 		}
 	}
@@ -457,7 +458,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs,
 
 	fd = open_vmlinux(module);
 	if (fd < 0) {
-		pr_warning("Failed to open debuginfo file.\n");
+		pr_warning("Failed to open debug information file.\n");
 		return fd;
 	}
 
@@ -690,39 +691,40 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 
 	/* Exclusion check */
 	if (pp->lazy_line && pp->line) {
-		semantic_error("Lazy pattern can't be used with line number.");
+		semantic_error("Lazy pattern can't be used with"
+			       " line number.\n");
 		return -EINVAL;
 	}
 
 	if (pp->lazy_line && pp->offset) {
-		semantic_error("Lazy pattern can't be used with offset.");
+		semantic_error("Lazy pattern can't be used with offset.\n");
 		return -EINVAL;
 	}
 
 	if (pp->line && pp->offset) {
-		semantic_error("Offset can't be used with line number.");
+		semantic_error("Offset can't be used with line number.\n");
 		return -EINVAL;
 	}
 
 	if (!pp->line && !pp->lazy_line && pp->file && !pp->function) {
 		semantic_error("File always requires line number or "
-			       "lazy pattern.");
+			       "lazy pattern.\n");
 		return -EINVAL;
 	}
 
 	if (pp->offset && !pp->function) {
-		semantic_error("Offset requires an entry function.");
+		semantic_error("Offset requires an entry function.\n");
 		return -EINVAL;
 	}
 
 	if (pp->retprobe && !pp->function) {
-		semantic_error("Return probe requires an entry function.");
+		semantic_error("Return probe requires an entry function.\n");
 		return -EINVAL;
 	}
 
 	if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
 		semantic_error("Offset/Line/Lazy pattern can't be used with "
-			       "return probe.");
+			       "return probe.\n");
 		return -EINVAL;
 	}
 
@@ -996,7 +998,7 @@ int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
 
 	return tmp - buf;
 error:
-	pr_debug("Failed to synthesize perf probe argument: %s",
+	pr_debug("Failed to synthesize perf probe argument: %s\n",
 		 strerror(-ret));
 	return ret;
 }
@@ -1046,7 +1048,7 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
 
 	return buf;
 error:
-	pr_debug("Failed to synthesize perf probe point: %s",
+	pr_debug("Failed to synthesize perf probe point: %s\n",
 		 strerror(-ret));
 	if (buf)
 		free(buf);
@@ -1787,7 +1789,7 @@ static int del_trace_probe_event(int fd, const char *group,
 
 	ret = e_snprintf(buf, 128, "%s:%s", group, event);
 	if (ret < 0) {
-		pr_err("Failed to copy event.");
+		pr_err("Failed to copy event.\n");
 		return ret;
 	}
 
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 3991d73..90b6292 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -627,8 +627,8 @@ static_var:
 	regs = get_arch_regstr(regn);
 	if (!regs) {
 		/* This should be a bug in DWARF or this tool */
-		pr_warning("Mapping for DWARF register number %u "
-			   "missing on this architecture.", regn);
+		pr_warning("Mapping for the register number %u "
+			   "missing on this architecture.\n", regn);
 		return -ERANGE;
 	}
 
@@ -674,13 +674,14 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		if (ret != DW_TAG_pointer_type &&
 		    ret != DW_TAG_array_type) {
 			pr_warning("Failed to cast into string: "
-				   "%s(%s) is not a pointer nor array.",
+				   "%s(%s) is not a pointer nor array.\n",
 				   dwarf_diename(vr_die), dwarf_diename(&type));
 			return -EINVAL;
 		}
 		if (ret == DW_TAG_pointer_type) {
 			if (die_get_real_type(&type, &type) == NULL) {
-				pr_warning("Failed to get a type information.");
+				pr_warning("Failed to get a type"
+					   " information.\n");
 				return -ENOENT;
 			}
 			while (*ref_ptr)
@@ -695,7 +696,7 @@ static int convert_variable_type(Dwarf_Die *vr_die,
 		if (!die_compare_name(&type, "char") &&
 		    !die_compare_name(&type, "unsigned char")) {
 			pr_warning("Failed to cast into string: "
-				   "%s is not (unsigned) char *.",
+				   "%s is not (unsigned) char *.\n",
 				   dwarf_diename(vr_die));
 			return -EINVAL;
 		}
@@ -805,8 +806,8 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 			return -EINVAL;
 		}
 		if (field->name[0] == '[') {
-			pr_err("Semantic error: %s is not a pointor nor array.",
-			       varname);
+			pr_err("Semantic error: %s is not a pointor"
+			       " nor array.\n", varname);
 			return -EINVAL;
 		}
 		if (field->ref) {
@@ -953,7 +954,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwarf_Addr paddr,
 	name = dwarf_diename(sp_die);
 	if (name) {
 		if (dwarf_entrypc(sp_die, &eaddr) != 0) {
-			pr_warning("Failed to get entry pc of %s\n",
+			pr_warning("Failed to get entry address of %s\n",
 				   dwarf_diename(sp_die));
 			return -ENOENT;
 		}
@@ -969,7 +970,7 @@ static int convert_to_trace_point(Dwarf_Die *sp_die, Dwarf_Addr paddr,
 	if (retprobe) {
 		if (eaddr != paddr) {
 			pr_warning("Return probe must be on the head of"
-				   " a real function\n");
+				   " a real function.\n");
 			return -EINVAL;
 		}
 		tp->retprobe = true;
@@ -1008,7 +1009,7 @@ static int call_probe_finder(Dwarf_Die *sp_die, struct probe_finder *pf)
 		Dwarf_Frame *frame;
 		if (dwarf_cfi_addrframe(pf->cfi, pf->addr, &frame) != 0 ||
 		    dwarf_frame_cfa(frame, &pf->fb_ops, &nops) != 0) {
-			pr_warning("Failed to get CFA on 0x%jx\n",
+			pr_warning("Failed to get call frame on 0x%jx\n",
 				   (uintmax_t)pf->addr);
 			return -ENOENT;
 		}
@@ -1035,7 +1036,7 @@ static int find_probe_point_by_line(struct probe_finder *pf)
 	int ret = 0;
 
 	if (dwarf_getsrclines(&pf->cu_die, &lines, &nlines) != 0) {
-		pr_warning("No source lines found in this CU.\n");
+		pr_warning("No source lines found.\n");
 		return -ENOENT;
 	}
 
@@ -1137,7 +1138,7 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
 	}
 
 	if (dwarf_getsrclines(&pf->cu_die, &lines, &nlines) != 0) {
-		pr_warning("No source lines found in this CU.\n");
+		pr_warning("No source lines found.\n");
 		return -ENOENT;
 	}
 
@@ -1195,7 +1196,7 @@ static int probe_point_inline_cb(Dwarf_Die *in_die, void *data)
 	else {
 		/* Get probe address */
 		if (dwarf_entrypc(in_die, &addr) != 0) {
-			pr_warning("Failed to get entry pc of %s.\n",
+			pr_warning("Failed to get entry address of %s.\n",
 				   dwarf_diename(in_die));
 			param->retval = -ENOENT;
 			return DWARF_CB_ABORT;
@@ -1236,8 +1237,8 @@ static int probe_point_search_cb(Dwarf_Die *sp_die, void *data)
 			param->retval = find_probe_point_lazy(sp_die, pf);
 		else {
 			if (dwarf_entrypc(sp_die, &pf->addr) != 0) {
-				pr_warning("Failed to get entry pc of %s.\n",
-					   dwarf_diename(sp_die));
+				pr_warning("Failed to get entry address of "
+					   "%s.\n", dwarf_diename(sp_die));
 				param->retval = -ENOENT;
 				return DWARF_CB_ABORT;
 			}
@@ -1279,7 +1280,7 @@ static int find_probes(int fd, struct probe_finder *pf)
 
 	dbg = dwfl_init_offline_dwarf(fd, &dwfl, &bias);
 	if (!dbg) {
-		pr_warning("No dwarf info found in the vmlinux - "
+		pr_warning("No debug information found in the vmlinux - "
 			"please rebuild with CONFIG_DEBUG_INFO=y.\n");
 		return -EBADF;
 	}
@@ -1524,7 +1525,7 @@ int find_perf_probe_point(unsigned long addr, struct perf_probe_point *ppt)
 	/* Open the live linux kernel */
 	dbg = dwfl_init_live_kernel_dwarf(addr, &dwfl, &bias);
 	if (!dbg) {
-		pr_warning("No dwarf info found in the vmlinux - "
+		pr_warning("No debug information found in the vmlinux - "
 			"please rebuild with CONFIG_DEBUG_INFO=y.\n");
 		ret = -EINVAL;
 		goto end;
@@ -1534,7 +1535,8 @@ int find_perf_probe_point(unsigned long addr, struct perf_probe_point *ppt)
 	addr += bias;
 	/* Find cu die */
 	if (!dwarf_addrdie(dbg, (Dwarf_Addr)addr - bias, &cudie)) {
-		pr_warning("No CU DIE is found at %lx\n", addr);
+		pr_warning("Failed to find debug information for address %lx\n",
+			   addr);
 		ret = -EINVAL;
 		goto end;
 	}
@@ -1659,7 +1661,7 @@ static int find_line_range_by_line(Dwarf_Die *sp_die, struct line_finder *lf)
 
 	line_list__init(&lf->lr->line_list);
 	if (dwarf_getsrclines(&lf->cu_die, &lines, &nlines) != 0) {
-		pr_warning("No source lines found in this CU.\n");
+		pr_warning("No source lines found.\n");
 		return -ENOENT;
 	}
 
@@ -1784,7 +1786,7 @@ int find_line_range(int fd, struct line_range *lr)
 
 	dbg = dwfl_init_offline_dwarf(fd, &dwfl, &bias);
 	if (!dbg) {
-		pr_warning("No dwarf info found in the vmlinux - "
+		pr_warning("No debug information found in the vmlinux - "
 			"please rebuild with CONFIG_DEBUG_INFO=y.\n");
 		return -EBADF;
 	}

[tip:perf/urgent] perf tools: Fix lazy wildcard matching

[tip-bot for Masami Hiramatsu]

Commit-ID:  ea187cfbb9a3de73e7bd7b7125ae345d92d4384b
Gitweb:     http://git.kernel.org/tip/ea187cfbb9a3de73e7bd7b7125ae345d92d4384b
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 17 Dec 2010 22:12:00 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 19:15:42 -0200

perf tools: Fix lazy wildcard matching

Fix lazy wildcard matching to ignore space after wild card.

Cc: 2nddept-manager@sdl.hitachi.co.jp
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20101217131200.24123.8202.stgit@ltc236.sdl.hitachi.co.jp>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/string.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 0409fc7..8fc0bd3 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -259,7 +259,7 @@ static bool __match_glob(const char *str, const char *pat, bool ignore_space)
 		if (!*pat)	/* Tail wild card matches all */
 			return true;
 		while (*str)
-			if (strglobmatch(str++, pat))
+			if (__match_glob(str++, pat, ignore_space))
 				return true;
 	}
 	return !*str && !*pat;

[tip:perf/urgent] perf probe: Fix to support libdwfl older than 0.148

[tip-bot for Masami Hiramatsu]

Commit-ID:  3b4694de3596edac0159524b15a4a06945875421
Gitweb:     http://git.kernel.org/tip/3b4694de3596edac0159524b15a4a06945875421
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 17 Dec 2010 22:12:18 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 21 Dec 2010 19:24:57 -0200

perf probe: Fix to support libdwfl older than 0.148

Since the libdwfl library before 0.148 fails to analyze live kernel debuginfo,
'perf probe --list' compiled with those old libdwfl sometimes crashes.

To avoid that bug, perf probe does not use libdwfl's live kernel analysis
routine when it is compiled with older libdwfl.

Side effect: perf with older libdwfl doesn't support listing probe in modules
with source code line. Those could be shown by symbol+offset.

Cc: 2nddept-manager@sdl.hitachi.co.jp
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20101217131218.24123.62424.stgit@ltc236.sdl.hitachi.co.jp>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c |   85 ++++++++++++++++++++++++++--------------
 1 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 3991d73..ddf4d45 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -117,28 +117,6 @@ static void line_list__free(struct list_head *head)
 }
 
 /* Dwarf FL wrappers */
-
-static int __linux_kernel_find_elf(Dwfl_Module *mod,
-				   void **userdata,
-				   const char *module_name,
-				   Dwarf_Addr base,
-				   char **file_name, Elf **elfp)
-{
-	int fd;
-	const char *path = kernel_get_module_path(module_name);
-
-	if (path) {
-		fd = open(path, O_RDONLY);
-		if (fd >= 0) {
-			*file_name = strdup(path);
-			return fd;
-		}
-	}
-	/* If failed, try to call standard method */
-	return dwfl_linux_kernel_find_elf(mod, userdata, module_name, base,
-					  file_name, elfp);
-}
-
 static char *debuginfo_path;	/* Currently dummy */
 
 static const Dwfl_Callbacks offline_callbacks = {
@@ -151,14 +129,6 @@ static const Dwfl_Callbacks offline_callbacks = {
 	.find_elf = dwfl_build_id_find_elf,
 };
 
-static const Dwfl_Callbacks kernel_callbacks = {
-	.find_debuginfo = dwfl_standard_find_debuginfo,
-	.debuginfo_path = &debuginfo_path,
-
-	.find_elf = __linux_kernel_find_elf,
-	.section_address = dwfl_linux_kernel_module_section_address,
-};
-
 /* Get a Dwarf from offline image */
 static Dwarf *dwfl_init_offline_dwarf(int fd, Dwfl **dwflp, Dwarf_Addr *bias)
 {
@@ -185,6 +155,38 @@ error:
 	return dbg;
 }
 
+#if _ELFUTILS_PREREQ(0, 148)
+/* This method is buggy if elfutils is older than 0.148 */
+static int __linux_kernel_find_elf(Dwfl_Module *mod,
+				   void **userdata,
+				   const char *module_name,
+				   Dwarf_Addr base,
+				   char **file_name, Elf **elfp)
+{
+	int fd;
+	const char *path = kernel_get_module_path(module_name);
+
+	pr_debug2("Use file %s for %s\n", path, module_name);
+	if (path) {
+		fd = open(path, O_RDONLY);
+		if (fd >= 0) {
+			*file_name = strdup(path);
+			return fd;
+		}
+	}
+	/* If failed, try to call standard method */
+	return dwfl_linux_kernel_find_elf(mod, userdata, module_name, base,
+					  file_name, elfp);
+}
+
+static const Dwfl_Callbacks kernel_callbacks = {
+	.find_debuginfo = dwfl_standard_find_debuginfo,
+	.debuginfo_path = &debuginfo_path,
+
+	.find_elf = __linux_kernel_find_elf,
+	.section_address = dwfl_linux_kernel_module_section_address,
+};
+
 /* Get a Dwarf from live kernel image */
 static Dwarf *dwfl_init_live_kernel_dwarf(Dwarf_Addr addr, Dwfl **dwflp,
 					  Dwarf_Addr *bias)
@@ -205,11 +207,34 @@ static Dwarf *dwfl_init_live_kernel_dwarf(Dwarf_Addr addr, Dwfl **dwflp,
 	dbg = dwfl_addrdwarf(*dwflp, addr, bias);
 	/* Here, check whether we could get a real dwarf */
 	if (!dbg) {
+		pr_debug("Failed to find kernel dwarf at %lx\n",
+			 (unsigned long)addr);
 		dwfl_end(*dwflp);
 		*dwflp = NULL;
 	}
 	return dbg;
 }
+#else
+/* With older elfutils, this just support kernel module... */
+static Dwarf *dwfl_init_live_kernel_dwarf(Dwarf_Addr addr __used, Dwfl **dwflp,
+					  Dwarf_Addr *bias)
+{
+	int fd;
+	const char *path = kernel_get_module_path("kernel");
+
+	if (!path) {
+		pr_err("Failed to find vmlinux path\n");
+		return NULL;
+	}
+
+	pr_debug2("Use file %s for debuginfo\n", path);
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	return dwfl_init_offline_dwarf(fd, dwflp, bias);
+}
+#endif
 
 /* Dwarf wrappers */
 

Re: [PATCH -tip 2/4] [BUGFIX]perf: Fix strlist__parse_list to handle const string

[Arnaldo Carvalho de Melo]

Em Wed, Dec 22, 2010 at 01:57:39PM +0900, Masami Hiramatsu escreveu:
> (2010/12/22 3:23), Arnaldo Carvalho de Melo wrote:
> > Em Fri, Dec 17, 2010 at 10:12:06PM +0900, Masami Hiramatsu escreveu:
> > Wouldn't be better to just stop changing the string by passing the
> > length to strlist__parse_list_entry, etc?
> 
> Yeah, it could be, but in that case, we need changing code
> wider.
> I think this is the simplest solution. :)

Ok, I'll try to fix it as I suggested, it it turns out difficult I'll
apply the minimal fix.
 
> (Of course, we have to check dupstr flag on top of strlist__load too.)
> 
> Thank you,

You're welcome,

- Arnaldo