Hi Leo,
On 25/04/2019 13:56, Leo Yan wrote:
Hi Rob,
On Thu, Apr 25, 2019 at 01:43:02PM +0100, Robert Walker wrote:
Hi,
I've noticed that recent versions of perf will crash when processing CoreSight trace with some combinations of decoding options.
The crash occurs when neither the 'b' (branch events), or 'l' flag (last branch entries) options to --itrace are specified for the perf report/script/inject commands - e.g. 'perf report --itrace=i1000i'. The default flags includes 'b', so the crash doesn't occur if no '--itrace' option is given.
The crash appears to be due to the prev_packet field of cs_etm_queue (tools/perf/util/cs-etm.c) not being initialised unless either 'b' or 'l' is included in the trace flags. There seems to have been a number of uses of this field added without checking whether it's valid - these have been added in 5.0 (e.g. commits 7100b12: perf cs-etm: Generate branch sample for exception packet, 24fff5e: perf cs-etm: Avoid stale branch samples when flush packet) and there have been more added for 5.1-rc1: (06220bf: perf cs-etm: Set sample flags for instruction range packet and related commits)
As this field is used more widely now and it's already hard to follow which functions have been called in a context where the validity of prev_packet has been checked (i.e. "if (etm->synth_opts.last_branch || etm->sample_branches) { ... }", I think it would be better to create prev_packet in all cases.
Sorry for regression ...
I used gdb to quickly narrow this segment fault, and found it's caused by without checking prev_packet in the function cs_etm__exception().
Below patch can fix the issue at my side, could you confirm this? I still need a bit more time to get clear what's exactly flow to introduce this issue.
I think this should fix the regression in 5.0, but for the later patches in 5.1-rc1, there are other cases where an uninitialised prev_packet can be accessed - e.g. cs_etm__set_sample_flags() and other functions called from there.
Regards
Rob
---8<---
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 27a374ddf661..da17f6528a54 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1002,6 +1002,9 @@ static int cs_etm__sample(struct cs_etm_queue *etmq) static int cs_etm__exception(struct cs_etm_queue *etmq) {
- if (!etmq->prev_packet)
return 0;- /*
- When the exception packet is inserted, whether the last instruction
- in previous range packet is taken branch or not, we need to force
Thanks, Leo Yan