All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf top: fix crash on annotate request
@ 2011-10-19 18:23 David Ahern
  2011-10-19 18:38 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: David Ahern @ 2011-10-19 18:23 UTC (permalink / raw)
  To: acme, linux-kernel; +Cc: mingo, peterz, fweisbec, David Ahern

Hitting an annotate case where src is not set and
perf-top crashes.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/perf/util/annotate.c             |    3 +++
 tools/perf/util/ui/browsers/annotate.c |    2 ++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bc8f477..26652b1 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -205,6 +205,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 	size_t line_len;
 	s64 line_ip, offset = -1;
 
+	if (!notes->src)
+		return -1;
+
 	if (getline(&line, &line_len, file) < 0)
 		return -1;
 
diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index a2c351c..5a67ead 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -410,6 +410,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	ui_helpline__push("Press <- or ESC to exit");
 
 	notes = symbol__annotation(sym);
+	if (!notes->src)
+		return -1;
 
 	list_for_each_entry(pos, &notes->src->source, node) {
 		struct objdump_line_rb_node *rbpos;
-- 
1.7.6.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 18:23 [PATCH] perf top: fix crash on annotate request David Ahern
@ 2011-10-19 18:38 ` Arnaldo Carvalho de Melo
  2011-10-19 18:44   ` David Ahern
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-19 18:38 UTC (permalink / raw)
  To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec

Em Wed, Oct 19, 2011 at 12:23:18PM -0600, David Ahern escreveu:
> Hitting an annotate case where src is not set and
> perf-top crashes.

How did you got there? Navigating thru callq? I  wonder if in this case
we'd instead shouldn't call

symbol__alloc_hist(sym, evlist->nr_entries)

And proceed, only complaining on ENOMEM, like we do in parse_source in
builtin-top.c.

I.e. if the user asked to go to a function without hits, no problem,
allocate the data structures needed for doing the annotation and show 0
hits on all lines.

- Arnaldo
 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  tools/perf/util/annotate.c             |    3 +++
>  tools/perf/util/ui/browsers/annotate.c |    2 ++
>  2 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bc8f477..26652b1 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -205,6 +205,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>  	size_t line_len;
>  	s64 line_ip, offset = -1;
>  
> +	if (!notes->src)
> +		return -1;
> +
>  	if (getline(&line, &line_len, file) < 0)
>  		return -1;
>  
> diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
> index a2c351c..5a67ead 100644
> --- a/tools/perf/util/ui/browsers/annotate.c
> +++ b/tools/perf/util/ui/browsers/annotate.c
> @@ -410,6 +410,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
>  	ui_helpline__push("Press <- or ESC to exit");
>  
>  	notes = symbol__annotation(sym);
> +	if (!notes->src)
> +		return -1;
>  
>  	list_for_each_entry(pos, &notes->src->source, node) {
>  		struct objdump_line_rb_node *rbpos;
> -- 
> 1.7.6.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 18:38 ` Arnaldo Carvalho de Melo
@ 2011-10-19 18:44   ` David Ahern
  2011-10-19 19:20     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: David Ahern @ 2011-10-19 18:44 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, mingo, peterz, fweisbec



On 10/19/2011 12:38 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 19, 2011 at 12:23:18PM -0600, David Ahern escreveu:
>> Hitting an annotate case where src is not set and
>> perf-top crashes.
> 
> How did you got there? 

One stack trace:
#0  0x0000000000434ba8 in list_add_tail (new=0x7fffec000c30, head=0x0)
    at util/include/../../../../include/linux/list.h:76
#1  0x00000000004351a5 in objdump__add_line (head=0x0,
line=0x7fffec000c30) at util/annotate.c:101
#2  0x000000000043584f in symbol__parse_objdump_line (sym=0xf505f0,
map=0x98de00, file=0x7fffec000aa0,
    privsize=40) at util/annotate.c:256
#3  0x0000000000435d45 in symbol__annotate (sym=0xf505f0, map=0x98de00,
privsize=40) at util/annotate.c:346
#4  0x00000000004829a0 in symbol__tui_annotate (sym=0xf505f0,
map=0x98de00, evidx=0, nr_events=1,
    timer=0x426f9e <perf_top__sort_new_samples>, arg=0x77a300,
delay_secs=2) at util/ui/browsers/annotate.c:405
#5  0x0000000000482867 in hist_entry__tui_annotate (he=0x991ca0,
evidx=0, nr_events=1,
    timer=0x426f9e <perf_top__sort_new_samples>, arg=0x77a300,
delay_secs=2) at util/ui/browsers/annotate.c:373
#6  0x0000000000485684 in perf_evsel__hists_browse (evsel=0x910710,
nr_events=1,
    helpline=0x5201e8 "For a higher level overview, try: perf top --sort
comm,dso", ev_name=0x910990 "cycles",
    left_exits=false, timer=0x426f9e <perf_top__sort_new_samples>,
arg=0x77a300, delay_secs=2)
    at util/ui/browsers/hists.c:991
...


I was starting perf top, selecting a symbol and pressing 'a'. In the
crash case it was the perf command itself.

David


