Thread (7 messages) 7 messages, 2 authors, 2016-06-29

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

From: David Laight <hidden>
Date: 2016-06-28 16:09:41
Also in: linuxppc-dev

From: Ravi Bangoria
quoted hunk ↗ jump to hunk
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 <redacted>
Signed-off-by: Ravi Bangoria <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help