linux-kernel - RE: [PATCH 3/4] perf annotate: add powerpc support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Jun 2016 16:07:10 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	'Ravi Bangoria' <ravi.bangoria@...ux.vnet.ibm.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"acme@...nel.org" <acme@...nel.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
CC:	"ananth@...ibm.com" <ananth@...ibm.com>,
	"naveen.n.rao@...ux.vnet.ibm.com" <naveen.n.rao@...ux.vnet.ibm.com>,
	"dja@...ens.net" <dja@...ens.net>
Subject: RE: [PATCH 3/4] perf annotate: add powerpc support

From: Ravi Bangoria
> Sent: 28 June 2016 12:37
> 
> Powerpc has long list of branch instructions and hardcoding them in table
> appears to be error-prone. So, add new function to find instruction
> instead of creating table.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
> ---
>  tools/perf/util/annotate.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 36a5825..96c6610 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -476,6 +476,68 @@ static int ins__cmp(const void *a, const void *b)
>  	return strcmp(ia->name, ib->name);
>  }
> 
> +static struct ins *ins__find_powerpc(const char *name)

It would be better if the function name include 'branch'.

> +{
> +	int i;
> +	struct ins *ins;
> +
> +	ins = zalloc(sizeof(struct ins));
> +	if (!ins)
> +		return NULL;
> +
> +	ins->name = strdup(name);
> +	if (!ins->name)
> +		return NULL;

	You leak 'ins' here.

> +
> +	if (name[0] == 'b') {
> +		/* branch instructions */
> +		ins->ops = &jump_ops;
> +
> +		/*
> +		 * - Few start with 'b', but aren't branch instructions.
> +		 * - Let's also ignore instructions involving 'ctr' and
> +		 *   'tar' since target branch addresses for those can't
> +		 *   be determined statically.
> +		 */
> +		if (!strncmp(name, "bcd", 3)   ||
> +		    !strncmp(name, "brinc", 5) ||
> +		    !strncmp(name, "bper", 4)  ||
> +		    strstr(name, "ctr")        ||
> +		    strstr(name, "tar"))
> +			return NULL;

	More importantly you leak 'ins' and 'ins->name' here.
	And on other paths below.

...

	David