> Navigating thru callq? I  wonder if in this case
> we'd instead shouldn't call
> 
> symbol__alloc_hist(sym, evlist->nr_entries)
> 
> And proceed, only complaining on ENOMEM, like we do in parse_source in
> builtin-top.c.
> 
> I.e. if the user asked to go to a function without hits, no problem,
> allocate the data structures needed for doing the annotation and show 0
> hits on all lines.
> 
> - Arnaldo
>  
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>>  tools/perf/util/annotate.c             |    3 +++
>>  tools/perf/util/ui/browsers/annotate.c |    2 ++
>>  2 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index bc8f477..26652b1 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -205,6 +205,9 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>>  	size_t line_len;
>>  	s64 line_ip, offset = -1;
>>  
>> +	if (!notes->src)
>> +		return -1;
>> +
>>  	if (getline(&line, &line_len, file) < 0)
>>  		return -1;
>>  
>> diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
>> index a2c351c..5a67ead 100644
>> --- a/tools/perf/util/ui/browsers/annotate.c
>> +++ b/tools/perf/util/ui/browsers/annotate.c
>> @@ -410,6 +410,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
>>  	ui_helpline__push("Press <- or ESC to exit");
>>  
>>  	notes = symbol__annotation(sym);
>> +	if (!notes->src)
>> +		return -1;
>>  
>>  	list_for_each_entry(pos, &notes->src->source, node) {
>>  		struct objdump_line_rb_node *rbpos;
>> -- 
>> 1.7.6.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 18:44   ` David Ahern
@ 2011-10-19 19:20     ` Arnaldo Carvalho de Melo
  2011-10-19 20:21       ` David Ahern
                         ` (2 more replies)
  0 siblings, 3 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-19 19:20 UTC (permalink / raw)
  To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec

Em Wed, Oct 19, 2011 at 12:44:48PM -0600, David Ahern escreveu:
> On 10/19/2011 12:38 PM, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Oct 19, 2011 at 12:23:18PM -0600, David Ahern escreveu:
> >> Hitting an annotate case where src is not set and
> >> perf-top crashes.

> > How did you got there? 
> 
> I was starting perf top, selecting a symbol and pressing 'a'. In the
> crash case it was the perf command itself.

Humm, looks like a race, the old 'perf top --tui' had this:

static void perf_top_browser__annotate(struct perf_top_browser *browser)
{
        struct sym_entry *syme = browser->selection;
        struct symbol *sym = sym_entry__symbol(syme);
        struct annotation *notes = symbol__annotation(sym);
        struct perf_top *top = browser->b.priv;

        if (notes->src != NULL)
                goto do_annotation;

        pthread_mutex_lock(&notes->lock);

        top->sym_filter_entry = NULL;

        if (symbol__alloc_hist(sym, top->evlist->nr_entries) < 0) {
                pr_err("Not enough memory for annotating '%s' symbol!\n",
                       sym->name);
                pthread_mutex_unlock(&notes->lock);
                return;
        }

        top->sym_filter_entry = syme;

        pthread_mutex_unlock(&notes->lock);
do_annotation:
        symbol__tui_annotate(sym, syme->map, 0, top->delay_secs * 1000);
}


Which is not even completely right, the notes->src should happen inside
the lock, like parse_source in the --stdio...

Can you check if that is the problem? I.e. take notes->lock, check if
->src is NULL, if so call symbol__alloc_hist, etc?

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 19:20     ` Arnaldo Carvalho de Melo
@ 2011-10-19 20:21       ` David Ahern
  2011-10-19 21:39       ` David Ahern
  2011-10-19 22:12       ` David Ahern
  2 siblings, 0 replies; 26+ messages in thread
From: David Ahern @ 2011-10-19 20:21 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, mingo, peterz, fweisbec



On 10/19/2011 01:20 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 19, 2011 at 12:44:48PM -0600, David Ahern escreveu:
>> On 10/19/2011 12:38 PM, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Oct 19, 2011 at 12:23:18PM -0600, David Ahern escreveu:
>>>> Hitting an annotate case where src is not set and
>>>> perf-top crashes.
> 
>>> How did you got there? 
>>
>> I was starting perf top, selecting a symbol and pressing 'a'. In the
>> crash case it was the perf command itself.
> 
> Humm, looks like a race, the old 'perf top --tui' had this:
> 
> static void perf_top_browser__annotate(struct perf_top_browser *browser)
> {
>         struct sym_entry *syme = browser->selection;
>         struct symbol *sym = sym_entry__symbol(syme);
>         struct annotation *notes = symbol__annotation(sym);
>         struct perf_top *top = browser->b.priv;
> 
>         if (notes->src != NULL)
>                 goto do_annotation;
> 
>         pthread_mutex_lock(&notes->lock);
> 
>         top->sym_filter_entry = NULL;
> 
>         if (symbol__alloc_hist(sym, top->evlist->nr_entries) < 0) {
>                 pr_err("Not enough memory for annotating '%s' symbol!\n",
>                        sym->name);
>                 pthread_mutex_unlock(&notes->lock);
>                 return;
>         }
> 
>         top->sym_filter_entry = syme;
> 
>         pthread_mutex_unlock(&notes->lock);
> do_annotation:
>         symbol__tui_annotate(sym, syme->map, 0, top->delay_secs * 1000);
> }
> 
> 
> Which is not even completely right, the notes->src should happen inside
> the lock, like parse_source in the --stdio...
> 
> Can you check if that is the problem? I.e. take notes->lock, check if
> ->src is NULL, if so call symbol__alloc_hist, etc?

Well, I rebooted my laptop during lunch and cannot repeat the crash. I
had installed prelink yesterday to get it to do its thing which it had
-- almost every process (gnome, daemons, terminals, shells, etc) had
deleted dso's.

Today (just a suspend overnight) I started looking at the perf-top code.
I noticed that the annotate key was doing nothing - except on the perf
command itself where it crashed.

After rebooting - which cleared all the prelink deletions - it works
fine. Re-running prelink (force mode) gets the prelink deletions, but
annotate in perf-top works fine. If it happens again I'll try your
suggestion.

David

> 
> - Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 19:20     ` Arnaldo Carvalho de Melo
  2011-10-19 20:21       ` David Ahern
@ 2011-10-19 21:39       ` David Ahern
  2011-10-20 12:51         ` Arnaldo Carvalho de Melo
  2011-10-19 22:12       ` David Ahern
  2 siblings, 1 reply; 26+ messages in thread
From: David Ahern @ 2011-10-19 21:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, mingo, peterz, fweisbec



On 10/19/2011 01:20 PM, Arnaldo Carvalho de Melo wrote:
> Can you check if that is the problem? I.e. take notes->lock, check if
> ->src is NULL, if so call symbol__alloc_hist, etc?
> 

Reproduced it on a lab box. This fixes the segfault:

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bc8f477..6bd501a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -253,6 +253,17 @@ static int symbol__parse_objdump_line(struct symbol
*sym, struct map *map,
        free(line);
        return -1;
    }
+
+   pthread_mutex_lock(&notes->lock);
+   if (notes->src == NULL &&
+       symbol__alloc_hist(sym, 1) < 0) {
+       pthread_mutex_unlock(&notes->lock);
+       ui__warning("Not enough memory for annotating '%s' symbol!\n",
+               sym->name);
+       return -1;
+   }
+   pthread_mutex_unlock(&notes->lock);
+
    objdump__add_line(&notes->src->source, objdump_line);

    return 0;

Should I plumb nr_events down versus assuming its 1?

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 19:20     ` Arnaldo Carvalho de Melo
  2011-10-19 20:21       ` David Ahern
  2011-10-19 21:39       ` David Ahern
@ 2011-10-19 22:12       ` David Ahern
  2011-10-20 13:00         ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 26+ messages in thread
From: David Ahern @ 2011-10-19 22:12 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, mingo, peterz, fweisbec



Getting another crash with the default sym sorting -- the addr is much
less than the start of the sym so the offset goes negative:

(gdb) bt
#0  0x0000000000429de3 in symbol__inc_addr_samples (sym=0x8f0f90,
map=0x8aae00, evidx=0,
    addr=329985) at util/annotate.c:73
#1  0x000000000041b073 in record_precise_ip (he=0x8a2a10, counter=0,
ip=329985)
    at builtin-top.c:221
#2  0x000000000041c821 in perf_event__process_sample (event=0x7fffefbc74c8,
    sample=0x7fffffffe1b0, session=0x89a140) at builtin-top.c:801
#3  0x000000000041c8d4 in perf_session__mmap_read_idx (self=0x89a140,
idx=12)
    at builtin-top.c:821
#4  0x000000000041c95b in perf_session__mmap_read (self=0x89a140) at
builtin-top.c:832
#5  0x000000000041cdf1 in __cmd_top () at builtin-top.c:960
#6  0x000000000041d585 in cmd_top (argc=0, argv=0x7fffffffe590, prefix=0x0)
    at builtin-top.c:1252
#7  0x00000000004077b9 in run_builtin (p=0x75fb68, argc=2,
argv=0x7fffffffe590) at perf.c:286
#8  0x00000000004079bb in handle_internal_command (argc=2,
argv=0x7fffffffe590) at perf.c:358
#9  0x0000000000407b07 in run_argv (argcp=0x7fffffffe47c,
argv=0x7fffffffe470) at perf.c:402
#10 0x0000000000407dee in main (argc=2, argv=0x7fffffffe590) at perf.c:512


The following fixes the crash. If it seems reasonable I'll add to the
other one:

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bc8f477..f1f20b5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -63,6 +63,8 @@ int symbol__inc_addr_samples(struct symbol *sym,
struct map *map,
        return -ENOMEM;

    pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map->unmap_ip(map,
addr));
+   if (addr < sym->start)
+       return 0;

    if (addr >= sym->end)
        return 0;


I'll combine

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 21:39       ` David Ahern
@ 2011-10-20 12:51         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-20 12:51 UTC (permalink / raw)
  To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec

Em Wed, Oct 19, 2011 at 03:39:30PM -0600, David Ahern escreveu:
> On 10/19/2011 01:20 PM, Arnaldo Carvalho de Melo wrote:
> > Can you check if that is the problem? I.e. take notes->lock, check if
> > ->src is NULL, if so call symbol__alloc_hist, etc?
> > 
> 
> Reproduced it on a lab box. This fixes the segfault:
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bc8f477..6bd501a 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -253,6 +253,17 @@ static int symbol__parse_objdump_line(struct symbol
> *sym, struct map *map,
>         free(line);
>         return -1;
>     }
> +
> +   pthread_mutex_lock(&notes->lock);
> +   if (notes->src == NULL &&
> +       symbol__alloc_hist(sym, 1) < 0) {
> +       pthread_mutex_unlock(&notes->lock);
> +       ui__warning("Not enough memory for annotating '%s' symbol!\n",
> +               sym->name);
> +       return -1;
> +   }
> +   pthread_mutex_unlock(&notes->lock);
> +
>     objdump__add_line(&notes->src->source, objdump_line);
> 
>     return 0;
> 
> Should I plumb nr_events down versus assuming its 1?

better to plumb it properly, i.e. at some point where you have access to
evlist to get its ->nr_entries so that we properly support multiple
events.

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-19 22:12       ` David Ahern
@ 2011-10-20 13:00         ` Arnaldo Carvalho de Melo
  2011-10-20 14:15           ` David Ahern
  2011-11-10 22:01           ` Brian Marete
  0 siblings, 2 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-20 13:00 UTC (permalink / raw)
  To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec

Em Wed, Oct 19, 2011 at 04:12:32PM -0600, David Ahern escreveu:
> 
> 
> Getting another crash with the default sym sorting -- the addr is much
> less than the start of the sym so the offset goes negative:
> 
> (gdb) bt
> #0  0x0000000000429de3 in symbol__inc_addr_samples (sym=0x8f0f90,
> map=0x8aae00, evidx=0,
>     addr=329985) at util/annotate.c:73
> #1  0x000000000041b073 in record_precise_ip (he=0x8a2a10, counter=0,
> ip=329985)
>     at builtin-top.c:221
> #2  0x000000000041c821 in perf_event__process_sample (event=0x7fffefbc74c8,
>     sample=0x7fffffffe1b0, session=0x89a140) at builtin-top.c:801
> #3  0x000000000041c8d4 in perf_session__mmap_read_idx (self=0x89a140,
> idx=12)
>     at builtin-top.c:821
> #4  0x000000000041c95b in perf_session__mmap_read (self=0x89a140) at
> builtin-top.c:832
> #5  0x000000000041cdf1 in __cmd_top () at builtin-top.c:960
> #6  0x000000000041d585 in cmd_top (argc=0, argv=0x7fffffffe590, prefix=0x0)
>     at builtin-top.c:1252
> #7  0x00000000004077b9 in run_builtin (p=0x75fb68, argc=2,
> argv=0x7fffffffe590) at perf.c:286
> #8  0x00000000004079bb in handle_internal_command (argc=2,
> argv=0x7fffffffe590) at perf.c:358
> #9  0x0000000000407b07 in run_argv (argcp=0x7fffffffe47c,
> argv=0x7fffffffe470) at perf.c:402
> #10 0x0000000000407dee in main (argc=2, argv=0x7fffffffe590) at perf.c:512
> 
> 
> The following fixes the crash. If it seems reasonable I'll add to the
> other one:
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bc8f477..f1f20b5 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -63,6 +63,8 @@ int symbol__inc_addr_samples(struct symbol *sym,
> struct map *map,
>         return -ENOMEM;
> 
>     pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map->unmap_ip(map,
> addr));
> +   if (addr < sym->start)
> +       return 0;

But this one seems like papering over some real problem, i.e. why would
we ask to bump an address that is _outside_ this symbol range? I.e.:

