* [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history
@ 2018-02-13 8:44 Jin Yao
2018-02-13 9:45 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Jin Yao @ 2018-02-13 8:44 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
Following command lines will cause perf crash.
perf record -j call -g -a <application>
perf report --branch-history
*** Error in `perf': double free or corruption (!prev): 0x00000000104aa040 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725]
/lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a]
/lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc]
perf[0x51b914]
perf(hist_entry_iter__add+0x1e5)[0x51f305]
perf[0x43cf01]
perf[0x4fa3bf]
perf[0x4fa923]
perf[0x4fd396]
perf[0x4f9614]
perf(perf_session__process_events+0x89e)[0x4fc38e]
perf(cmd_report+0x15d2)[0x43f202]
perf[0x4a059f]
perf(main+0x631)[0x427b71]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830]
perf(_start+0x29)[0x427d89]
The memory corruption happens at:
iter_add_next_cumulative_entry()
{
...
for (i = 0; i < iter->curr; i++) {
...
}
Whatever in iter_next_cumulative_entry() or in iter_add_next_cumulative_entry(),
they all don't check if iter->curr exceeds the array 'he_cache[]'.
If there are too many nodes in callchain, it's possible that iter->curr >
iter->max_stack, then memory corruption occurs.
This patch will reallocate array 'he_cache[]' in iter_next_cumulative_entry()
if necessary (the case of too many nodes in callchain).
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/util/hist.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b614095..71f07d2 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -926,11 +926,32 @@ iter_next_cumulative_entry(struct hist_entry_iter *iter,
struct addr_location *al)
{
struct callchain_cursor_node *node;
+ struct hist_entry **tmp;
+ int i;
node = callchain_cursor_current(&callchain_cursor);
if (node == NULL)
return 0;
+ /*
+ * If there are too many nodes in callchain,
+ * increase the size of he_cache[].
+ */
+ if (iter->curr == iter->max_stack) {
+ i = 2 * iter->max_stack + 1;
+ tmp = realloc(iter->priv, sizeof(struct hist_entry *) * i);
+ if (tmp == NULL) {
+ /*
+ * No need to free iter->priv here. It will be
+ * freed in iter_finish_cumulative_entry.
+ */
+ return 0;
+ }
+
+ iter->priv = tmp;
+ iter->max_stack = i;
+ }
+
return fill_callchain_info(al, node, iter->hide_unresolved);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history 2018-02-13 8:44 [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history Jin Yao @ 2018-02-13 9:45 ` Jiri Olsa 2018-02-13 14:00 ` Jin, Yao 0 siblings, 1 reply; 8+ messages in thread From: Jiri Olsa @ 2018-02-13 9:45 UTC (permalink / raw) To: Jin Yao Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Tue, Feb 13, 2018 at 04:44:28PM +0800, Jin Yao wrote: > Following command lines will cause perf crash. > > perf record -j call -g -a <application> > perf report --branch-history > > *** Error in `perf': double free or corruption (!prev): 0x00000000104aa040 *** > ======= Backtrace: ========= > /lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725] > /lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a] > /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc] > perf[0x51b914] > perf(hist_entry_iter__add+0x1e5)[0x51f305] > perf[0x43cf01] > perf[0x4fa3bf] > perf[0x4fa923] > perf[0x4fd396] > perf[0x4f9614] > perf(perf_session__process_events+0x89e)[0x4fc38e] > perf(cmd_report+0x15d2)[0x43f202] > perf[0x4a059f] > perf(main+0x631)[0x427b71] > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830] > perf(_start+0x29)[0x427d89] > > The memory corruption happens at: > > iter_add_next_cumulative_entry() > { > ... > for (i = 0; i < iter->curr; i++) { > ... > } > > Whatever in iter_next_cumulative_entry() or in iter_add_next_cumulative_entry(), > they all don't check if iter->curr exceeds the array 'he_cache[]'. > > If there are too many nodes in callchain, it's possible that iter->curr > > iter->max_stack, then memory corruption occurs. > > This patch will reallocate array 'he_cache[]' in iter_next_cumulative_entry() > if necessary (the case of too many nodes in callchain). right, the max_stack does not say how many nodes end up in callchain_cursor at the end.. good catch, please mention that also in the changelog however we know the final count from callchain_cursor itself, the attached patch might do the same job, right? also could we now get rid of iter->max_stack? thanks, jirka --- diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index b6140950301e..b50b7b70dcca 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter, * cumulated only one time to prevent entries more than 100% * overhead. */ - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); if (he_cache == NULL) return -ENOMEM; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history 2018-02-13 9:45 ` Jiri Olsa @ 2018-02-13 14:00 ` Jin, Yao 2018-02-16 2:25 ` Jin, Yao 0 siblings, 1 reply; 8+ messages in thread From: Jin, Yao @ 2018-02-13 14:00 UTC (permalink / raw) To: Jiri Olsa Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On 2/13/2018 5:45 PM, Jiri Olsa wrote: > On Tue, Feb 13, 2018 at 04:44:28PM +0800, Jin Yao wrote: >> Following command lines will cause perf crash. >> >> perf record -j call -g -a <application> >> perf report --branch-history >> >> *** Error in `perf': double free or corruption (!prev): 0x00000000104aa040 *** >> ======= Backtrace: ========= >> /lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725] >> /lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a] >> /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc] >> perf[0x51b914] >> perf(hist_entry_iter__add+0x1e5)[0x51f305] >> perf[0x43cf01] >> perf[0x4fa3bf] >> perf[0x4fa923] >> perf[0x4fd396] >> perf[0x4f9614] >> perf(perf_session__process_events+0x89e)[0x4fc38e] >> perf(cmd_report+0x15d2)[0x43f202] >> perf[0x4a059f] >> perf(main+0x631)[0x427b71] >> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830] >> perf(_start+0x29)[0x427d89] >> >> The memory corruption happens at: >> >> iter_add_next_cumulative_entry() >> { >> ... >> for (i = 0; i < iter->curr; i++) { >> ... >> } >> >> Whatever in iter_next_cumulative_entry() or in iter_add_next_cumulative_entry(), >> they all don't check if iter->curr exceeds the array 'he_cache[]'. >> >> If there are too many nodes in callchain, it's possible that iter->curr > >> iter->max_stack, then memory corruption occurs. >> >> This patch will reallocate array 'he_cache[]' in iter_next_cumulative_entry() >> if necessary (the case of too many nodes in callchain). > > right, the max_stack does not say how many nodes end up in > callchain_cursor at the end.. good catch, please mention > that also in the changelog > max_stack looks only to limit the number of calls but not for other branches. > however we know the final count from callchain_cursor itself, > the attached patch might do the same job, right? > I think the attached patch is ok. > also could we now get rid of iter->max_stack? > From my opinion, the option '--max-stack' in perf report looks not very necessary. While it's just my personal opinion, need to hear from more people. :) Thanks Jin Yao > thanks, > jirka > > > --- > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > index b6140950301e..b50b7b70dcca 100644 > --- a/tools/perf/util/hist.c > +++ b/tools/perf/util/hist.c > @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter, > * cumulated only one time to prevent entries more than 100% > * overhead. > */ > - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); > + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); > if (he_cache == NULL) > return -ENOMEM; > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history 2018-02-13 14:00 ` Jin, Yao @ 2018-02-16 2:25 ` Jin, Yao 2018-02-16 7:53 ` Jiri Olsa 0 siblings, 1 reply; 8+ messages in thread From: Jin, Yao @ 2018-02-16 2:25 UTC (permalink / raw) To: Jiri Olsa Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On 2/13/2018 10:00 PM, Jin, Yao wrote: > > > On 2/13/2018 5:45 PM, Jiri Olsa wrote: >> On Tue, Feb 13, 2018 at 04:44:28PM +0800, Jin Yao wrote: >>> Following command lines will cause perf crash. >>> >>> perf record -j call -g -a <application> >>> perf report --branch-history >>> >>> *** Error in `perf': double free or corruption (!prev): >>> 0x00000000104aa040 *** >>> ======= Backtrace: ========= >>> /lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725] >>> /lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a] >>> /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc] >>> perf[0x51b914] >>> perf(hist_entry_iter__add+0x1e5)[0x51f305] >>> perf[0x43cf01] >>> perf[0x4fa3bf] >>> perf[0x4fa923] >>> perf[0x4fd396] >>> perf[0x4f9614] >>> perf(perf_session__process_events+0x89e)[0x4fc38e] >>> perf(cmd_report+0x15d2)[0x43f202] >>> perf[0x4a059f] >>> perf(main+0x631)[0x427b71] >>> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830] >>> perf(_start+0x29)[0x427d89] >>> >>> The memory corruption happens at: >>> >>> iter_add_next_cumulative_entry() >>> { >>> ... >>> for (i = 0; i < iter->curr; i++) { >>> ... >>> } >>> >>> Whatever in iter_next_cumulative_entry() or in >>> iter_add_next_cumulative_entry(), >>> they all don't check if iter->curr exceeds the array 'he_cache[]'. >>> >>> If there are too many nodes in callchain, it's possible that >>> iter->curr > >>> iter->max_stack, then memory corruption occurs. >>> >>> This patch will reallocate array 'he_cache[]' in >>> iter_next_cumulative_entry() >>> if necessary (the case of too many nodes in callchain). >> >> right, the max_stack does not say how many nodes end up in >> callchain_cursor at the end.. good catch, please mention >> that also in the changelog >> > > max_stack looks only to limit the number of calls but not for other > branches. > >> however we know the final count from callchain_cursor itself, >> the attached patch might do the same job, right? >> > > I think the attached patch is ok. > >> also could we now get rid of iter->max_stack? >> > > From my opinion, the option '--max-stack' in perf report looks not very > necessary. While it's just my personal opinion, need to hear from more > people. :) > > Thanks > Jin Yao > >> thanks, >> jirka >> >> >> --- >> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c >> index b6140950301e..b50b7b70dcca 100644 >> --- a/tools/perf/util/hist.c >> +++ b/tools/perf/util/hist.c >> @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct >> hist_entry_iter *iter, >> * cumulated only one time to prevent entries more than 100% >> * overhead. >> */ >> - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); >> + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); >> if (he_cache == NULL) >> return -ENOMEM; >> Hi Jiri, I guess you will post this patch, right? Thanks Jin Yao ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history 2018-02-16 2:25 ` Jin, Yao @ 2018-02-16 7:53 ` Jiri Olsa 2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa 0 siblings, 1 reply; 8+ messages in thread From: Jiri Olsa @ 2018-02-16 7:53 UTC (permalink / raw) To: Jin, Yao Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Fri, Feb 16, 2018 at 10:25:31AM +0800, Jin, Yao wrote: SNIP > > From my opinion, the option '--max-stack' in perf report looks not very > > necessary. While it's just my personal opinion, need to hear from more > > people. :) > > > > Thanks > > Jin Yao > > > > > thanks, > > > jirka > > > > > > > > > --- > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > > index b6140950301e..b50b7b70dcca 100644 > > > --- a/tools/perf/util/hist.c > > > +++ b/tools/perf/util/hist.c > > > @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct > > > hist_entry_iter *iter, > > > * cumulated only one time to prevent entries more than 100% > > > * overhead. > > > */ > > > - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); > > > + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); > > > if (he_cache == NULL) > > > return -ENOMEM; > > > > > Hi Jiri, > > I guess you will post this patch, right? yep, later today jirka ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history 2018-02-16 7:53 ` Jiri Olsa @ 2018-02-16 12:36 ` Jiri Olsa 2018-02-16 13:02 ` Arnaldo Carvalho de Melo 2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jiri Olsa 0 siblings, 2 replies; 8+ messages in thread From: Jiri Olsa @ 2018-02-16 12:36 UTC (permalink / raw) To: Jin, Yao, acme Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Fri, Feb 16, 2018 at 08:53:04AM +0100, Jiri Olsa wrote: > On Fri, Feb 16, 2018 at 10:25:31AM +0800, Jin, Yao wrote: > > SNIP > > > > From my opinion, the option '--max-stack' in perf report looks not very > > > necessary. While it's just my personal opinion, need to hear from more > > > people. :) > > > > > > Thanks > > > Jin Yao > > > > > > > thanks, > > > > jirka > > > > > > > > > > > > --- > > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > > > index b6140950301e..b50b7b70dcca 100644 > > > > --- a/tools/perf/util/hist.c > > > > +++ b/tools/perf/util/hist.c > > > > @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct > > > > hist_entry_iter *iter, > > > > * cumulated only one time to prevent entries more than 100% > > > > * overhead. > > > > */ > > > > - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); > > > > + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); > > > > if (he_cache == NULL) > > > > return -ENOMEM; > > > > > > > > Hi Jiri, > > > > I guess you will post this patch, right? > > yep, later today here it is.. I think we want this change now to fix the crash, and some more fixes later to ensure that the branch stack code follows properly the logic of --max-stack, which is not the case now thanks, jirka --- Jin Yao reported memory corrupton in perf report with branch info used for stack trace: > Following command lines will cause perf crash. > perf record -j call -g -a <application> > perf report --branch-history > > *** Error in `perf': double free or corruption (!prev): 0x00000000104aa040 *** > ======= Backtrace: ========= > /lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725] > /lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a] > /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc] > perf[0x51b914] > perf(hist_entry_iter__add+0x1e5)[0x51f305] > perf[0x43cf01] > perf[0x4fa3bf] > perf[0x4fa923] > perf[0x4fd396] > perf[0x4f9614] > perf(perf_session__process_events+0x89e)[0x4fc38e] > perf(cmd_report+0x15d2)[0x43f202] > perf[0x4a059f] > perf(main+0x631)[0x427b71] > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830] > perf(_start+0x29)[0x427d89] For the cumulative output, we allocate he_cache array based on the --max-stack option value and populate it with data from callchain_cursor. The --max-stack option value does not ensure now the limit for number of callchain_cursor nodes, so the cumulative iter code will allocate smaller array than it's actually needed and cause above corruption. I think the --max-stack limit does not apply here anyway, because we add callchain data as normal hist entries, while the --max-stack control the limit of single entry callchain depth. Using the callchain_cursor.nr as he_cache array count to fix this. Also removing struct hist_entry_iter::max_stack, because there's no longer any use for it. We need more fixes to ensure that the branch stack code follows properly the logic of --max-stack, which is not the case at the moment. Reported-by: Jin Yao <yao.jin@linux.intel.com> Original-patch-by: Jin Yao <yao.jin@linux.intel.com> Link: http://lkml.kernel.org/n/tip-qj1kdpvyu25ac6w22lhmy7m2@git.kernel.org Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/util/hist.c | 4 +--- tools/perf/util/hist.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index b6140950301e..44a8456cea10 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter, * cumulated only one time to prevent entries more than 100% * overhead. */ - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); if (he_cache == NULL) return -ENOMEM; @@ -1045,8 +1045,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, if (err) return err; - iter->max_stack = max_stack_depth; - err = iter->ops->prepare_entry(iter, al); if (err) goto out; diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 02721b579746..e869cad4d89f 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -107,7 +107,6 @@ struct hist_entry_iter { int curr; bool hide_unresolved; - int max_stack; struct perf_evsel *evsel; struct perf_sample *sample; -- 2.13.6 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history 2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa @ 2018-02-16 13:02 ` Arnaldo Carvalho de Melo 2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jiri Olsa 1 sibling, 0 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-02-16 13:02 UTC (permalink / raw) To: Jiri Olsa Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin Em Fri, Feb 16, 2018 at 01:36:19PM +0100, Jiri Olsa escreveu: > > here it is.. I think we want this change now to fix the crash, and > some more fixes later to ensure that the branch stack code follows > properly the logic of --max-stack, which is not the case now Thanks, applied. - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/core] perf report: Fix memory corruption in --branch-history mode --branch-history 2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa 2018-02-16 13:02 ` Arnaldo Carvalho de Melo @ 2018-02-17 11:34 ` tip-bot for Jiri Olsa 1 sibling, 0 replies; 8+ messages in thread From: tip-bot for Jiri Olsa @ 2018-02-17 11:34 UTC (permalink / raw) To: linux-tip-commits Cc: acme, peterz, hpa, mingo, jolsa, alexander.shishkin, kan.liang, tglx, ak, jolsa, yao.jin, linux-kernel Commit-ID: e3ebaa465136ecfedf9c6f4671df02bf625f8125 Gitweb: https://git.kernel.org/tip/e3ebaa465136ecfedf9c6f4671df02bf625f8125 Author: Jiri Olsa <jolsa@redhat.com> AuthorDate: Fri, 16 Feb 2018 13:36:19 +0100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Fri, 16 Feb 2018 14:55:47 -0300 perf report: Fix memory corruption in --branch-history mode --branch-history Jin Yao reported memory corrupton in perf report with branch info used for stack trace: > Following command lines will cause perf crash. > perf record -j call -g -a <application> > perf report --branch-history > > *** Error in `perf': double free or corruption (!prev): 0x00000000104aa040 *** > ======= Backtrace: ========= > /lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725] > /lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a] > /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc] > perf[0x51b914] > perf(hist_entry_iter__add+0x1e5)[0x51f305] > perf[0x43cf01] > perf[0x4fa3bf] > perf[0x4fa923] > perf[0x4fd396] > perf[0x4f9614] > perf(perf_session__process_events+0x89e)[0x4fc38e] > perf(cmd_report+0x15d2)[0x43f202] > perf[0x4a059f] > perf(main+0x631)[0x427b71] > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830] > perf(_start+0x29)[0x427d89] For the cumulative output, we allocate the he_cache array based on the --max-stack option value and populate it with data from 'callchain_cursor'. The --max-stack option value does not ensure now the limit for number of callchain_cursor nodes, so the cumulative iter code will allocate smaller array than it's actually needed and cause above corruption. I think the --max-stack limit does not apply here anyway, because we add callchain data as normal hist entries, while the --max-stack control the limit of single entry callchain depth. Using the callchain_cursor.nr as he_cache array count to fix this. Also removing struct hist_entry_iter::max_stack, because there's no longer any use for it. We need more fixes to ensure that the branch stack code follows properly the logic of --max-stack, which is not the case at the moment. Original-patch-by: Jin Yao <yao.jin@linux.intel.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Reported-by: Jin Yao <yao.jin@linux.intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Andi Kleen <ak@linux.intel.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Kan Liang <kan.liang@intel.com> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20180216123619.GA9945@krava Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/hist.c | 4 +--- tools/perf/util/hist.h | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index b614095..44a8456 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter, * cumulated only one time to prevent entries more than 100% * overhead. */ - he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); + he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1)); if (he_cache == NULL) return -ENOMEM; @@ -1045,8 +1045,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, if (err) return err; - iter->max_stack = max_stack_depth; - err = iter->ops->prepare_entry(iter, al); if (err) goto out; diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 02721b57..e869cad 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -107,7 +107,6 @@ struct hist_entry_iter { int curr; bool hide_unresolved; - int max_stack; struct perf_evsel *evsel; struct perf_sample *sample; ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-17 11:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-13 8:44 [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history Jin Yao 2018-02-13 9:45 ` Jiri Olsa 2018-02-13 14:00 ` Jin, Yao 2018-02-16 2:25 ` Jin, Yao 2018-02-16 7:53 ` Jiri Olsa 2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa 2018-02-16 13:02 ` Arnaldo Carvalho de Melo 2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jiri Olsa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox