Re: [PATCH v4 2/4] perf tools: add Java demangling support

[Arnaldo Carvalho de Melo <acme@redhat.com>]

Em Tue, Mar 24, 2015 at 11:25:31PM +0100, Stephane Eranian escreveu:

<SNIP>

> +static char *
> +__demangle_java_sym(const char *str, const char *end, char *buf, int maxlen, int mode)
> +{
> +	int rlen = 0;
> +	int array = 0;
> +	int narg = 0;
> +	const char *q;
> +
> +	if (!end)
> +		end = str + strlen(str);
> +
> +	for (q = str; q != end; q++) {
> +
> +		if (rlen == (maxlen - 1))
> +			break;
> +
> +		switch (*q) {
> +		case 'L':
> +			if (mode == MODE_PREFIX || mode == MODE_CTYPE) {
> +				if (mode == MODE_CTYPE) {
> +					if (narg)
> +						rlen += snprintf(buf+rlen, maxlen - rlen, ", ");
> +					narg++;
> +				}
> +				rlen += snprintf(buf+rlen, maxlen - rlen, "class ");

I think you should replace all these snprintf with scnprintf to avoid
that trap described in its man page:

-----
Return value:

       The functions snprintf() and vsnprintf() do not write more than
size bytes (including the terminating null byte ('\0')).  If the output
was truncated due to this limit then  the  return  value  is  the number
of  characters (excluding  the  terminating  null  byte) which would
have been written to the final string if enough space had been
available.  Thus, a return value of size or more means that the output
was truncated.  (See also below under NOTES.)
-----

I.e. it will not return the number of bytes _printed_, which is what you
expect, no? I.e. you don't seem to be checking for truncation.

Nit:

please have spaces separating operators consistently, i.e.  sometimes
you use 'a+b', sometimes 'a + b', please use consistently 'a + b'.

- Arnaldo


> +				if (mode == MODE_PREFIX)
> +					mode = MODE_CLASS;
> +			} else
> +				buf[rlen++] = *q;
> +			break;
> +		case 'B':
> +		case 'C':
> +		case 'D':
> +		case 'F':
> +		case 'I':
> +		case 'J':
> +		case 'S':
> +		case 'Z':
> +			if (mode == MODE_TYPE) {
> +				if (narg)
> +					rlen += snprintf(buf+rlen, maxlen - rlen, ", ");
> +				rlen += snprintf(buf+rlen, maxlen - rlen, "%s", base_types[*q - 'A']);
> +				while(array--)
> +					rlen += snprintf(buf+rlen, maxlen - rlen, "[]");
> +				array = 0;
> +				narg++;
> +			} else
> +				buf[rlen++] = *q;
> +			break;
> +		case 'V':
> +			if (mode == MODE_TYPE) {
> +				rlen += snprintf(buf+rlen, maxlen - rlen, "void");
> +				while(array--)
> +					rlen += snprintf(buf+rlen, maxlen - rlen, "[]");
> +				array = 0;
> +			} else
> +				buf[rlen++] = *q;
> +			break;
> +		case '[':
> +			if (mode != MODE_TYPE)
> +				goto error;
> +			array++;
> +			break;
> +		case '(':
> +			if (mode != MODE_FUNC)
> +				goto error;
> +			buf[rlen++] = *q;
> +			mode = MODE_TYPE;
> +			break;
> +		case ')':
> +			if (mode != MODE_TYPE)
> +				goto error;
> +			buf[rlen++] = *q;
> +			narg = 0;
> +			break;
> +		case ';':
> +			if (mode != MODE_CLASS && mode != MODE_CTYPE)
> +				goto error;
> +			/* safe because at least one other char to process */
> +			if (isalpha(*(q+1)))
> +				rlen += snprintf(buf+rlen, maxlen - rlen, ".");
> +			if (mode == MODE_CLASS)
> +				mode = MODE_FUNC;
> +			else if (mode == MODE_CTYPE)
> +				mode = MODE_TYPE;
> +			break;
> +		case '/':
> +			if (mode != MODE_CLASS && mode != MODE_CTYPE)
> +				goto error;
> +			rlen += snprintf(buf+rlen, maxlen - rlen, ".");
> +			break;
> +		default :
> +			buf[rlen++] = *q;
> +		}
> +	}
> +	buf[rlen] = '\0';
> +	return buf;
> +error:
> +	return NULL;
> +}
> +
> +/*
> + * Demangle Java function signature (openJDK, not GCJ)
> + * input:
> + * 	str: string to parse. String is not modified
> + *    flags: comobination of JAVA_DEMANGLE_* flags to modify demangling
> + * return:
> + *	if input can be demangled, then a newly allocated string is returned.
> + *	if input cannot be demangled, then NULL is returned
> + *
> + * Note: caller is responsible for freeing demangled string
> + */
> +char *
> +java_demangle_sym(const char *str, int flags)
> +{
> +	char *buf, *ptr;
> +	char *p;
> +	size_t len, l1 = 0;
> +
> +	if (!str)
> +		return NULL;
> +
> +	/* find start of retunr type */
> +	p = strrchr(str, ')');
> +	if (!p)
> +		return NULL;
> +
> +	/*
> +	 * expansion factor estimated to 3x
> +	 */
> +	len = strlen(str) * 3 + 1;
> +	buf = malloc(len);
> +	if (!buf)
> +		return NULL;
> +
> +	buf[0] = '\0';
> +	if (!(flags & JAVA_DEMANGLE_NORET)) {
> +		/*
> +		 * get return type first
> +		 */
> +		ptr = __demangle_java_sym(p+1, NULL, buf, len, MODE_TYPE);
> +		if (!ptr)
> +			goto error;
> +
> +		/* add space between return type and function prototype */
> +		l1 = strlen(buf);
> +		buf[l1++] = ' ';
> +	}
> +
> +	/* process function up to return type */
> +	ptr = __demangle_java_sym(str, p + 1, buf + l1, len - l1, MODE_PREFIX);
> +	if (!ptr)
> +		goto error;
> +
> +	return buf;
> +error:
> +	free(buf);
> +	return NULL;
> +}
> diff --git a/tools/perf/util/demangle-java.h b/tools/perf/util/demangle-java.h
> new file mode 100644
> index 0000000..a981c1f
> --- /dev/null
> +++ b/tools/perf/util/demangle-java.h
> @@ -0,0 +1,10 @@
> +#ifndef __PERF_DEMANGLE_JAVA
> +#define __PERF_DEMANGLE_JAVA 1
> +/*
> + * demangle function flags
> + */
> +#define JAVA_DEMANGLE_NORET	0x1 /* do not process return type */
> +
> +char * java_demangle_sym(const char *str, int flags);
> +
> +#endif /* __PERF_DEMANGLE_JAVA */
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 476268c..2174bf4 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -6,6 +6,7 @@
>  #include <inttypes.h>
>  
>  #include "symbol.h"
> +#include "demangle-java.h"
>  #include "machine.h"
>  #include "vdso.h"
>  #include <symbol/kallsyms.h>
> @@ -1046,6 +1047,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
>  				demangle_flags = DMGL_PARAMS | DMGL_ANSI;
>  
>  			demangled = bfd_demangle(NULL, elf_name, demangle_flags);
> +			if (demangled == NULL)
> +				demangled = java_demangle_sym(elf_name, JAVA_DEMANGLE_NORET);
>  			if (demangled != NULL)
>  				elf_name = demangled;
>  		}
> -- 
> 1.9.1