In record_precise_ip() this part must be wrong:

        ip = he->ms.map->map_ip(he->ms.map, ip);
        symbol__inc_addr_samples(sym, he->ms.map, counter, ip)

I.e. the map_ip for this method is messing up things, what symbol is
this? I.e. please provide:

p *sym
p *map

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-20 13:00         ` Arnaldo Carvalho de Melo
@ 2011-10-20 14:15           ` David Ahern
  2011-11-10 22:01           ` Brian Marete
  1 sibling, 0 replies; 26+ messages in thread
From: David Ahern @ 2011-10-20 14:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: linux-kernel, mingo, peterz, fweisbec



On 10/20/2011 07:00 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 19, 2011 at 04:12:32PM -0600, David Ahern escreveu:
> 
> But this one seems like papering over some real problem, i.e. why would
> we ask to bump an address that is _outside_ this symbol range? I.e.:
> 
> In record_precise_ip() this part must be wrong:
> 
>         ip = he->ms.map->map_ip(he->ms.map, ip);
>         symbol__inc_addr_samples(sym, he->ms.map, counter, ip)
> 
> I.e. the map_ip for this method is messing up things, what symbol is
> this? I.e. please provide:
> 
> p *sym
> p *map
> 
> - Arnaldo

recreated. new backtrace:
(gdb) bt
#0  0x0000000000429de3 in symbol__inc_addr_samples (sym=0xd744d0,
map=0x11d0650, evidx=0,
    addr=140081) at util/annotate.c:73
#1  0x000000000041b073 in record_precise_ip (he=0x126cd10, counter=0,
ip=140081)
    at builtin-top.c:221
#2  0x000000000041c821 in perf_event__process_sample (event=0x7ffff7e67510,
    sample=0x7fffffffa0e0, session=0x8774a0) at builtin-top.c:801
#3  0x000000000041c8d4 in perf_session__mmap_read_idx (self=0x8774a0,
idx=2) at builtin-top.c:821
#4  0x000000000041c95b in perf_session__mmap_read (self=0x8774a0) at
builtin-top.c:832
#5  0x000000000041ce94 in __cmd_top () at builtin-top.c:981
#6  0x000000000041d585 in cmd_top (argc=0, argv=0x7fffffffa4c0,
prefix=0x0) at builtin-top.c:1252
#7  0x00000000004077b9 in run_builtin (p=0x75fb68, argc=2,
argv=0x7fffffffa4c0) at perf.c:286
#8  0x00000000004079bb in handle_internal_command (argc=2,
argv=0x7fffffffa4c0) at perf.c:358
#9  0x0000000000407b07 in run_argv (argcp=0x7fffffffa3ac,
argv=0x7fffffffa3a0) at perf.c:402
#10 0x0000000000407dee in main (argc=2, argv=0x7fffffffa4c0) at perf.c:512

(gdb) fr 1
(gdb) p *sym
$2 = {
  rb_node = {
    rb_parent_color = 14360401,
    rb_right = 0x0,
    rb_left = 0x0
  },
  start = 484096,
  end = 484282,
  namelen = 13,
  binding = 0 '\000',
  ignore = false,
  name = 0xd744d0 "Q\037", <incomplete sequence \333>
}

(gdb) p *he
$5 = {
  rb_node_in = {
    rb_parent_color = 19381681,
    rb_right = 0x13396c0,
    rb_left = 0x1287cb0
  },
  rb_node = {
    rb_parent_color = 0,
    rb_right = 0x0,
    rb_left = 0x0
  },
  period = 7072393,
  period_sys = 0,
  period_us = 7072393,
  period_guest_sys = 0,
  period_guest_us = 0,
  ms = {
    map = 0x11d0650,
    sym = 0xd744d0,
    unfolded = false,
    has_children = false
  },
  thread = 0x1071b00,
  ip = 484204,
  cpu = -1,
  nr_events = 7,
  row_offset = 0,
  nr_rows = 0,
  init_have_children = false,
  level = 46 '.',
  used = false,
  filtered = 0 '\000',
  parent = 0x0,
  {
    position = 0,
    pair = 0x0,
    sorted_chain = {
      rb_node = 0x0
    }
  },
  callchain = 0x126cd10
}

(gdb) p {struct map} 0x11d0650
$7 = {
  {
    rb_node = {
      rb_parent_color = 1,
      rb_right = 0x11d0320,
      rb_left = 0x11cfc00
    },
    node = {
      next = 0x1,
      prev = 0x11d0320
    }
  },
  start = 4150226944,
  end = 4151816191,
  type = 0 '\000',
  referenced = true,
  priv = 0,
  pgoff = 0,
  map_ip = 0x4506ed <map__map_ip>,
  unmap_ip = 0x45073d <map__unmap_ip>,
  dso = 0x88f820,
  groups = 0x1071b18
}


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-20 13:00         ` Arnaldo Carvalho de Melo
  2011-10-20 14:15           ` David Ahern
@ 2011-11-10 22:01           ` Brian Marete
  2011-11-13 13:43             ` Arnaldo Carvalho de Melo
  2011-11-30 13:23             ` Brian Marete
  1 sibling, 2 replies; 26+ messages in thread
From: Brian Marete @ 2011-11-10 22:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

Hello,

On Thu, Oct 20, 2011 at 4:00 PM, Arnaldo Carvalho de Melo
<acme@ghostprotocols.net> wrote:
> I.e. the map_ip for this method is messing up things, what symbol is
> this? I.e. please provide:
>
> p *sym
> p *map

I have am experiencing the same segfault using perf from the latest
linus' tree. The gdb backtrace is below. Which patch fixes it? Or is
it already fixed in some git tree on kernel.org?

Thanks

Program received signal SIGSEGV, Segmentation fault.
0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=0x9677a0,
    evidx=0, addr=256544) at util/annotate.c:73
73		h->addr[offset]++;
#0  0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=
    0x9677a0, evidx=0, addr=256544) at util/annotate.c:73
#1  0x00000000004215e4 in record_precise_ip (he=0x1e7b260, counter=0,
    ip=256544) at builtin-top.c:223
#2  0x0000000000422ba1 in perf_event__process_sample (event=0x7ffff7ec4c60,
    evsel=0x837a10, sample=0x7fffffffe340, session=0x837e80)
    at builtin-top.c:801
#3  0x0000000000422c8c in perf_session__mmap_read_idx (self=0x837e80, idx=1)
    at builtin-top.c:825
#4  0x0000000000422d43 in perf_session__mmap_read (self=0x837e80)
    at builtin-top.c:839
#5  0x0000000000423295 in __cmd_top () at builtin-top.c:1003
#6  0x0000000000423940 in cmd_top (argc=0, argv=0x7fffffffe720, prefix=0x0)
    at builtin-top.c:1274
#7  0x000000000040ffdc in run_builtin (p=0x6a4bc8, argc=1, argv=0x7fffffffe720)
    at perf.c:286
#8  0x00000000004101af in handle_internal_command (argc=1, argv=0x7fffffffe720)
    at perf.c:358
#9  0x00000000004102b5 in run_argv (argcp=0x7fffffffe60c, argv=0x7fffffffe600)
    at perf.c:402
#10 0x00000000004104f8 in main (argc=1, argv=0x7fffffffe720) at perf.c:512

# Output of p *sym
$1 = {rb_node = {rb_parent_color = 19423281, rb_right = 0x0, rb_left = 0x0},
  start = 1124896, end = 1126163, namelen = 16, binding = 0 '\000',
  ignore = false, name = 0x1279e70 "1`(\001"}

# Output of p *map
$2 = {{rb_node = {rb_parent_color = 9861408, rb_right = 0x967860, rb_left =
    0x9676e0}, node = {next = 0x967920, prev = 0x967860}}, start = 4142972928,
  end = 4144365568, type = 0 '\000', referenced = true, priv = 0, pgoff = 0,
  map_ip = 0x44fc69 <map__map_ip>, unmap_ip = 0x44fc92 <map__unmap_ip>, dso =
    0x8e5490, groups = 0x964cd8}

--
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-11-10 22:01           ` Brian Marete
@ 2011-11-13 13:43             ` Arnaldo Carvalho de Melo
  2011-11-13 21:03               ` Brian Marete
  2011-11-30 13:23             ` Brian Marete
  1 sibling, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-11-13 13:43 UTC (permalink / raw)
  To: Brian Marete; +Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

Em Fri, Nov 11, 2011 at 01:01:47AM +0300, Brian Marete escreveu:
> On Thu, Oct 20, 2011 at 4:00 PM, Arnaldo Carvalho de Melo
> <acme@ghostprotocols.net> wrote:
> > I.e. the map_ip for this method is messing up things, what symbol is
> > this? I.e. please provide:

> > p *sym
> > p *map

> I have am experiencing the same segfault using perf from the latest
> linus' tree. The gdb backtrace is below. Which patch fixes it? Or is
> it already fixed in some git tree on kernel.org?

Can you perform this command?

[acme@felicio linux]$ git log | head -1
commit 3439a8da16bcad6b0982ece938c9f8299bb53584

 
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=0x9677a0,
>     evidx=0, addr=256544) at util/annotate.c:73
> 73		h->addr[offset]++;

> # Output of p *sym
> $1 = {rb_node = {rb_parent_color = 19423281, rb_right = 0x0, rb_left = 0x0},
>   start = 1124896, end = 1126163, namelen = 16, binding = 0 '\000',
>   ignore = false, name = 0x1279e70 "1`(\001"}

It is going well before the start of this symbol:

addr(256544) < sym->start(1124896)
 
- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-11-13 13:43             ` Arnaldo Carvalho de Melo
@ 2011-11-13 21:03               ` Brian Marete
  2011-11-13 21:42                 ` Brian Marete
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Marete @ 2011-11-13 21:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

On Sun, Nov 13, 2011 at 4:43 PM, Arnaldo Carvalho de Melo
<acme@ghostprotocols.net> wrote:
> Em Fri, Nov 11, 2011 at 01:01:47AM +0300, Brian Marete escreveu:
>> On Thu, Oct 20, 2011 at 4:00 PM, Arnaldo Carvalho de Melo
>> <acme@ghostprotocols.net> wrote:
>> > I.e. the map_ip for this method is messing up things, what symbol is
>> > this? I.e. please provide:
>
>> > p *sym
>> > p *map
>
>> I have am experiencing the same segfault using perf from the latest
>> linus' tree. The gdb backtrace is below. Which patch fixes it? Or is
>> it already fixed in some git tree on kernel.org?
>
> Can you perform this command?
>
> [acme@felicio linux]$ git log | head -1
> commit 3439a8da16bcad6b0982ece938c9f8299bb53584

I can answer this straight away: Yes. This is the same HEAD I have.

>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=0x9677a0,
>>     evidx=0, addr=256544) at util/annotate.c:73
>> 73            h->addr[offset]++;
>
>> # Output of p *sym
>> $1 = {rb_node = {rb_parent_color = 19423281, rb_right = 0x0, rb_left = 0x0},
>>   start = 1124896, end = 1126163, namelen = 16, binding = 0 '\000',
>>   ignore = false, name = 0x1279e70 "1`(\001"}
>
> It is going well before the start of this symbol:
>
> addr(256544) < sym->start(1124896)
>

I will reproduce (which I am always able to do) run another bt and report back.

-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-11-13 21:03               ` Brian Marete
@ 2011-11-13 21:42                 ` Brian Marete
  0 siblings, 0 replies; 26+ messages in thread
From: Brian Marete @ 2011-11-13 21:42 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

On Mon, Nov 14, 2011 at 12:03 AM, Brian Marete <marete@toshnix.com> wrote:
> On Sun, Nov 13, 2011 at 4:43 PM, Arnaldo Carvalho de Melo
>> It is going well before the start of this symbol:
>>
>> addr(256544) < sym->start(1124896)
>>
>
> I will reproduce (which I am always able to do) run another bt and report back.
>

Sorry, had misread your comment above as a request for information.

And yes, I can reproduce this on the HEAD you have printed above with git-log.

Let me know if you need further information.

-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-11-10 22:01           ` Brian Marete
  2011-11-13 13:43             ` Arnaldo Carvalho de Melo
@ 2011-11-30 13:23             ` Brian Marete
  2011-11-30 18:10               ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 26+ messages in thread
From: Brian Marete @ 2011-11-30 13:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

On Fri, Nov 11, 2011 at 1:01 AM, Brian Marete <marete@toshnix.com> wrote:
>
> Hello,
>
> On Thu, Oct 20, 2011 at 4:00 PM, Arnaldo Carvalho de Melo
> <acme@ghostprotocols.net> wrote:
> > I.e. the map_ip for this method is messing up things, what symbol is
> > this? I.e. please provide:
> >
> > p *sym
> > p *map
>
> I have am experiencing the same segfault using perf from the latest
> linus' tree. The gdb backtrace is below. Which patch fixes it? Or is
> it already fixed in some git tree on kernel.org?
>
> Thanks
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=0x9677a0,
>    evidx=0, addr=256544) at util/annotate.c:73
> 73              h->addr[offset]++;
> #0  0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=
>    0x9677a0, evidx=0, addr=256544) at util/annotate.c:73
> #1  0x00000000004215e4 in record_precise_ip (he=0x1e7b260, counter=0,
>    ip=256544) at builtin-top.c:223
> #2  0x0000000000422ba1 in perf_event__process_sample (event=0x7ffff7ec4c60,
>    evsel=0x837a10, sample=0x7fffffffe340, session=0x837e80)
>    at builtin-top.c:801
> #3  0x0000000000422c8c in perf_session__mmap_read_idx (self=0x837e80, idx=1)
>    at builtin-top.c:825
> #4  0x0000000000422d43 in perf_session__mmap_read (self=0x837e80)
>    at builtin-top.c:839
> #5  0x0000000000423295 in __cmd_top () at builtin-top.c:1003
> #6  0x0000000000423940 in cmd_top (argc=0, argv=0x7fffffffe720, prefix=0x0)
>    at builtin-top.c:1274
> #7  0x000000000040ffdc in run_builtin (p=0x6a4bc8, argc=1, argv=0x7fffffffe720)
>    at perf.c:286
> #8  0x00000000004101af in handle_internal_command (argc=1, argv=0x7fffffffe720)
>    at perf.c:358
> #9  0x00000000004102b5 in run_argv (argcp=0x7fffffffe60c, argv=0x7fffffffe600)
>    at perf.c:402
> #10 0x00000000004104f8 in main (argc=1, argv=0x7fffffffe720) at perf.c:512
>
> # Output of p *sym
> $1 = {rb_node = {rb_parent_color = 19423281, rb_right = 0x0, rb_left = 0x0},
>  start = 1124896, end = 1126163, namelen = 16, binding = 0 '\000',
>  ignore = false, name = 0x1279e70 "1`(\001"}
>
> # Output of p *map
> $2 = {{rb_node = {rb_parent_color = 9861408, rb_right = 0x967860, rb_left =
>    0x9676e0}, node = {next = 0x967920, prev = 0x967860}}, start = 4142972928,
>  end = 4144365568, type = 0 '\000', referenced = true, priv = 0, pgoff = 0,
>  map_ip = 0x44fc69 <map__map_ip>, unmap_ip = 0x44fc92 <map__unmap_ip>, dso =
>    0x8e5490, groups = 0x964cd8}

Hello. I can still reproduce this every time with the tip Linus' tree.
Is there a tree that fixes this? I beginning to wonder if anyone uses
`perf top' :)

Thanks.

--
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-11-30 13:23             ` Brian Marete
@ 2011-11-30 18:10               ` Arnaldo Carvalho de Melo
  2011-12-01 13:17                 ` Brian Marete
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-11-30 18:10 UTC (permalink / raw)
  To: Brian Marete; +Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

Em Wed, Nov 30, 2011 at 04:23:17PM +0300, Brian Marete escreveu:
> On Fri, Nov 11, 2011 at 1:01 AM, Brian Marete <marete@toshnix.com> wrote:
> >
> > Hello,
> >
> > On Thu, Oct 20, 2011 at 4:00 PM, Arnaldo Carvalho de Melo
> > <acme@ghostprotocols.net> wrote:
> > > I.e. the map_ip for this method is messing up things, what symbol is
> > > this? I.e. please provide:
> > >
> > > p *sym
> > > p *map
> >
> > I have am experiencing the same segfault using perf from the latest
> > linus' tree. The gdb backtrace is below. Which patch fixes it? Or is
> > it already fixed in some git tree on kernel.org?
> >
> > Thanks
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=0x9677a0,
> >    evidx=0, addr=256544) at util/annotate.c:73
> > 73              h->addr[offset]++;
> > #0  0x000000000042e27e in symbol__inc_addr_samples (sym=0x1279e70, map=
> >    0x9677a0, evidx=0, addr=256544) at util/annotate.c:73
> > #1  0x00000000004215e4 in record_precise_ip (he=0x1e7b260, counter=0,
> >    ip=256544) at builtin-top.c:223
> > #2  0x0000000000422ba1 in perf_event__process_sample (event=0x7ffff7ec4c60,
> >    evsel=0x837a10, sample=0x7fffffffe340, session=0x837e80)
> >    at builtin-top.c:801
> > #3  0x0000000000422c8c in perf_session__mmap_read_idx (self=0x837e80, idx=1)
> >    at builtin-top.c:825
> > #4  0x0000000000422d43 in perf_session__mmap_read (self=0x837e80)
> >    at builtin-top.c:839
> > #5  0x0000000000423295 in __cmd_top () at builtin-top.c:1003
> > #6  0x0000000000423940 in cmd_top (argc=0, argv=0x7fffffffe720, prefix=0x0)
> >    at builtin-top.c:1274
> > #7  0x000000000040ffdc in run_builtin (p=0x6a4bc8, argc=1, argv=0x7fffffffe720)
> >    at perf.c:286
> > #8  0x00000000004101af in handle_internal_command (argc=1, argv=0x7fffffffe720)
> >    at perf.c:358
> > #9  0x00000000004102b5 in run_argv (argcp=0x7fffffffe60c, argv=0x7fffffffe600)
> >    at perf.c:402
> > #10 0x00000000004104f8 in main (argc=1, argv=0x7fffffffe720) at perf.c:512
> >
> > # Output of p *sym
> > $1 = {rb_node = {rb_parent_color = 19423281, rb_right = 0x0, rb_left = 0x0},
> >  start = 1124896, end = 1126163, namelen = 16, binding = 0 '\000',
> >  ignore = false, name = 0x1279e70 "1`(\001"}
> >
> > # Output of p *map
> > $2 = {{rb_node = {rb_parent_color = 9861408, rb_right = 0x967860, rb_left =
> >    0x9676e0}, node = {next = 0x967920, prev = 0x967860}}, start = 4142972928,
> >  end = 4144365568, type = 0 '\000', referenced = true, priv = 0, pgoff = 0,
> >  map_ip = 0x44fc69 <map__map_ip>, unmap_ip = 0x44fc92 <map__unmap_ip>, dso =
> >    0x8e5490, groups = 0x964cd8}
> 
> Hello. I can still reproduce this every time with the tip Linus' tree.
> Is there a tree that fixes this? I beginning to wonder if anyone uses
> `perf top' :)

Can you try with:

https://github.com/acmel/linux/tree/perf/core

It just fell thru the cracks, sorry, I should have tried to reproduce
and fix this on the tree you reported.

But if you can try with the above tree...

Persistence is key, thanks for using it :)

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-11-30 18:10               ` Arnaldo Carvalho de Melo
@ 2011-12-01 13:17                 ` Brian Marete
  2011-12-01 14:11                   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Marete @ 2011-12-01 13:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

On Wed, Nov 30, 2011 at 9:10 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> https://github.com/acmel/linux/tree/perf/core
>
> It just fell thru the cracks, sorry, I should have tried to reproduce
> and fix this on the tree you reported.
>
> But if you can try with the above tree...
>
> Persistence is key, thanks for using it :)
>

Hello Arnaldo, Thanks for your reply. The perf/core branch of the tree
you suggest above gives me the same crash (with what looks to me as
the same b/trace). Would you like the details of the b/trace? The
master branch of the said tree appears too old to me (so of course it
does not suffer the crash). Which other branch of the tree should I
try?

Thanks!

-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-12-01 13:17                 ` Brian Marete
@ 2011-12-01 14:11                   ` Arnaldo Carvalho de Melo
  2011-12-06  7:22                     ` Brian Gitonga Marete
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-12-01 14:11 UTC (permalink / raw)
  To: Brian Marete; +Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

