Re: [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string

Re: [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string

[Arnaldo Carvalho de Melo]

Em Wed, Nov 18, 2015 at 03:40:14PM +0900, Masami Hiramatsu escreveu:
> Since system_path() returns malloc'd string if given path is
> not an absolute path, perf_exec_path sometimes returns static
> string and sometimes returns malloc'd string depends on the
> environment variables or command options.
> 
> This causes a memory leak because caller can not free the
> returned string.
> 
> This fixes perf_exec_path and system_path to always return
> malloc'd string, so caller can always free it.
 
>  /* Returns the highest-priority, location to look for perf programs. */
> -const char *perf_exec_path(void)
> +char *perf_exec_path(void)
>  {
> -	const char *env;
> +	char *env;
>  
>  	if (argv_exec_path)
> -		return argv_exec_path;
> +		return strdup(argv_exec_path);

So here you return strdup without checking if it fails

>  
>  	env = getenv(EXEC_PATH_ENVIRONMENT);
>  	if (env && *env) {
> -		return env;
> +		env = strdup(env);
> +		if (env)
> +			return env;

But here you change the behaviour by, having a EXEC_PATH_ENVIRONMENT,
fallback to PERF_EXEC_PATH if strdup fails, that in turn can end up
returning NULL, since system_path() doesn't check the strdup() result?

I wonder if in those cases we couldn't check the address range for the
pointer being freed and realize it is in .bss, .data or that it is a
stack variable? Maybe there is some malloc() friend that, given a
pointer, tells that yeah, it was allocated?

- Arnaldo

>  	}
>  
>  	return system_path(PERF_EXEC_PATH);
> @@ -83,9 +85,11 @@ void setup_path(void)
>  {
>  	const char *old_path = getenv("PATH");
>  	struct strbuf new_path = STRBUF_INIT;
> +	char *tmp = perf_exec_path();
>  
> -	add_path(&new_path, perf_exec_path());
> +	add_path(&new_path, tmp);
>  	add_path(&new_path, argv0_path);
> +	free(tmp);
>  
>  	if (old_path)
>  		strbuf_addstr(&new_path, old_path);
> diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
> index bc4b915..48b4175 100644
> --- a/tools/perf/util/exec_cmd.h
> +++ b/tools/perf/util/exec_cmd.h
> @@ -3,10 +3,11 @@
>  
>  extern void perf_set_argv_exec_path(const char *exec_path);
>  extern const char *perf_extract_argv0_path(const char *path);
> -extern const char *perf_exec_path(void);
>  extern void setup_path(void);
>  extern int execv_perf_cmd(const char **argv); /* NULL terminated */
>  extern int execl_perf_cmd(const char *cmd, ...);
> -extern const char *system_path(const char *path);
> +/* perf_exec_path and system_path return malloc'd string, caller must free it */
> +extern char *perf_exec_path(void);
> +extern char *system_path(const char *path);
>  
>  #endif /* __PERF_EXEC_CMD_H */
> diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
> index 86c37c4..fa1fc4a 100644
> --- a/tools/perf/util/help.c
> +++ b/tools/perf/util/help.c
> @@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
>  		struct cmdnames *other_cmds)
>  {
>  	const char *env_path = getenv("PATH");
> -	const char *exec_path = perf_exec_path();
> +	char *exec_path = perf_exec_path();
>  
>  	if (exec_path) {
>  		list_commands_in_dir(main_cmds, exec_path, prefix);
> @@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
>  		      sizeof(*other_cmds->names), cmdname_compare);
>  		uniq(other_cmds);
>  	}
> +	free(exec_path);
>  	exclude_cmds(other_cmds, main_cmds);
>  }
>  
> @@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
>  			longest = other_cmds->names[i]->len;
>  
>  	if (main_cmds->cnt) {
> -		const char *exec_path = perf_exec_path();
> +		char *exec_path = perf_exec_path();
>  		printf("available %s in '%s'\n", title, exec_path);
>  		printf("----------------");
>  		mput_char('-', strlen(title) + strlen(exec_path));
>  		putchar('\n');
>  		pretty_print_string_list(main_cmds, longest);
>  		putchar('\n');
> +		free(exec_path);
>  	}
>  
>  	if (other_cmds->cnt) {

----- End forwarded message -----

RE: [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string

[平松雅巳 / HIRAMATU，MASAMI]

From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
>
>Em Wed, Nov 18, 2015 at 03:40:14PM +0900, Masami Hiramatsu escreveu:
>> Since system_path() returns malloc'd string if given path is
>> not an absolute path, perf_exec_path sometimes returns static
>> string and sometimes returns malloc'd string depends on the
>> environment variables or command options.
>>
>> This causes a memory leak because caller can not free the
>> returned string.
>>
>> This fixes perf_exec_path and system_path to always return
>> malloc'd string, so caller can always free it.
>
>>  /* Returns the highest-priority, location to look for perf programs. */
>> -const char *perf_exec_path(void)
>> +char *perf_exec_path(void)
>>  {
>> -	const char *env;
>> +	char *env;
>>
>>  	if (argv_exec_path)
>> -		return argv_exec_path;
>> +		return strdup(argv_exec_path);
>
>So here you return strdup without checking if it fails
>
>>
>>  	env = getenv(EXEC_PATH_ENVIRONMENT);
>>  	if (env && *env) {
>> -		return env;
>> +		env = strdup(env);
>> +		if (env)
>> +			return env;
>
>But here you change the behaviour by, having a EXEC_PATH_ENVIRONMENT,
>fallback to PERF_EXEC_PATH if strdup fails, that in turn can end up
>returning NULL, since system_path() doesn't check the strdup() result?

Ah, right. actually this is the first part where I fixed, and at that
point I thought this is better. But afterwards, I changed my mind to
return strdup directly. (If there is no memory caller must handle it)
So, I think the above should be

	if (env && *env)
		return strdup(env);

>
>I wonder if in those cases we couldn't check the address range for the
>pointer being freed and realize it is in .bss, .data or that it is a
>stack variable? Maybe there is some malloc() friend that, given a
>pointer, tells that yeah, it was allocated?

I wonder too. But as far as I took a look, I couldn't find such functions.

Thank you,

>
>- Arnaldo
>
>>  	}
>>
>>  	return system_path(PERF_EXEC_PATH);
>> @@ -83,9 +85,11 @@ void setup_path(void)
>>  {
>>  	const char *old_path = getenv("PATH");
>>  	struct strbuf new_path = STRBUF_INIT;
>> +	char *tmp = perf_exec_path();
>>
>> -	add_path(&new_path, perf_exec_path());
>> +	add_path(&new_path, tmp);
>>  	add_path(&new_path, argv0_path);
>> +	free(tmp);
>>
>>  	if (old_path)
>>  		strbuf_addstr(&new_path, old_path);
>> diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
>> index bc4b915..48b4175 100644
>> --- a/tools/perf/util/exec_cmd.h
>> +++ b/tools/perf/util/exec_cmd.h
>> @@ -3,10 +3,11 @@
>>
>>  extern void perf_set_argv_exec_path(const char *exec_path);
>>  extern const char *perf_extract_argv0_path(const char *path);
>> -extern const char *perf_exec_path(void);
>>  extern void setup_path(void);
>>  extern int execv_perf_cmd(const char **argv); /* NULL terminated */
>>  extern int execl_perf_cmd(const char *cmd, ...);
>> -extern const char *system_path(const char *path);
>> +/* perf_exec_path and system_path return malloc'd string, caller must free it */
>> +extern char *perf_exec_path(void);
>> +extern char *system_path(const char *path);
>>
>>  #endif /* __PERF_EXEC_CMD_H */
>> diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
>> index 86c37c4..fa1fc4a 100644
>> --- a/tools/perf/util/help.c
>> +++ b/tools/perf/util/help.c
>> @@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
>>  		struct cmdnames *other_cmds)
>>  {
>>  	const char *env_path = getenv("PATH");
>> -	const char *exec_path = perf_exec_path();
>> +	char *exec_path = perf_exec_path();
>>
>>  	if (exec_path) {
>>  		list_commands_in_dir(main_cmds, exec_path, prefix);
>> @@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
>>  		      sizeof(*other_cmds->names), cmdname_compare);
>>  		uniq(other_cmds);
>>  	}
>> +	free(exec_path);
>>  	exclude_cmds(other_cmds, main_cmds);
>>  }
>>
>> @@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
>>  			longest = other_cmds->names[i]->len;
>>
>>  	if (main_cmds->cnt) {
>> -		const char *exec_path = perf_exec_path();
>> +		char *exec_path = perf_exec_path();
>>  		printf("available %s in '%s'\n", title, exec_path);
>>  		printf("----------------");
>>  		mput_char('-', strlen(title) + strlen(exec_path));
>>  		putchar('\n');
>>  		pretty_print_string_list(main_cmds, longest);
>>  		putchar('\n');
>> +		free(exec_path);
>>  	}
>>
>>  	if (other_cmds->cnt) {
>
>----- End forwarded message -----

[PATCH perf/core v2] perf: Make perf_exec_path always returns malloc'd string

[Masami Hiramatsu]

Since system_path() returns malloc'd string if given path is
not an absolute path, perf_exec_path sometimes returns static
string and sometimes returns malloc'd string depends on the
environment variables or command options.

This causes a memory leak because caller can not free the
returned string.

This fixes perf_exec_path and system_path to always return
malloc'd string, so caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
  Changes in v2:
   - do not change the behavior if strdup is failed.
---
 tools/perf/util/exec_cmd.c |   21 +++++++++++----------
 tools/perf/util/exec_cmd.h |    5 +++--
 tools/perf/util/help.c     |    6 ++++--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..1099e92 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,17 +52,16 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	char *env;
 
 	if (argv_exec_path)
-		return argv_exec_path;
+		return strdup(argv_exec_path);
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
-	if (env && *env) {
-		return env;
-	}
+	if (env && *env)
+		return strdup(env);
 
 	return system_path(PERF_EXEC_PATH);
 }
@@ -83,9 +82,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {

[tip:perf/core] perf tools: Make perf_exec_path() always return malloc'd string

[tip-bot for Masami Hiramatsu]

Commit-ID:  c4068f51d40df151a661a384ab1309b11d7f012e
Gitweb:     http://git.kernel.org/tip/c4068f51d40df151a661a384ab1309b11d7f012e
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Thu, 19 Nov 2015 15:04:53 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Nov 2015 13:19:19 -0300

perf tools: Make perf_exec_path() always return malloc'd string

Since system_path() returns malloc'd string if given path is not an
absolute path, perf_exec_path() sometimes returns a static string and
sometimes returns a malloc'd string depending on the environment
variables or command options.

This may cause a memory leak because the caller can not unconditionally
free the returned string.

This fixes perf_exec_path() and system_path() to always return a
malloc'd string, so the caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151119060453.14210.65666.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/exec_cmd.c | 21 +++++++++++----------
 tools/perf/util/exec_cmd.h |  5 +++--
 tools/perf/util/help.c     |  6 ++++--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..1099e92 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,17 +52,16 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	char *env;
 
 	if (argv_exec_path)
-		return argv_exec_path;
+		return strdup(argv_exec_path);
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
-	if (env && *env) {
-		return env;
-	}
+	if (env && *env)
+		return strdup(env);
 
 	return system_path(PERF_EXEC_PATH);
 }
@@ -83,9 +82,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {

[PATCH perf/core 00/13] perf memory/refcnt leak fixes

[Masami Hiramatsu]

Hi,

Here is a series to fix some memory leaks and refcount
leaks on map and dso. This also includes the refcnt APIs
with backtrace debugging feature.

The story has started from the posible memory leak report
reported by Wnag Nan.
I've tried to use valgrind to ensure the perf probe doesn't
have other memory leaks. The result is here:

  ----
  # valgrind ./perf probe vfs_read
  ==17521== Memcheck, a memory error detector
  ==17521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==17521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
  ==17521== Command: ./perf probe vfs_read
  ==17521==
  Added new event:
    probe:vfs_read       (on vfs_read)
  
  You can now use it in all perf tools, such as:
  
          perf record -e probe:vfs_read -aR sleep 1
  
  ==17521==
  ==17521== HEAP SUMMARY:
  ==17521==     in use at exit: 3,512,761 bytes in 38,012 blocks
  ==17521==   total heap usage: 74,723 allocs, 36,711 frees, 24,014,927
  bytes allocated
  ==17521==
  ==17521== LEAK SUMMARY:
  ==17521==    definitely lost: 6,857 bytes in 49 blocks
  ==17521==    indirectly lost: 3,501,287 bytes in 37,891 blocks
  ==17521==      possibly lost: 0 bytes in 0 blocks
  ==17521==    still reachable: 4,617 bytes in 72 blocks
  ==17521==         suppressed: 0 bytes in 0 blocks
  ==17521== Rerun with --leak-check=full to see details of leaked memory
  ==17521==
  ==17521== For counts of detected and suppressed errors, rerun with: -v
  ==17521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
  ----

Oops! It leaked almost 4 MB memories. I've tried to find
the root causes, and what I've found is there are many
leaks in not only perf-probe specific code, but also maps
and dsos (and some other pieces).

The first 3 patches are just fixing 'easy' memory leaks. However,
most of the leaks are caused by refcnt. Since valgrind seems not
able to debug this kind of issues, I introduced a hand-made refcnt
backtrace APIs for debugging.
The rest of patches are for fixing refcnt leak bugs and replcing
refcnt apis.

After all, most of the issues are gone, except for just a few issues
in elfutils. I'll continue to investigate that.

  ----
  valgrind ./perf probe vfs_read
  ==29521== Memcheck, a memory error detector
  ==29521== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
  ==29521== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
  ==29521== Command: ./perf probe vfs_read
  ==29521==
  Added new event:
    probe:vfs_read       (on vfs_read)
  
  You can now use it in all perf tools, such as:
  
          perf record -e probe:vfs_read -aR sleep 1
  
  ==29521==
  ==29521== HEAP SUMMARY:
  ==29521==     in use at exit: 5,137 bytes in 75 blocks
  ==29521==   total heap usage: 74,723 allocs, 74,648 frees, 24,014,927
  bytes allocated
  ==29521==
  ==29521== LEAK SUMMARY:
  ==29521==    definitely lost: 520 bytes in 3 blocks
  ==29521==    indirectly lost: 0 bytes in 0 blocks
  ==29521==      possibly lost: 0 bytes in 0 blocks
  ==29521==    still reachable: 4,617 bytes in 72 blocks
  ==29521==         suppressed: 0 bytes in 0 blocks
  ==29521== Rerun with --leak-check=full to see details of leaked memory
  ==29521==
  ==29521== For counts of detected and suppressed errors, rerun with: -v
  ==29521== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
  ----

Anyway, I decided to release these fixes and the debugging feature
because at least this will improve perf quality.

Thank you,

---

Masami Hiramatsu (13):
      perf probe: Fix to free temporal Dwarf_Frame
      perf: Make perf_exec_path always returns malloc'd string
      perf: Introduce generic refcount APIs with debug feature
      perf: make map to use refcnt
      perf: Fix machine__findnew_module_map to put registered map
      perf: Fix machine__destroy_kernel_maps to put vmlinux_maps
      perf: Fix to destroy kernel maps when machine exits
      perf: Fix to put new map after inserting to map_groups in dso__load_sym
      perf: Make dso to use refcnt for debug
      perf: Fix __dsos__addnew to put dso after adding it to the list
      perf: Fix machine__create_kernel_maps to put kernel dso
      perf: Fix machine__findnew_module_map to put dso
      perf: Fix dso__load_sym to put dso


 tools/perf/config/Makefile     |    5 ++
 tools/perf/util/Build          |    1 
 tools/perf/util/dso.c          |    9 ++-
 tools/perf/util/exec_cmd.c     |   20 ++++--
 tools/perf/util/exec_cmd.h     |    5 +-
 tools/perf/util/help.c         |    6 +-
 tools/perf/util/machine.c      |   17 ++++-
 tools/perf/util/map.c          |    7 +-
 tools/perf/util/map.h          |    3 +
 tools/perf/util/probe-finder.c |    9 ++-
 tools/perf/util/refcnt.c       |  125 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/refcnt.h       |   65 +++++++++++++++++++++
 tools/perf/util/symbol-elf.c   |    4 +
 13 files changed, 250 insertions(+), 26 deletions(-)
 create mode 100644 tools/perf/util/refcnt.c
 create mode 100644 tools/perf/util/refcnt.h


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering 
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string

[Masami Hiramatsu]

Since system_path() returns malloc'd string if given path is
not an absolute path, perf_exec_path sometimes returns static
string and sometimes returns malloc'd string depends on the
environment variables or command options.

This causes a memory leak because caller can not free the
returned string.

This fixes perf_exec_path and system_path to always return
malloc'd string, so caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/exec_cmd.c |   20 ++++++++++++--------
 tools/perf/util/exec_cmd.h |    5 +++--
 tools/perf/util/help.c     |    6 ++++--
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..7031ffc 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,16 +52,18 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	char *env;
 
 	if (argv_exec_path)
-		return argv_exec_path;
+		return strdup(argv_exec_path);
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
-		return env;
+		env = strdup(env);
+		if (env)
+			return env;
 	}
 
 	return system_path(PERF_EXEC_PATH);
@@ -83,9 +85,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {