Re: [PATCH] perf, tools: Add better errors to annotate

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

Em Thu, Nov 05, 2015 at 07:06:07PM -0800, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> When the browser fails to annotate it is difficult for users to find
> out what went wrong.
> 
> Add some errors for objdump failures that are displayed in the UI.
> 
> Note it would be event better to handle these errors smarter, like
> falling back to the binary when the debug info is somehow
> corrupted. But for now just giving a better error is an improvement.

So this is an improvement for 'perf ann otate --stdio', where, after I
added a (1 ||) to one of the paths you added a pr_err(), I got:

  [root@zoo ~]# perf annotate --stdio intel_idle
  no symbols found in /usr/bin/procmail, maybe install a debug package?
  Failed to open /tmp/perf-8683.map, continuing without symbols
  Failure running objdump  --start-address=0xffffffff81418290 --stop-address=0xffffffff814183ae -l -d --no-show-ra
   Percent |      Source code & Disassembly of vmlinux for cycles:pp
  ------------------------------------------------------------------

But doesn't work at all for --tui (and probably --gtk), where it will
print the warning on the last line in the screen, that gets immediatelly
rewritten :-\

I am applying it, as it improves the --stdio case, and will fix this in
a way that works for all UIs, with some strerror() interface to convert
whatever error happened to a string that then gets used in whatever
UI is being used.

Thanks,

- Arnaldo

 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/annotate.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index d1eece7..07f9544 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1081,6 +1081,7 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
>  	struct kcore_extract kce;
>  	bool delete_extract = false;
>  	int lineno = 0;
> +	int nline;
>  
>  	if (filename)
>  		symbol__join_symfs(symfs_filename, filename);
> @@ -1176,6 +1177,9 @@ fallback:
>  
>  		ret = decompress_to_file(m.ext, symfs_filename, fd);
>  
> +		if (ret)
> +			pr_err("Cannot decompress %s %s\n", m.ext, symfs_filename);
> +
>  		free(m.ext);
>  		close(fd);
>  
> @@ -1201,13 +1205,25 @@ fallback:
>  	pr_debug("Executing: %s\n", command);
>  
>  	file = popen(command, "r");
> -	if (!file)
> +	if (!file) {
> +		pr_err("Failure running %s\n", command);
> +		/*
> +		 * If we were using debug info should retry with
> +		 * original binary.
> +		 */
>  		goto out_remove_tmp;
> +	}
>  
> -	while (!feof(file))
> +	nline = 0;
> +	while (!feof(file)) {
>  		if (symbol__parse_objdump_line(sym, map, file, privsize,
>  			    &lineno) < 0)
>  			break;
> +		nline++;
> +	}
> +
> +	if (nline == 0)
> +		pr_err("No output from %s\n", command);
>  
>  	/*
>  	 * kallsyms does not have symbol sizes so there may a nop at the end.
> -- 
> 2.4.3