Em Thu, Dec 01, 2011 at 04:17:49PM +0300, Brian Marete escreveu:
> On Wed, Nov 30, 2011 at 9:10 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > https://github.com/acmel/linux/tree/perf/core

> > It just fell thru the cracks, sorry, I should have tried to reproduce
> > and fix this on the tree you reported.

> > But if you can try with the above tree...

> > Persistence is key, thanks for using it :)

> Hello Arnaldo, Thanks for your reply. The perf/core branch of the tree
> you suggest above gives me the same crash (with what looks to me as
> the same b/trace). Would you like the details of the b/trace? The
> master branch of the said tree appears too old to me (so of course it
> does not suffer the crash). Which other branch of the tree should I
> try?

Its just this one, can you try doing something like:

perf top -vvv > /tmp/debug.top 2>&1
bzip2 /tmp/debug.top

That will print lots of information about symbols being loaded,
overlapping maps, etc that can help me understand why that symbol name
looks garbage.

And send me in pvt the result together with a fresh backtrace with those
symbols printed? Also which distro are you using?

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-12-01 14:11                   ` Arnaldo Carvalho de Melo
@ 2011-12-06  7:22                     ` Brian Gitonga Marete
  2011-12-06 13:44                       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Gitonga Marete @ 2011-12-06  7:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

On Thu, Dec 1, 2011 at 5:11 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
>
> Its just this one, can you try doing something like:
>
> perf top -vvv > /tmp/debug.top 2>&1
> bzip2 /tmp/debug.top
>
> That will print lots of information about symbols being loaded,
> overlapping maps, etc that can help me understand why that symbol name
> looks garbage.
>
> And send me in pvt the result together with a fresh backtrace with those
> symbols printed? Also which distro are you using?
>

Hello Arnaldo. Curiously, I am unable to reproduce the crash when
stdout is redirected to a file. I am able to capture lots of output if
I redirect stdout to a file in verbose mode, but it won't crash when I
do that. Just the same, I am still able to always reproduce (within a
few seconds) with your perf/core branch on the tree you pointed me to.
This time, I did a `thread apply all bt' in gdb to show the trace for
the UI thread since it now seems to me to probably have something with
the UI output (because I cannot reproduce if o/put is to a file). The
trace is attached in the file gdb.txt.

Is the `perf top -vvv > /tmp/debug.top 2>&1' output useful even if it
is captured in a session that does not crash? If so will send the
output privately to you.

I am using Ubuntu 10.10

Thanks!

-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

[-- Attachment #2: gdb.txt --]
[-- Type: text/plain, Size: 5355 bytes --]

Reading symbols from /home/marebri/devel/perf-core.git/tools/perf/perf...done.
Starting program: /home/marebri/devel/perf-core.git/tools/perf/perf top
[Thread debugging using libthread_db enabled]
[New Thread 0x7ffff4959700 (LWP 7780)]

Program received signal SIGSEGV, Segmentation fault.
0x000000000042fda7 in symbol__inc_addr_samples (sym=0x17e6a60, map=0x8e8b60, 
    evidx=0, addr=93553) at util/annotate.c:73
73		h->addr[offset]++;

###### Output of bt follows

#0  0x000000000042fda7 in symbol__inc_addr_samples (sym=0x17e6a60, map=
    0x8e8b60, evidx=0, addr=93553) at util/annotate.c:73
#1  0x000000000042229d in perf_top__record_precise_ip (top=0x7fffffffd5a0, he=
    0x222e8c0, counter=0, ip=93553) at builtin-top.c:188
#2  0x0000000000423a8a in perf_event__process_sample (tool=0x7fffffffd5a0, 
    event=0x7ffff7f45710, evsel=0x832e90, sample=0x7fffffffcbe0, machine=
    0x833370) at builtin-top.c:743
#3  0x0000000000423ce5 in perf_top__mmap_read_idx (top=0x7fffffffd5a0, idx=0)
    at builtin-top.c:804
#4  0x0000000000423da7 in perf_top__mmap_read (top=0x7fffffffd5a0)
    at builtin-top.c:819
#5  0x0000000000424380 in __cmd_top (top=0x7fffffffd5a0) at builtin-top.c:985
#6  0x000000000042535e in cmd_top (argc=0, argv=0x7fffffffda30, prefix=0x0)
    at builtin-top.c:1272
#7  0x00000000004102d5 in run_builtin (p=0x6a6be8, argc=1, argv=0x7fffffffda30)
    at perf.c:273
#8  0x00000000004104a8 in handle_internal_command (argc=1, argv=0x7fffffffda30)
    at perf.c:345
#9  0x00000000004105ae in run_argv (argcp=0x7fffffffd91c, argv=0x7fffffffd910)
    at perf.c:389
#10 0x00000000004107b3 in main (argc=1, argv=0x7fffffffda30) at perf.c:487

####### Output of p *sym follows

$1 = {rb_node = {rb_parent_color = 18917441, rb_right = 0x1213130, rb_left = 
    0x179d920}, start = 228720, end = 228916, namelen = 14, 
  binding = 1 '\001', ignore = false, name = 0x17e6a60 "A\250 \001"}

###### Output of p *map follows

$2 = {{rb_node = {rb_parent_color = 9342289, rb_right = 0x8e8c50, rb_left = 
    0x8e8a70}, node = {next = 0x8e8d51, prev = 0x8e8c50}}, start = 4144279552, 
  end = 4145426432, type = 0 '\000', referenced = true, priv = 0, pgoff = 0, 
  map_ip = 0x4523cd <map__map_ip>, unmap_ip = 0x4523f6 <map__unmap_ip>, dso = 
    0x8e8bc0, groups = 0x8eaf68}

###### Output of info thread follows

  2 Thread 0x7ffff4959700 (LWP 7780)  0x00007ffff65602c3 in select ()
    at ../sysdeps/unix/syscall-template.S:82
* 1 Thread 0x7ffff7fc1700 (LWP 7777)  0x000000000042fda7 in symbol__inc_addr_samples (sym=0x17e6a60, map=0x8e8b60, evidx=0, addr=93553) at util/annotate.c:73

####### Output of thread apply all bt follows

Thread 2 (Thread 0x7ffff4959700 (LWP 7780)):
#0  0x00007ffff65602c3 in select () at ../sysdeps/unix/syscall-template.S:82
#1  0x0000000000471ad1 in ui__getch (delay_secs=2) at util/ui/setup.c:62
#2  0x00000000004729ee in ui_browser__run (self=0xca23b0, delay_secs=2)
    at util/ui/browser.c:364
#3  0x0000000000474e4e in hist_browser__run (self=0xca23b0, ev_name=
    0x833140 "cycles", timer=0x42335d <perf_top__sort_new_samples>, arg=
    0x7fffffffd5a0, delay_secs=2) at util/ui/browsers/hists.c:324
#4  0x0000000000476442 in perf_evsel__hists_browse (evsel=0x832e90, 
    nr_events=1, helpline=
    0x4847c8 "For a higher level overview, try: perf top --sort comm,dso", 
    ev_name=0x833140 "cycles", left_exits=false, timer=
    0x42335d <perf_top__sort_new_samples>, arg=0x7fffffffd5a0, delay_secs=2)
    at util/ui/browsers/hists.c:883
#5  0x0000000000477317 in perf_evlist__tui_browse_hists (evlist=0x8095c0, help=
    0x4847c8 "For a higher level overview, try: perf top --sort comm,dso", 
    timer=0x42335d <perf_top__sort_new_samples>, arg=0x7fffffffd5a0, 
    delay_secs=2) at util/ui/browsers/hists.c:1254
#6  0x0000000000423470 in display_thread_tui (arg=0x7fffffffd5a0)
    at builtin-top.c:546
