Consider the following code:
00000000004000f4 <test_branch>: 4000f4: eb02003f cmp x1, x2 4000f8: 54000109 b.ls 400118 <taken_path>
00000000004000fc <fallthrough>: 4000fc: d282f2c2 mov x2, #0x1796 // #6038 ...
0000000000400118 <taken_path>: ...
When execution reaches 0x4000f8, the b.ls condition evaluates false, and execution falls through to 0x4000fc. If an asynchronous exception (for example, an interrupt) is taken before the next instruction executes, cs_etm__exception() currently forces the previous range packet's last_instr_taken_branch flag to true - in the case above, perf incorrectly synthesizes a branch sample for the untaken branch, even though execution actually falls through.
The trace log contains the following bogus branch sample at 0x4000f8:
perf script --itrace=b -F comm,ip,insn,disasm,sym
b_ls_not_taken_ 400108 fallthrough insn: 61 ff ff 54 b.ne #0x4000f4 b_ls_not_taken_ 400108 fallthrough insn: 61 ff ff 54 b.ne #0x4000f4 b_ls_not_taken_ 4000f8 test_branch insn: 09 01 00 54 b.ls #0x400118 b_ls_not_taken_ ffff800080010c80 vectors insn: 03 00 00 14 b #0xffff800080010c8c b_ls_not_taken_ ffff800080010ca4 vectors insn: 41 02 00 14 b #0xffff8000800115a8
The special fixup is only needed for SVC. For SVC exception entry, cs_etm__set_sample_flags() has already identified the previous range as a syscall branch, but last_instr_taken_branch is not set for the SVC instruction. Restrict to force last_instr_taken_branch to true for a system call only.
After:
b_ls_not_taken_ 400108 fallthrough insn: 61 ff ff 54 b.ne #0x4000f4 b_ls_not_taken_ 400108 fallthrough insn: 61 ff ff 54 b.ne #0x4000f4 b_ls_not_taken_ ffff800080010c80 vectors insn: 03 00 00 14 b #0xffff800080010c8c b_ls_not_taken_ ffff800080010ca4 vectors insn: 41 02 00 14 b #0xffff8000800115a8
Fixes: 7100b12cf474 ("perf cs-etm: Generate branch sample for exception packet") Signed-off-by: Leo Yan leo.yan@arm.com --- tools/perf/util/cs-etm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 95e3ec1171acc8442d8539a72a26a1a5a53a2f37..05e698f666bbe07af9caeda1a673f22d7196817a 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1999,17 +1999,20 @@ static int cs_etm__context(struct cs_etm_queue *etmq, static int cs_etm__exception(struct cs_etm_traceid_queue *tidq) { /* - * When the exception packet is inserted, whether the last instruction - * in previous range packet is taken branch or not, we need to force - * to set 'prev_packet->last_instr_taken_branch' to true. This ensures - * to generate branch sample for the instruction range before the - * exception is trapped to kernel or before the exception returning. + * cs_etm__set_sample_flags() has already copied exception flags to + * the previous range packet. Do not mark every exception boundary as a + * taken branch: an async exception can arrive after an untaken branch, + * and forcing the flag would synthesize a bogus branch sample. * - * The exception packet includes the dummy address values, so don't - * swap PACKET with PREV_PACKET. This keeps PREV_PACKET to be useful - * for generating instruction and branch samples. + * Keep the fixup only for SVC. The decoder reports the range ending in + * SVC without last_instr_taken_branch set, but perf represents SVC + * exception entry as a syscall branch and needs the flag to emit that + * branch sample. */ - if (tidq->prev_packet->sample_type == CS_ETM_RANGE) + if (tidq->prev_packet->sample_type == CS_ETM_RANGE && + tidq->prev_packet->flags == (PERF_IP_FLAG_BRANCH | + PERF_IP_FLAG_CALL | + PERF_IP_FLAG_SYSCALLRET)) tidq->prev_packet->last_instr_taken_branch = true;
return 0;