[PATCH v2] perf annotate: fix annotate crash for fused instructions

[PATCH v2] perf annotate: fix annotate crash for fused instructions

[Yao]

From: yao <dragonlinux@gmail.com>

Function is_fused tries to find the previous fused instruction with the
current conditional jump instruction in the annotate_line list.
disasm_line is the container of annotate_line.
The annotate_line list uses list_head and the head is in annotated_source:
notes->src which is not an annotate_line, so it couldn't walk without
checking the list_head. If so it will read annoted_source neighbour memory
mistakenly and most likely cause crash.

Fix the case where current instruction is a conditional jump and it is
the first instruction in the disassembled list, the original code will
walk to the head and treats it as disasm_line which is not.

    -------- backtrace --------
    #0 0x564dd8d5d5cd in ui__signal_backtrace setup.c:0
    #1 0x7f1737e3e6f0 libc.so.6[3e6f0]
    #2 0x7f1737ec10b1 strstr-sse2-unaligned.S:41
    #3 0x564dd8d625ef in intel__ins_is_fused annotate-x86.c:0
    #4 0x564dd8d4bcde in annotate_browser__refresh annotate.c:0
    #5 0x564dd8d49bd6 in __ui_browser__refresh.isra.0 browser.c:0
    #6 0x564dd8d4a960 in ui_browser__run ??:0
    #7 0x564dd8d4cbcf in __hist_entry__tui_annotate ??:0
    #8 0x564dd8d50a91 in do_annotate hists.c:0
    #9 0x564dd8d58bb1 in evsel__hists_browse hists.c:0
    #10 0x564dd8d5a7fa in evlist__tui_browse_hists ??:0
    #11 0x564dd8bf7a98 in cmd_report help.c:109
    #12 0x564dd8c713e4 in handle_internal_command help.c:109
    #13 0x564dd8bd6ac2 in main ??:0
    #14 0x7f1737e27781 libc_start_call_main.h:83
    #15 0x7f1737e278b9 in __libc_start_main libc-start.c:127
    #16 0x564dd8bd7095 in _start help.c:109

Fixes: 7efbcc8c075c ("perf annotate: Fix fused instr logic for assembly functions")

Signed-off-by: yao <dragonlinux@gmail.com>
---
 tools/perf/ui/browsers/annotate.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index cf36b5347f72..07449944fa2e 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -155,17 +155,26 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 
 static int is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
 {
-	struct disasm_line *pos = list_prev_entry(cursor, al.node);
+	struct annotation *notes = browser__annotation(&ab->b);
+	struct list_head *node = cursor->al.node.prev;
+	struct disasm_line *pos;
 	const char *name;
 	int diff = 1;
 
-	while (pos && pos->al.offset == -1) {
-		pos = list_prev_entry(pos, al.node);
+	/*
+	 * backwalk to the nearest neighbour instruction till the list head
+	 * instructions/non-insn are ordered from head to tail
+	 * head is not a annotate_line/disasm_line
+	 */
+	for (; node != &notes->src->source; node = node->prev) {
+		pos = list_entry(node, struct disasm_line, al.node);
+		if (pos->al.offset != -1)
+			break;
 		if (!annotate_opts.hide_src_code)
 			diff++;
 	}
 
-	if (!pos)
+	if (node == &notes->src->source)
 		return 0;
 
 	if (ins__is_lock(&pos->ins))
-- 
2.56.1

Re: [PATCH v2] perf annotate: fix annotate crash for fused instructions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] perf annotate: single-word handle in Signed-off-by trailer
--

commit 209a79b063ed73e21e886fe346b1c09d14939eb2
Author: yao <dragonlinux@gmail.com>

> perf annotate: fix annotate crash for fused instructions
>
> Function is_fused tries to find the previous fused instruction with the
> current conditional jump instruction in the annotate_line list.
> disasm_line is the container of annotate_line.
> The annotate_line list uses list_head and the head is in annotated_source:
> notes->src which is not an annotate_line, so it couldn't walk without
> checking the list_head. If so it will read annoted_source neighbour memory
> mistakenly and most likely cause crash.
>
> Fix the case where current instruction is a conditional jump and it is
> the first instruction in the disassembled list, the original code will
> walk to the head and treats it as disasm_line which is not.
>
>     -------- backtrace --------
>     #0 0x564dd8d5d5cd in ui__signal_backtrace setup.c:0
>     #1 0x7f1737e3e6f0 libc.so.6[3e6f0]
>     #2 0x7f1737ec10b1 strstr-sse2-unaligned.S:41
>     #3 0x564dd8d625ef in intel__ins_is_fused annotate-x86.c:0
>     #4 0x564dd8d4bcde in annotate_browser__refresh annotate.c:0
>     #5 0x564dd8d49bd6 in __ui_browser__refresh.isra.0 browser.c:0
>     #6 0x564dd8d4a960 in ui_browser__run ??:0
>     #7 0x564dd8d4cbcf in __hist_entry__tui_annotate ??:0
>     #8 0x564dd8d50a91 in do_annotate hists.c:0
>     #9 0x564dd8d58bb1 in evsel__hists_browse hists.c:0
>     #10 0x564dd8d5a7fa in evlist__tui_browse_hists ??:0
>     #11 0x564dd8bf7a98 in cmd_report help.c:109
>     #12 0x564dd8c713e4 in handle_internal_command help.c:109
>     #13 0x564dd8bd6ac2 in main ??:0
>     #14 0x7f1737e27781 libc_start_call_main.h:83
>     #15 0x7f1737e278b9 in __libc_start_main libc-start.c:127
>     #16 0x564dd8bd7095 in _start help.c:109
>
> Fixes: 7efbcc8c075c ("perf annotate: Fix fused instr logic for assembly functions")
>
> Signed-off-by: yao <dragonlinux@gmail.com>

[Severity: High]
The Author header and Signed-off-by trailer use a single-word handle
("yao") instead of a full real human name, which does not meet the
Developer's Certificate of Origin requirements in
Documentation/process/submitting-patches.rst.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114183124.685561-1-dragonlinux@gmail.com?part=1