#7  0x00007ffff7bc6971 in start_thread (arg=<value optimized out>)
    at pthread_create.c:304
#8  0x00007ffff656792d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#9  0x0000000000000000 in ?? ()

Thread 1 (Thread 0x7ffff7fc1700 (LWP 7777)):
#0  0x000000000042fda7 in symbol__inc_addr_samples (sym=0x17e6a60, map=
    0x8e8b60, evidx=0, addr=93553) at util/annotate.c:73
#1  0x000000000042229d in perf_top__record_precise_ip (top=0x7fffffffd5a0, he=
    0x222e8c0, counter=0, ip=93553) at builtin-top.c:188
#2  0x0000000000423a8a in perf_event__process_sample (tool=0x7fffffffd5a0, 
    event=0x7ffff7f45710, evsel=0x832e90, sample=0x7fffffffcbe0, machine=
    0x833370) at builtin-top.c:743
#3  0x0000000000423ce5 in perf_top__mmap_read_idx (top=0x7fffffffd5a0, idx=0)
    at builtin-top.c:804
#4  0x0000000000423da7 in perf_top__mmap_read (top=0x7fffffffd5a0)
    at builtin-top.c:819
#5  0x0000000000424380 in __cmd_top (top=0x7fffffffd5a0) at builtin-top.c:985
#6  0x000000000042535e in cmd_top (argc=0, argv=0x7fffffffda30, prefix=0x0)
    at builtin-top.c:1272
#7  0x00000000004102d5 in run_builtin (p=0x6a6be8, argc=1, argv=0x7fffffffda30)
    at perf.c:273
#8  0x00000000004104a8 in handle_internal_command (argc=1, argv=0x7fffffffda30)
    at perf.c:345
#9  0x00000000004105ae in run_argv (argcp=0x7fffffffd91c, argv=0x7fffffffd910)
    at perf.c:389
#10 0x00000000004107b3 in main (argc=1, argv=0x7fffffffda30) at perf.c:487
Kill the program being debugged? (y or n) 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-12-06  7:22                     ` Brian Gitonga Marete
@ 2011-12-06 13:44                       ` Arnaldo Carvalho de Melo
  2011-12-15 21:01                         ` Brian Gitonga Marete
  0 siblings, 1 reply; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-12-06 13:44 UTC (permalink / raw)
  To: Brian Gitonga Marete; +Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

Em Tue, Dec 06, 2011 at 10:22:57AM +0300, Brian Gitonga Marete escreveu:
> On Thu, Dec 1, 2011 at 5:11 PM, Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > Its just this one, can you try doing something like:

> > perf top -vvv > /tmp/debug.top 2>&1
> > bzip2 /tmp/debug.top

> > That will print lots of information about symbols being loaded,
> > overlapping maps, etc that can help me understand why that symbol name
> > looks garbage.

> > And send me in pvt the result together with a fresh backtrace with those
> > symbols printed? Also which distro are you using?

> Hello Arnaldo. Curiously, I am unable to reproduce the crash when
> stdout is redirected to a file. I am able to capture lots of output if

That is good input, so can you please try using just:

perf top --stdio

I.e. probably the problem is with the TUI code and running it in --stdio
mode will help us pinpoint that. No need to redirect anything.

> I redirect stdout to a file in verbose mode, but it won't crash when I
> do that. Just the same, I am still able to always reproduce (within a
> few seconds) with your perf/core branch on the tree you pointed me to.
> This time, I did a `thread apply all bt' in gdb to show the trace for
> the UI thread since it now seems to me to probably have something with
> the UI output (because I cannot reproduce if o/put is to a file). The
> trace is attached in the file gdb.txt.
 
> Is the `perf top -vvv > /tmp/debug.top 2>&1' output useful even if it
> is captured in a session that does not crash? If so will send the
> output privately to you.

please.
 
> I am using Ubuntu 10.10

Ok,

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-12-06 13:44                       ` Arnaldo Carvalho de Melo
@ 2011-12-15 21:01                         ` Brian Gitonga Marete
  2011-12-15 22:04                           ` Brian Gitonga Marete
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Gitonga Marete @ 2011-12-15 21:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

On Tue, Dec 6, 2011 at 4:44 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
>
> Em Tue, Dec 06, 2011 at 10:22:57AM +0300, Brian Gitonga Marete escreveu:
> > On Thu, Dec 1, 2011 at 5:11 PM, Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > > Its just this one, can you try doing something like:
>
> > > perf top -vvv > /tmp/debug.top 2>&1
> > > bzip2 /tmp/debug.top
>
> > > That will print lots of information about symbols being loaded,
> > > overlapping maps, etc that can help me understand why that symbol name
> > > looks garbage.
>
> > > And send me in pvt the result together with a fresh backtrace with those
> > > symbols printed? Also which distro are you using?
>
> > Hello Arnaldo. Curiously, I am unable to reproduce the crash when
> > stdout is redirected to a file. I am able to capture lots of output if
>
> That is good input, so can you please try using just:
>
> perf top --stdio
>
> I.e. probably the problem is with the TUI code and running it in --stdio
> mode will help us pinpoint that. No need to redirect anything.
>

Indeed `--stdio' avoids the crash. See my bisection report below.

> > I redirect stdout to a file in verbose mode, but it won't crash when I
> > do that. Just the same, I am still able to always reproduce (within a
> > few seconds) with your perf/core branch on the tree you pointed me to.
> > This time, I did a `thread apply all bt' in gdb to show the trace for
> > the UI thread since it now seems to me to probably have something with
> > the UI output (because I cannot reproduce if o/put is to a file). The
> > trace is attached in the file gdb.txt.
>
> > Is the `perf top -vvv > /tmp/debug.top 2>&1' output useful even if it
> > is captured in a session that does not crash? If so will send the
> > output privately to you.
>
> please.

Coming in a separate email to you.

> > I am using Ubuntu 10.10
>

Hi. I have bisected this on Linus' tree and the commit that introduced
the crash is: 8b1bfdbdb30

commit 8b1bfdbdb3041c0503c42ef49bab25caabeaa558
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Oct 5 19:41:31 2011 -0300

perf top: Use the TUI interface by default

It has to do with UI. Reverting it eliminates the crash. (As does
using --stdio as I have said above)

BGM.

--
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-12-15 21:01                         ` Brian Gitonga Marete
@ 2011-12-15 22:04                           ` Brian Gitonga Marete
  2011-12-16 23:46                             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Gitonga Marete @ 2011-12-15 22:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

On Fri, Dec 16, 2011 at 12:01 AM, Brian Gitonga Marete
<marete@toshnix.com> wrote:
> On Tue, Dec 6, 2011 at 4:44 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
>>
>> Em Tue, Dec 06, 2011 at 10:22:57AM +0300, Brian Gitonga Marete escreveu:
>> > On Thu, Dec 1, 2011 at 5:11 PM, Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
>> > > Its just this one, can you try doing something like:
>>
>> > > perf top -vvv > /tmp/debug.top 2>&1
>> > > bzip2 /tmp/debug.top
>>
>> > > That will print lots of information about symbols being loaded,
>> > > overlapping maps, etc that can help me understand why that symbol name
>> > > looks garbage.
>>
>> > > And send me in pvt the result together with a fresh backtrace with those
>> > > symbols printed? Also which distro are you using?
>>
>> > Hello Arnaldo. Curiously, I am unable to reproduce the crash when
>> > stdout is redirected to a file. I am able to capture lots of output if
>>
>> That is good input, so can you please try using just:
>>
>> perf top --stdio
>>
>> I.e. probably the problem is with the TUI code and running it in --stdio
>> mode will help us pinpoint that. No need to redirect anything.
>>
>
> Indeed `--stdio' avoids the crash. See my bisection report below.
>
>> > I redirect stdout to a file in verbose mode, but it won't crash when I
>> > do that. Just the same, I am still able to always reproduce (within a
>> > few seconds) with your perf/core branch on the tree you pointed me to.
>> > This time, I did a `thread apply all bt' in gdb to show the trace for
>> > the UI thread since it now seems to me to probably have something with
>> > the UI output (because I cannot reproduce if o/put is to a file). The
>> > trace is attached in the file gdb.txt.
>>
>> > Is the `perf top -vvv > /tmp/debug.top 2>&1' output useful even if it
>> > is captured in a session that does not crash? If so will send the
>> > output privately to you.
>>
>> please.
>
> Coming in a separate email to you.
>
>> > I am using Ubuntu 10.10
>>
>
> Hi. I have bisected this on Linus' tree and the commit that introduced
> the crash is: 8b1bfdbdb30
>
> commit 8b1bfdbdb3041c0503c42ef49bab25caabeaa558
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Oct 5 19:41:31 2011 -0300
>
> perf top: Use the TUI interface by default
>
> It has to do with UI. Reverting it eliminates the crash. (As does
> using --stdio as I have said above)

Looking at the code, it seems that the connection between the TUI
interface and the core (where the crash occurs) is that the TUI
interface activates the annotation code which otherwise is not run. So
the bug is probably in the annotation code? Or in the version of the
binary (ELF) decoding libraries that are on my system -- Ubuntu 10.10

-- 
Brian Gitonga Marete
Toshnix Systems
Tel: +254722151590

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-12-15 22:04                           ` Brian Gitonga Marete
@ 2011-12-16 23:46                             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-12-16 23:46 UTC (permalink / raw)
  To: Brian Gitonga Marete; +Cc: David Ahern, linux-kernel, mingo, peterz, fweisbec

Em Fri, Dec 16, 2011 at 01:04:14AM +0300, Brian Gitonga Marete escreveu:
> Looking at the code, it seems that the connection between the TUI
> interface and the core (where the crash occurs) is that the TUI
> interface activates the annotation code which otherwise is not run. So
> the bug is probably in the annotation code? Or in the version of the
> binary (ELF) decoding libraries that are on my system -- Ubuntu 10.10

I'm travelling, so I was unable to work on this report. I should've
already, and thank you for investigating it further.

While doing presentations about perf on an Atom (32 bit) machine I
noticed crashes while collecting callchains.

Are your crashes noticed on 32-bit or 64-bit?

I'll try to get it fixed as soon as I get back home.

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-20 20:39 David Ahern
  2011-10-20 21:30 ` Arnaldo Carvalho de Melo
@ 2011-10-20 23:26 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-20 23:26 UTC (permalink / raw)
  To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec

Em Thu, Oct 20, 2011 at 02:39:28PM -0600, David Ahern escreveu:
> Command:
>   perf top -ag --sort comm,dso

Aha, so this is what leads to the crash, i.e. a non symbolic view, when
"sym" is not in --sort. I'm applying this fix instead:

diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index af12e6f..4663dcb 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -882,6 +882,13 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			 */
 			goto out_free_stack;
 		case 'a':
+			if (!browser->has_symbols) {
+				ui__warning(
+			"Annotation is only available for symbolic views, "
+			"include \"sym\" in --sort to use it.");
+				continue;
+			}
+
 			if (browser->selection == NULL ||
 			    browser->selection->sym == NULL ||
 			    browser->selection->map->dso->annotate_warned)

I had already fixed this for the menu case, where all the things that
only make sense for symbols are not present, forgot about the 'a'
hotkey.

Thanks for reporting and for the patch anyway!

- Arnaldo
 
