2015-02-26 07:14:08

by Naohiro Aota

[permalink] [raw]
Subject: [PATCH 1/2] perf probe: export get_real_path

Export it to use from util/probe-finder.c

Signed-off-by: Naohiro Aota <[email protected]>
---
tools/perf/util/probe-event.c | 2 +-
tools/perf/util/probe-event.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 919937e..1d0d505 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -520,7 +520,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
* 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, const char *comp_dir,
+int get_real_path(const char *raw_path, const char *comp_dir,
char **new_path)
{
const char *prefix = symbol_conf.source_prefix;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e01e994..30a3391 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -135,6 +135,8 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
struct strfilter *filter, bool externs);
extern int show_available_funcs(const char *module, struct strfilter *filter,
bool user);
+extern int get_real_path(const char *raw_path, const char *comp_dir,
+ char **new_path);

/* Maximum index number of event-name postfix */
#define MAX_EVENT_INDEX 1024
--
2.3.0


2015-02-26 07:14:31

by Naohiro Aota

[permalink] [raw]
Subject: [PATCH 2/2] perf probe: Find compilation directory path for lazy matching

If we use lazy matching, it failed to open a souce file if perf command
is invoked outside of compilation directory:

$ perf probe -a '__schedule;clear_*'
Failed to open kernel/sched/core.c: No such file or directory
Error: Failed to add events. (-2)

OTOH, other commands like "probe -L" can solve the souce directory by
themselves. Let's make it possible for lazy matching too!

Signed-off-by: Naohiro Aota <[email protected]>
---
tools/perf/util/probe-finder.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index b5247d7..8e0714c 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -39,6 +39,7 @@
#include "util.h"
#include "symbol.h"
#include "probe-finder.h"
+#include "probe-event.h"

