Re: [PATCH v2] perf probe: Find compilation directory path for lazy matching

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Mar 04, 2015 at 09:34:38PM +0900, Masami Hiramatsu escreveu:
> (2015/03/04 16:52), 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!
> > 
> 
> Looks good to me :)
> 
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

This doesn't make sense... se below:
 
> Thank you!
> 
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 46f009a..e6c0262 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -849,11 +849,22 @@ 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) {
> > +			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;
> >  	}
> > @@ -1616,3 +1627,61 @@ found:
> >  	return (ret < 0) ? ret : lf.found;
> >  }
> >  
> > +/*
> > + * 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, const char *comp_dir,
> > +			 char **new_path)

The function is marked "static"

> > +{
> > +	const char *prefix = symbol_conf.source_prefix;
> > +
> > +	if (!prefix) {
> > +		if (raw_path[0] != '/' && comp_dir)
> > +			/* If not an absolute path, try to use comp_dir */
> > +			prefix = comp_dir;
> > +		else {
> > +			if (access(raw_path, R_OK) == 0) {
> > +				*new_path = strdup(raw_path);
> > +				return *new_path ? 0 : -ENOMEM;
> > +			} else
> > +				return -errno;
> > +		}
> > +	}
> > +
> > +	*new_path = malloc((strlen(prefix) + strlen(raw_path) + 2));
> > +	if (!*new_path)
> > +		return -ENOMEM;
> > +
> > +	for (;;) {
> > +		sprintf(*new_path, "%s/%s", prefix, raw_path);
> > +
> > +		if (access(*new_path, R_OK) == 0)
> > +			return 0;
> > +
> > +		if (!symbol_conf.source_prefix) {
> > +			/* In case of searching comp_dir, don't retry */
> > +			zfree(new_path);
> > +			return -errno;
> > +		}
> > +
> > +		switch (errno) {
> > +		case ENAMETOOLONG:
> > +		case ENOENT:
> > +		case EROFS:
> > +		case EFAULT:
> > +			raw_path = strchr(++raw_path, '/');
> > +			if (!raw_path) {
> > +				zfree(new_path);
> > +				return -ENOENT;
> > +			}
> > +			continue;
> > +
> > +		default:
> > +			zfree(new_path);
> > +			return -errno;
> > +		}
> > +	}
> > +}
> > diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
> > index 92590b2..5ef82dd 100644
> > --- a/tools/perf/util/probe-finder.h
> > +++ b/tools/perf/util/probe-finder.h
> > @@ -55,6 +55,10 @@ extern int debuginfo__find_available_vars_at(struct debuginfo *dbg,
> >  					     struct variable_list **vls,
> >  					     int max_points, bool externs);
> >  
> > +/* Find a src file from a DWARF tag path */
> > +extern int get_real_path(const char *raw_path, const char *comp_dir,
> > +			 char **new_path);
> > +

And then you mark it "extern"? Have you tried to compile this? I tried:
 
  CC       /tmp/build/perf/tests/task-exit.o
util/probe-finder.c:1636:12: error: static declaration of ‘get_real_path’ follows non-static declaration
 static int get_real_path(const char *raw_path, const char *comp_dir,
            ^
In file included from util/probe-finder.c:41:0:
util/probe-finder.h:59:12: note: previous declaration of ‘get_real_path’ was here
 extern int get_real_path(const char *raw_path, const char *comp_dir,
            ^
util/probe-finder.c:1636:12: error: ‘get_real_path’ defined but not used [-Werror=unused-function]
 static int get_real_path(const char *raw_path, const char *comp_dir,
            ^
cc1: all warnings being treated as errors
make[3]: *** [/tmp/build/perf/util/probe-finder.o] Error 1
make[3]: *** Waiting for unfinished jobs....
  CC       /tmp/build/perf/tests/sw-clock.o

Also please refrain from using 'extern' in front of function prototypes, its just noise.

- Arnaldo

> >  struct probe_finder {
> >  	struct perf_probe_event	*pev;		/* Target probe event */
> >  
> > 
> 
> 
> -- 
> Masami HIRAMATSU
> Software Platform Research Dept. Linux Technology Research Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>