> crashes with signature:
> 
>     nr_events=1, timer=0x417220 <perf_top__sort_new_samples>, arg=0x751540, delay_secs=2)
>     at util/ui/browsers/annotate.c:405
>     evidx=<value optimized out>, nr_events=<value optimized out>, timer=<value optimized out>,
>     arg=<value optimized out>, delay_secs=<value optimized out>)
>     at util/ui/browsers/annotate.c:373
>     helpline=<value optimized out>, ev_name=0x860b60 "cycles", left_exits=false,
>     timer=0x417220 <perf_top__sort_new_samples>, arg=0x751540, delay_secs=2)
>     at util/ui/browsers/hists.c:997
>     help=0x502b30 "For a higher level overview, try: perf top --sort comm,dso",
>     timer=0x417220 <perf_top__sort_new_samples>, arg=0x751540, delay_secs=2)
>     at util/ui/browsers/hists.c:1204
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  tools/perf/builtin-annotate.c          |    7 ++++---
>  tools/perf/builtin-top.c               |    2 +-
>  tools/perf/util/annotate.c             |   24 +++++++++++++++++++-----
>  tools/perf/util/annotate.h             |    5 +++--
>  tools/perf/util/hist.c                 |    4 ++--
>  tools/perf/util/hist.h                 |    3 ++-
>  tools/perf/util/ui/browsers/annotate.c |    3 ++-
>  7 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 3ea764a..2c8362a 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -108,10 +108,11 @@ static int process_sample_event(union perf_event *event,
>  	return 0;
>  }
>  
> -static int hist_entry__tty_annotate(struct hist_entry *he, int evidx)
> +static int hist_entry__tty_annotate(struct hist_entry *he, int evidx,
> +				    int nr_events)
>  {
>  	return symbol__tty_annotate(he->ms.sym, he->ms.map, evidx,
> -				    print_line, full_paths, 0, 0);
> +				    print_line, full_paths, 0, 0, nr_events);
>  }
>  
>  static void hists__find_annotations(struct hists *self, int evidx,
> @@ -154,7 +155,7 @@ find_next:
>  			if (next != NULL)
>  				nd = next;
>  		} else {
> -			hist_entry__tty_annotate(he, evidx);
> +			hist_entry__tty_annotate(he, evidx, nr_events);
>  			nd = rb_next(nd);
>  			/*
>  			 * Since we have a hist_entry per IP for the same
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 7a87171..b6e8353 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -177,7 +177,7 @@ static int parse_source(struct hist_entry *he)
>  		return err;
>  	}
>  
> -	err = symbol__annotate(sym, map, 0);
> +	err = symbol__annotate(sym, map, 0, top.evlist->nr_entries);
>  	if (err == 0) {
>  out_assign:
>  		top.sym_filter_entry = he;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bc8f477..c9090d8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -197,7 +197,8 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
>  }
>  
>  static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
> -				      FILE *file, size_t privsize)
> +				      FILE *file, size_t privsize,
> +				      int nr_events)
>  {
>  	struct annotation *notes = symbol__annotation(sym);
>  	struct objdump_line *objdump_line;
> @@ -253,12 +254,24 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
>  		free(line);
>  		return -1;
>  	}
> +
> +	pthread_mutex_lock(&notes->lock);
> +	if (notes->src == NULL &&
> +		symbol__alloc_hist(sym, nr_events) < 0) {
> +		pthread_mutex_unlock(&notes->lock);
> +		ui__warning("Not enough memory for annotating '%s' symbol!\n",
> +			    sym->name);
> +		return -1;
> +	}
> +	pthread_mutex_unlock(&notes->lock);
> +
>  	objdump__add_line(&notes->src->source, objdump_line);
>  
>  	return 0;
>  }
>  
> -int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
> +int symbol__annotate(struct symbol *sym, struct map *map,
> +		     size_t privsize, int nr_events)
>  {
>  	struct dso *dso = map->dso;
>  	char *filename = dso__build_id_filename(dso, NULL, 0);
> @@ -343,7 +356,8 @@ fallback:
>  		goto out_free_filename;
>  
>  	while (!feof(file))
> -		if (symbol__parse_objdump_line(sym, map, file, privsize) < 0)
> +		if (symbol__parse_objdump_line(sym, map, file,
> +					       privsize, nr_events) < 0)
>  			break;
>  
>  	pclose(file);
> @@ -583,14 +597,14 @@ void objdump_line_list__purge(struct list_head *head)
>  
>  int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
>  			 bool print_lines, bool full_paths, int min_pcnt,
> -			 int max_lines)
> +			 int max_lines, int nr_events)
>  {
>  	struct dso *dso = map->dso;
>  	const char *filename = dso->long_name;
>  	struct rb_root source_line = RB_ROOT;
>  	u64 len;
>  
> -	if (symbol__annotate(sym, map, 0) < 0)
> +	if (symbol__annotate(sym, map, 0, nr_events) < 0)
>  		return -1;
>  
>  	len = sym->end - sym->start;
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index d907252..1a7842d 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -75,7 +75,8 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
>  int symbol__alloc_hist(struct symbol *sym, int nevents);
>  void symbol__annotate_zero_histograms(struct symbol *sym);
>  
> -int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
> +int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize,
> +		     int nr_events);
>  int symbol__annotate_init(struct map *map __used, struct symbol *sym);
>  int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
>  			    bool full_paths, int min_pcnt, int max_lines,
> @@ -86,7 +87,7 @@ void objdump_line_list__purge(struct list_head *head);
>  
>  int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
>  			 bool print_lines, bool full_paths, int min_pcnt,
> -			 int max_lines);
> +			 int max_lines, int nr_events);
>  
>  #ifdef NO_NEWT_SUPPORT
>  static inline int symbol__tui_annotate(struct symbol *sym __used,
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index f6a9939..0ebfa3e 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1180,9 +1180,9 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
>  	return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip);
>  }
>  
> -int hist_entry__annotate(struct hist_entry *he, size_t privsize)
> +int hist_entry__annotate(struct hist_entry *he, size_t privsize, int nr_events)
>  {
> -	return symbol__annotate(he->ms.sym, he->ms.map, privsize);
> +	return symbol__annotate(he->ms.sym, he->ms.map, privsize, nr_events);
>  }
>  
>  void hists__inc_nr_events(struct hists *hists, u32 type)
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 575bcbc..99ead17 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -94,7 +94,8 @@ size_t hists__fprintf(struct hists *self, struct hists *pair,
>  		      int max_rows, int max_cols, FILE *fp);
>  
>  int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
> -int hist_entry__annotate(struct hist_entry *self, size_t privsize);
> +int hist_entry__annotate(struct hist_entry *self, size_t privsize,
> +			 int nr_events);
>  
>  void hists__filter_by_dso(struct hists *hists);
>  void hists__filter_by_thread(struct hists *hists);
> diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
> index 1a12d8f..4eb1faf 100644
> --- a/tools/perf/util/ui/browsers/annotate.c
> +++ b/tools/perf/util/ui/browsers/annotate.c
> @@ -402,7 +402,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
>  	if (map->dso->annotate_warned)
>  		return -1;
>  
> -	if (symbol__annotate(sym, map, sizeof(struct objdump_line_rb_node)) < 0) {
> +	if (symbol__annotate(sym, map, sizeof(struct objdump_line_rb_node),
> +			     nr_events) < 0) {
>  		ui__error_window(ui_helpline__last_msg);
>  		return -1;
>  	}
> -- 
> 1.7.6.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH] perf top: fix crash on annotate request
  2011-10-20 20:39 David Ahern
@ 2011-10-20 21:30 ` Arnaldo Carvalho de Melo
  2011-10-20 23:26 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 26+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-20 21:30 UTC (permalink / raw)
  To: David Ahern; +Cc: linux-kernel, mingo, peterz, fweisbec

Em Thu, Oct 20, 2011 at 02:39:28PM -0600, David Ahern escreveu:
> Command:
>   perf top -ag --sort comm,dso

Thanks, will review/apply later or early tomorrow,

- Arnaldo

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] perf top: fix crash on annotate request
@ 2011-10-20 20:39 David Ahern
  2011-10-20 21:30 ` Arnaldo Carvalho de Melo
  2011-10-20 23:26 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 26+ messages in thread
From: David Ahern @ 2011-10-20 20:39 UTC (permalink / raw)
  To: acme, linux-kernel; +Cc: mingo, peterz, fweisbec, David Ahern

Command:
  perf top -ag --sort comm,dso

crashes with signature:

    nr_events=1, timer=0x417220 <perf_top__sort_new_samples>, arg=0x751540, delay_secs=2)
    at util/ui/browsers/annotate.c:405
    evidx=<value optimized out>, nr_events=<value optimized out>, timer=<value optimized out>,
    arg=<value optimized out>, delay_secs=<value optimized out>)
    at util/ui/browsers/annotate.c:373
    helpline=<value optimized out>, ev_name=0x860b60 "cycles", left_exits=false,
    timer=0x417220 <perf_top__sort_new_samples>, arg=0x751540, delay_secs=2)
    at util/ui/browsers/hists.c:997
    help=0x502b30 "For a higher level overview, try: perf top --sort comm,dso",
    timer=0x417220 <perf_top__sort_new_samples>, arg=0x751540, delay_secs=2)
    at util/ui/browsers/hists.c:1204

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/perf/builtin-annotate.c          |    7 ++++---
 tools/perf/builtin-top.c               |    2 +-
 tools/perf/util/annotate.c             |   24 +++++++++++++++++++-----
 tools/perf/util/annotate.h             |    5 +++--
 tools/perf/util/hist.c                 |    4 ++--
 tools/perf/util/hist.h                 |    3 ++-
 tools/perf/util/ui/browsers/annotate.c |    3 ++-
 7 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 3ea764a..2c8362a 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -108,10 +108,11 @@ static int process_sample_event(union perf_event *event,
 	return 0;
 }
 
-static int hist_entry__tty_annotate(struct hist_entry *he, int evidx)
+static int hist_entry__tty_annotate(struct hist_entry *he, int evidx,
+				    int nr_events)
 {
 	return symbol__tty_annotate(he->ms.sym, he->ms.map, evidx,
-				    print_line, full_paths, 0, 0);
+				    print_line, full_paths, 0, 0, nr_events);
 }
 
 static void hists__find_annotations(struct hists *self, int evidx,
@@ -154,7 +155,7 @@ find_next:
 			if (next != NULL)
 				nd = next;
 		} else {
-			hist_entry__tty_annotate(he, evidx);
+			hist_entry__tty_annotate(he, evidx, nr_events);
 			nd = rb_next(nd);
 			/*
 			 * Since we have a hist_entry per IP for the same
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7a87171..b6e8353 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -177,7 +177,7 @@ static int parse_source(struct hist_entry *he)
 		return err;
 	}
 
-	err = symbol__annotate(sym, map, 0);
+	err = symbol__annotate(sym, map, 0, top.evlist->nr_entries);
 	if (err == 0) {
 out_assign:
 		top.sym_filter_entry = he;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bc8f477..c9090d8 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -197,7 +197,8 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
 }
 
 static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
-				      FILE *file, size_t privsize)
+				      FILE *file, size_t privsize,
+				      int nr_events)
 {
 	struct annotation *notes = symbol__annotation(sym);
 	struct objdump_line *objdump_line;
@@ -253,12 +254,24 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 		free(line);
 		return -1;
 	}
+
+	pthread_mutex_lock(&notes->lock);
+	if (notes->src == NULL &&
+		symbol__alloc_hist(sym, nr_events) < 0) {
+		pthread_mutex_unlock(&notes->lock);
+		ui__warning("Not enough memory for annotating '%s' symbol!\n",
+			    sym->name);
+		return -1;
+	}
+	pthread_mutex_unlock(&notes->lock);
+
 	objdump__add_line(&notes->src->source, objdump_line);
 
 	return 0;
 }
 
-int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
+int symbol__annotate(struct symbol *sym, struct map *map,
+		     size_t privsize, int nr_events)
 {
 	struct dso *dso = map->dso;
 	char *filename = dso__build_id_filename(dso, NULL, 0);
@@ -343,7 +356,8 @@ fallback:
 		goto out_free_filename;
 
 	while (!feof(file))
-		if (symbol__parse_objdump_line(sym, map, file, privsize) < 0)
+		if (symbol__parse_objdump_line(sym, map, file,
+					       privsize, nr_events) < 0)
 			break;
 
 	pclose(file);
@@ -583,14 +597,14 @@ void objdump_line_list__purge(struct list_head *head)
 
 int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 			 bool print_lines, bool full_paths, int min_pcnt,
-			 int max_lines)
+			 int max_lines, int nr_events)
 {
 	struct dso *dso = map->dso;
 	const char *filename = dso->long_name;
 	struct rb_root source_line = RB_ROOT;
 	u64 len;
 
-	if (symbol__annotate(sym, map, 0) < 0)
+	if (symbol__annotate(sym, map, 0, nr_events) < 0)
 		return -1;
 
 	len = sym->end - sym->start;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index d907252..1a7842d 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -75,7 +75,8 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 int symbol__alloc_hist(struct symbol *sym, int nevents);
 void symbol__annotate_zero_histograms(struct symbol *sym);
 
-int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
+int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize,
+		     int nr_events);
 int symbol__annotate_init(struct map *map __used, struct symbol *sym);
 int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
 			    bool full_paths, int min_pcnt, int max_lines,
@@ -86,7 +87,7 @@ void objdump_line_list__purge(struct list_head *head);
 
 int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 			 bool print_lines, bool full_paths, int min_pcnt,
-			 int max_lines);
+			 int max_lines, int nr_events);
 
 #ifdef NO_NEWT_SUPPORT
 static inline int symbol__tui_annotate(struct symbol *sym __used,
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f6a9939..0ebfa3e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1180,9 +1180,9 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
 	return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip);
 }
 
-int hist_entry__annotate(struct hist_entry *he, size_t privsize)
+int hist_entry__annotate(struct hist_entry *he, size_t privsize, int nr_events)
 {
-	return symbol__annotate(he->ms.sym, he->ms.map, privsize);
+	return symbol__annotate(he->ms.sym, he->ms.map, privsize, nr_events);
 }
 
 void hists__inc_nr_events(struct hists *hists, u32 type)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 575bcbc..99ead17 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -94,7 +94,8 @@ size_t hists__fprintf(struct hists *self, struct hists *pair,
 		      int max_rows, int max_cols, FILE *fp);
 
 int hist_entry__inc_addr_samples(struct hist_entry *self, int evidx, u64 addr);
-int hist_entry__annotate(struct hist_entry *self, size_t privsize);
+int hist_entry__annotate(struct hist_entry *self, size_t privsize,
+			 int nr_events);
 
 void hists__filter_by_dso(struct hists *hists);
 void hists__filter_by_thread(struct hists *hists);
diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index 1a12d8f..4eb1faf 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -402,7 +402,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	if (map->dso->annotate_warned)
 		return -1;
 
-	if (symbol__annotate(sym, map, sizeof(struct objdump_line_rb_node)) < 0) {
+	if (symbol__annotate(sym, map, sizeof(struct objdump_line_rb_node),
+			     nr_events) < 0) {
 		ui__error_window(ui_helpline__last_msg);
 		return -1;
 	}
-- 
1.7.6.4


^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2011-12-16 23:47 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-19 18:23 [PATCH] perf top: fix crash on annotate request David Ahern
2011-10-19 18:38 ` Arnaldo Carvalho de Melo
2011-10-19 18:44   ` David Ahern
2011-10-19 19:20     ` Arnaldo Carvalho de Melo
2011-10-19 20:21       ` David Ahern
2011-10-19 21:39       ` David Ahern
2011-10-20 12:51         ` Arnaldo Carvalho de Melo
2011-10-19 22:12       ` David Ahern
2011-10-20 13:00         ` Arnaldo Carvalho de Melo
2011-10-20 14:15           ` David Ahern
2011-11-10 22:01           ` Brian Marete
2011-11-13 13:43             ` Arnaldo Carvalho de Melo
2011-11-13 21:03               ` Brian Marete
2011-11-13 21:42                 ` Brian Marete
2011-11-30 13:23             ` Brian Marete
2011-11-30 18:10               ` Arnaldo Carvalho de Melo
2011-12-01 13:17                 ` Brian Marete
2011-12-01 14:11                   ` Arnaldo Carvalho de Melo
2011-12-06  7:22                     ` Brian Gitonga Marete
2011-12-06 13:44                       ` Arnaldo Carvalho de Melo
2011-12-15 21:01                         ` Brian Gitonga Marete
2011-12-15 22:04                           ` Brian Gitonga Marete
2011-12-16 23:46                             ` Arnaldo Carvalho de Melo
2011-10-20 20:39 David Ahern
2011-10-20 21:30 ` Arnaldo Carvalho de Melo
2011-10-20 23:26 ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.