/* Kprobe tracer basic type is up to u64 */
#define MAX_BASIC_TYPE_BITS 64
@@ -849,11 +850,23 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
{
int ret = 0;
+ char *fpath;

if (intlist__empty(pf->lcache)) {
+ const char *comp_dir;
+
+ comp_dir = cu_get_comp_dir(&pf->cu_die);
+ ret = get_real_path(pf->fname, comp_dir, &fpath);
+ if (ret < 0) {
+ free(fpath);
+ pr_warning("Failed to find source file path.\n");
+ return ret;
+ }
+
/* Matching lazy line pattern */
- ret = find_lazy_match_lines(pf->lcache, pf->fname,
+ ret = find_lazy_match_lines(pf->lcache, fpath,
pf->pev->point.lazy_line);
+ free(fpath);
if (ret <= 0)
return ret;
}
--
2.3.0

Subject: Re: [PATCH 1/2] perf probe: export get_real_path

(2015/02/26 16:12), Naohiro Aota wrote:
> Export it to use from util/probe-finder.c

Please fold this in to the next patch, since this exported symbol
is not used until applying the next one.

BTW, since get_real_path is compiled only when HAVE_DWARF_SUPPORT=y,
we can also move it into probe-finder.c.
Could you also move it into probe-finder.c and export it at probe-finder.h?

Thank you,

>
> Signed-off-by: Naohiro Aota <[email protected]>
> ---
> tools/perf/util/probe-event.c | 2 +-
> tools/perf/util/probe-event.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 919937e..1d0d505 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -520,7 +520,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> * 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, const char *comp_dir,
> +int get_real_path(const char *raw_path, const char *comp_dir,
> char **new_path)
> {
> const char *prefix = symbol_conf.source_prefix;
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index e01e994..30a3391 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -135,6 +135,8 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
> struct strfilter *filter, bool externs);
> extern int show_available_funcs(const char *module, struct strfilter *filter,
> bool user);
> +extern int get_real_path(const char *raw_path, const char *comp_dir,
> + char **new_path);
>
> /* Maximum index number of event-name postfix */
> #define MAX_EVENT_INDEX 1024
>


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: Re: [PATCH 2/2] perf probe: Find compilation directory path for lazy matching

(2015/02/26 16:12), Naohiro Aota wrote:
> If we use lazy matching, it failed to open a souce file if perf command
> is invoked outside of compilation directory:
>
> $ perf probe -a '__schedule;clear_*'
> Failed to open kernel/sched/core.c: No such file or directory
> Error: Failed to add events. (-2)
>
> OTOH, other commands like "probe -L" can solve the souce directory by
> themselves. Let's make it possible for lazy matching too!
>
> Signed-off-by: Naohiro Aota <[email protected]>
> ---
> tools/perf/util/probe-finder.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index b5247d7..8e0714c 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -39,6 +39,7 @@
> #include "util.h"
> #include "symbol.h"
> #include "probe-finder.h"
> +#include "probe-event.h"
>
> /* Kprobe tracer basic type is up to u64 */
> #define MAX_BASIC_TYPE_BITS 64
> @@ -849,11 +850,23 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
> static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
> {
> int ret = 0;
> + char *fpath;
>
> if (intlist__empty(pf->lcache)) {
> + const char *comp_dir;
> +
> + comp_dir = cu_get_comp_dir(&pf->cu_die);
> + ret = get_real_path(pf->fname, comp_dir, &fpath);
> + if (ret < 0) {
> + free(fpath);

Here, if the get_real_path is failed, fpath should be freed before returning.
If not, there is a bug, and yeah, there is a bug...

Thank you!

> + pr_warning("Failed to find source file path.\n");
> + return ret;
> + }
> +
> /* Matching lazy line pattern */
> - ret = find_lazy_match_lines(pf->lcache, pf->fname,
> + ret = find_lazy_match_lines(pf->lcache, fpath,
> pf->pev->point.lazy_line);
> + free(fpath);
> if (ret <= 0)
> return ret;
> }
>


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: [PATCH perf/core ] [BUGFIX] perf-probe: Fix get_real_path to free allocated memory in error path

Fix get_real_path to free allocated memory when comp_dir
is used for complementing path and getting an error.

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/probe-event.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 9dfbed9..43cc534 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -549,9 +549,11 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
if (access(*new_path, R_OK) == 0)
return 0;

- if (!symbol_conf.source_prefix)
+ if (!symbol_conf.source_prefix) {
/* In case of searching comp_dir, don't retry */
+ zfree(new_path);
return -errno;
+ }

switch (errno) {
case ENAMETOOLONG:

2015-02-26 14:46:42

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH perf/core ] [BUGFIX] perf-probe: Fix get_real_path to free allocated memory in error path

Em Thu, Feb 26, 2015 at 05:25:04PM +0900, Masami Hiramatsu escreveu:
> Fix get_real_path to free allocated memory when comp_dir
> is used for complementing path and getting an error.


While reviewing this patch I noticed this is needed, ack?

- Arnaldo


diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 4a93bf433344..9526cf37682e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -533,7 +533,7 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
else {
if (access(raw_path, R_OK) == 0) {
*new_path = strdup(raw_path);
- return 0;
+ return *new_path ? 0 : -ENOMEM;
} else
return -errno;
}

Subject: Re: [PATCH perf/core ] [BUGFIX] perf-probe: Fix get_real_path to free allocated memory in error path

(2015/02/26 23:46), Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 26, 2015 at 05:25:04PM +0900, Masami Hiramatsu escreveu:
>> Fix get_real_path to free allocated memory when comp_dir
>> is used for complementing path and getting an error.
>
>
> While reviewing this patch I noticed this is needed, ack?

Ah, right!

Acked-by: Masami Hiramatsu <[email protected]>

Thank you,

>
> - Arnaldo
>
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 4a93bf433344..9526cf37682e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -533,7 +533,7 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
> else {
> if (access(raw_path, R_OK) == 0) {
> *new_path = strdup(raw_path);
> - return 0;
> + return *new_path ? 0 : -ENOMEM;
> } else
> return -errno;
> }
>


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: [tip:perf/core] perf probe: Fix get_real_path to free allocated memory in error path

Commit-ID: eb47cb2eb22dfacac9689708f5bd3cb0e975e290
Gitweb: http://git.kernel.org/tip/eb47cb2eb22dfacac9689708f5bd3cb0e975e290
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Thu, 26 Feb 2015 17:25:04 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 26 Feb 2015 11:59:05 -0300

perf probe: Fix get_real_path to free allocated memory in error path

Fix get_real_path to free allocated memory when comp_dir is used for
complementing path and getting an error.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Naohiro Aota <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/probe-event.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 662d454..4a93bf4 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -549,9 +549,11 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
if (access(*new_path, R_OK) == 0)
return 0;

- if (!symbol_conf.source_prefix)
+ if (!symbol_conf.source_prefix) {
/* In case of searching comp_dir, don't retry */
+ zfree(new_path);
return -errno;
+ }

switch (errno) {
case ENAMETOOLONG: