[PATCH 1/2] tools/perf: Add field to annotation options to determine disassembler used

[Venkat Rao Bagalkote]

Applied this patch on next-20250227, and issue is fixed.

./perf -v

perf version 6.14.rc4.g2a432e11ebbb

./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err
./perf annotate --stdio 1> out 2>err


Please add below tag.

Tested-By: Venkat Rao Bagalkote <venkat88 at linux.ibm.com>

Regards,

Venkat.

On 04/03/25 9:11 pm, Athira Rajeev wrote:
> When doing "perf annotate", perf tool provides option to
> use specific disassembler like llvm/objdump/capstone. The
> order picked is to use llvm first and if that fails fallback
> to objdump ie to use PERF_DISASM_LLVM, PERF_DISASM_CAPSTONE
> and PERF_DISASM_OBJDUMP
>
> In powerpc, when using "data type" sort keys, first preferred
> approach is to read the raw instruction from the DSO. In objdump
> is specified in "--objdump" option, it picks the symbol disassemble
> using objdump. Currently disasm_line__parse_powerpc() function
> uses length of the "line" to determine if objdump is used.
> But there are few cases, where if objdump doesn't recognise the
> instruction, the disassembled string will be empty.
>
> Example:
>
>       134cdc:	c4 05 82 41 	beq     1352a0 <getcwd+0x6e0>
>       134ce0:	ac 00 8e 40 	bne     cr3,134d8c <getcwd+0x1cc>
>       134ce4:	0f 00 10 04 	pld     r9,1028308
> ====>134ce8:	d4 b0 20 e5
>       134cec:	16 00 40 39 	li      r10,22
>       134cf0:	48 01 21 ea 	ld      r17,328(r1)
>
> So depending on length of line will give bad results.
>
> Add a new filed to annotation options structure,
> "struct annotation_options" to save the disassembler used.
> Use this info to determine if disassembly is done while
> parsing the disasm line.
>
> Reported-by: Tejas Manhas <Tejas.Manhas1 at ibm.com>
> Signed-off-by: Athira Rajeev <atrajeev at linux.ibm.com>
> ---
>   tools/perf/util/annotate.h |  1 +
>   tools/perf/util/disasm.c   | 22 +++++++++++++---------
>   2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 98db1b88daf4..30a344afd91a 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -58,6 +58,7 @@ struct annotation_options {
>   	     full_addr;
>   	u8   offset_level;
>   	u8   disassemblers[MAX_DISASSEMBLERS];
> +	u8   disassembler_used;
>   	int  min_pcnt;
>   	int  max_lines;
>   	int  context;
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 50c5c206b70e..a53e8c4e5bca 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -48,7 +48,7 @@ static int call__scnprintf(struct ins *ins, char *bf, size_t size,
>   
>   static void ins__sort(struct arch *arch);
>   static int disasm_line__parse(char *line, const char **namep, char **rawp);
> -static int disasm_line__parse_powerpc(struct disasm_line *dl);
> +static int disasm_line__parse_powerpc(struct disasm_line *dl, struct annotate_args *args);
>   static char *expand_tabs(char *line, char **storage, size_t *storage_len);
>   
>   static __attribute__((constructor)) void symbol__init_regexpr(void)
> @@ -968,24 +968,24 @@ static int disasm_line__parse(char *line, const char **namep, char **rawp)
>   #define	PPC_OP(op)	(((op) >> 26) & 0x3F)
>   #define	RAW_BYTES	11
>   
> -static int disasm_line__parse_powerpc(struct disasm_line *dl)
> +static int disasm_line__parse_powerpc(struct disasm_line *dl, struct annotate_args *args)
>   {
>   	char *line = dl->al.line;
>   	const char **namep = &dl->ins.name;
>   	char **rawp = &dl->ops.raw;
>   	char *tmp_raw_insn, *name_raw_insn = skip_spaces(line);
>   	char *name = skip_spaces(name_raw_insn + RAW_BYTES);
> -	int objdump = 0;
> +	int disasm = 0;
>   
> -	if (strlen(line) > RAW_BYTES)
> -		objdump = 1;
> +	if (args->options->disassembler_used)
> +		disasm = 1;
>   
>   	if (name_raw_insn[0] == '\0')
>   		return -1;
>   
> -	if (objdump) {
> +	if (disasm)
>   		disasm_line__parse(name, namep, rawp);
> -	} else
> +	else
>   		*namep = "";
>   
>   	tmp_raw_insn = strndup(name_raw_insn, 11);
> @@ -995,7 +995,7 @@ static int disasm_line__parse_powerpc(struct disasm_line *dl)
>   	remove_spaces(tmp_raw_insn);
>   
>   	sscanf(tmp_raw_insn, "%x", &dl->raw.raw_insn);
> -	if (objdump)
> +	if (disasm)
>   		dl->raw.raw_insn = be32_to_cpu(dl->raw.raw_insn);
>   
>   	return 0;
> @@ -1054,7 +1054,7 @@ struct disasm_line *disasm_line__new(struct annotate_args *args)
>   
>   	if (args->offset != -1) {
>   		if (arch__is(args->arch, "powerpc")) {
> -			if (disasm_line__parse_powerpc(dl) < 0)
> +			if (disasm_line__parse_powerpc(dl, args) < 0)
>   				goto out_free_line;
>   		} else if (disasm_line__parse(dl->al.line, &dl->ins.name, &dl->ops.raw) < 0)
>   			goto out_free_line;
> @@ -2289,16 +2289,20 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
>   
>   		switch (dis) {
>   		case PERF_DISASM_LLVM:
> +			args->options->disassembler_used = PERF_DISASM_LLVM;
>   			err = symbol__disassemble_llvm(symfs_filename, sym, args);
>   			break;
>   		case PERF_DISASM_CAPSTONE:
> +			args->options->disassembler_used = PERF_DISASM_CAPSTONE;
>   			err = symbol__disassemble_capstone(symfs_filename, sym, args);
>   			break;
>   		case PERF_DISASM_OBJDUMP:
> +			args->options->disassembler_used = PERF_DISASM_OBJDUMP;
>   			err = symbol__disassemble_objdump(symfs_filename, sym, args);
>   			break;
>   		case PERF_DISASM_UNKNOWN: /* End of disassemblers. */
>   		default:
> +			args->options->disassembler_used = PERF_DISASM_UNKNOWN;
>   			goto out_remove_tmp;
>   		}
>   		if (err == 0)