perf annotate: Defer allocating sym_priv->hist array

commit 628ada0cb03666dd463f7c25947eaccdf440c309 upstream

Because symbol->end is not fixed up at symbol_filter time, only
after all symbols for a DSO are loaded, and that, for asm
symbols, may be bogus, causing segfaults when hits happen in
these symbols.

Backported-from: 628ada0
Reported-by: David Miller <davem@davemloft.net>
Reported-by: Anton Blanchard <anton@samba.org>
Acked-by: David Miller <davem@davemloft.net>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
LKML-Reference: <20100225155740.GB8553@ghostprotocols.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

3 files changed, 35 insertions, 32 deletions

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 593ff25006de..0b1ba36393dd 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -53,32 +53,20 @@ struct sym_priv {
 
 static const char *sym_hist_filter;
 
-static int symbol_filter(struct map *map __used, struct symbol *sym)
+static int sym__alloc_hist(struct symbol *self)
 {
-        if (sym_hist_filter == NULL ||
-            strcmp(sym->name, sym_hist_filter) == 0) {
-                struct sym_priv *priv = symbol__priv(sym);
-                const int size = (sizeof(*priv->hist) +
-                                  (sym->end - sym->start) * sizeof(u64));
+        struct sym_priv *priv = symbol__priv(self);
+        const int size = (sizeof(*priv->hist) +
+                          (self->end - self->start) * sizeof(u64));
 
-                priv->hist = malloc(size);
-                if (priv->hist)
-                        memset(priv->hist, 0, size);
-                return 0;
-        }
-        /*
-         * FIXME: We should really filter it out, as we don't want to go thru symbols
-         * we're not interested, and if a DSO ends up with no symbols, delete it too,
-         * but right now the kernel loading routines in symbol.c bail out if no symbols
-         * are found, fix it later.
-         */
-        return 0;
+        priv->hist = zalloc(size);
+        return priv->hist == NULL ? -1 : 0;
 }
 
 /*
  * collect histogram counts
  */
-static void hist_hit(struct hist_entry *he, u64 ip)
+static int annotate__hist_hit(struct hist_entry *he, u64 ip)
 {
         unsigned int sym_size, offset;
         struct symbol *sym = he->sym;
@@ -88,11 +76,11 @@ static void hist_hit(struct hist_entry *he, u64 ip)
         he->count++;
 
         if (!sym || !he->map)
-                return;
+                return 0;
 
         priv = symbol__priv(sym);
-        if (!priv->hist)
-                return;
+        if (priv->hist == NULL && sym__alloc_hist(sym) < 0)
+                return -ENOMEM;
 
         sym_size = sym->end - sym->start;
         offset = ip - sym->start;
@@ -102,7 +90,7 @@ static void hist_hit(struct hist_entry *he, u64 ip)
                         he->map->unmap_ip(he->map, ip));
 
         if (offset >= sym_size)
-                return;
+                return 0;
 
         h = priv->hist;
         h->sum++;
@@ -114,18 +102,31 @@ static void hist_hit(struct hist_entry *he, u64 ip)
                         he->sym->name,
                         (void *)(unsigned long)ip, ip - he->sym->start,
                         h->ip[offset]);
+        return 0;
 }
 
 static int perf_session__add_hist_entry(struct perf_session *self,
                                         struct addr_location *al, u64 count)
 {
-        bool hit;
-        struct hist_entry *he = __perf_session__add_hist_entry(self, al, NULL,
-                                                               count, &hit);
-        if (he == NULL)
-                return -ENOMEM;
-        hist_hit(he, al->addr);
-        return 0;
+        bool hit;
+        struct hist_entry *he;
+
+        if (sym_hist_filter != NULL &&
+            (al->sym == NULL || strcmp(sym_hist_filter, al->sym->name) != 0)) {
+                /* We're only interested in a symbol named sym_hist_filter */
+                if (al->sym != NULL) {
+                        rb_erase(&al->sym->rb_node,
+                                 &al->map->dso->symbols[al->map->type]);
+                        symbol__delete(al->sym);
+                }
+                return 0;
+        }
+
+        he = __perf_session__add_hist_entry(self, al, NULL, count, &hit);
+        if (he == NULL)
+                return -ENOMEM;
+
+        return annotate__hist_hit(he, al->addr);
 }
 
 static int process_sample_event(event_t *event, struct perf_session *session)
@@ -135,7 +136,7 @@ static int process_sample_event(event_t *event, struct perf_session *session)
         dump_printf("(IP, %d): %d: %p\n", event->header.misc,
                     event->ip.pid, (void *)(long)event->ip.ip);
 
-        if (event__preprocess_sample(event, session, &al, symbol_filter) < 0) {
+        if (event__preprocess_sample(event, session, &al, NULL) < 0) {
                 fprintf(stderr, "problem processing %d event, skipping it.\n",
                         event->header.type);
                 return -1;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 72547b914ad2..fcb8919887ae 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -149,7 +149,7 @@ static struct symbol *symbol__new(u64 start, u64 len, const char *name)
         return self;
 }
 
-static void symbol__delete(struct symbol *self)
+void symbol__delete(struct symbol *self)
 {
         free(((void *)self) - symbol_conf.priv_size);
 }
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 8aded2356f79..400227a22d09 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -49,6 +49,8 @@ struct symbol {
         char            name[0];
 };
 
+void symbol__delete(struct symbol *self);
+
 struct strlist;
 
 struct symbol_conf {