[PATCH 1/5] perf tools: fix incorrect ordering of callchain entries

From: Chris Phlipot
Date: Tue Apr 19 2016 - 04:57:21 EST


The existing implentation implementation of thread__resolve_callchain,
under certain circumstanes, can assemble callchain entries in the
incorrect order.

A the callchain entries are resolved incorrectly for a sample when all
of the following conditions are met:

1. callchain_param.order is set to ORDER_CALLER

2. thread__resolve_callchain_sample is able to resolve callchain entries
for the sample.

3. unwind__get_entries is also able to resolve callchain entries for the
sample.

The fix is accomplished by reversing the order in which
thread__resolve_callchain_sample and unwind__get_entries are called
when callchain_param.order is set to ORDER_CALLER.

Unwind specific code from thread__resolve_callchain is also moved into a
new static function to improve readability of the fix.

Signed-off-by: Chris Phlipot <cphlipot0@xxxxxxxxx>
---
tools/perf/util/machine.c | 57 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0c4dabc..dd086c8 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1806,8 +1806,6 @@ static int thread__resolve_callchain_sample(struct thread *thread,
int skip_idx = -1;
int first_call = 0;

- callchain_cursor_reset(cursor);
-
if (has_branch_callstack(evsel)) {
err = resolve_lbr_callchain_sample(thread, cursor, sample, parent,
root_al, max_stack);
@@ -1918,20 +1916,12 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
entry->map, entry->sym);
}

-int thread__resolve_callchain(struct thread *thread,
- struct callchain_cursor *cursor,
- struct perf_evsel *evsel,
- struct perf_sample *sample,
- struct symbol **parent,
- struct addr_location *root_al,
- int max_stack)
-{
- int ret = thread__resolve_callchain_sample(thread, cursor, evsel,
- sample, parent,
- root_al, max_stack);
- if (ret)
- return ret;
-
+static int thread__resolve_callchain_unwind(struct thread *thread,
+ struct callchain_cursor *cursor,
+ struct perf_evsel *evsel,
+ struct perf_sample *sample,
+ int max_stack)
+{
/* Can we do dwarf post unwind? */
if (!((evsel->attr.sample_type & PERF_SAMPLE_REGS_USER) &&
(evsel->attr.sample_type & PERF_SAMPLE_STACK_USER)))
@@ -1944,7 +1934,42 @@ int thread__resolve_callchain(struct thread *thread,

return unwind__get_entries(unwind_entry, cursor,
thread, sample, max_stack);
+}
+
+int thread__resolve_callchain(struct thread *thread,
+ struct callchain_cursor *cursor,
+ struct perf_evsel *evsel,
+ struct perf_sample *sample,
+ struct symbol **parent,
+ struct addr_location *root_al,
+ int max_stack)
+{
+ int ret = 0;
+ callchain_cursor_reset(&callchain_cursor);
+
+ if (callchain_param.order == ORDER_CALLEE) {
+ ret = thread__resolve_callchain_sample(thread, cursor,
+ evsel, sample,
+ parent, root_al,
+ max_stack);
+ if (ret)
+ return ret;
+ ret = thread__resolve_callchain_unwind(thread, cursor,
+ evsel, sample,
+ max_stack);
+ } else {
+ ret = thread__resolve_callchain_unwind(thread, cursor,
+ evsel, sample,
+ max_stack);
+ if (ret)
+ return ret;
+ ret = thread__resolve_callchain_sample(thread, cursor,
+ evsel, sample,
+ parent, root_al,
+ max_stack);
+ }

+ return ret;
}

int machine__for_each_thread(struct machine *machine,
--
2.7.4