Re: [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction disassembly mismatch with objdump

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak of the dynamically allocated string `name` on an error path if a comma is missing in the disassembled instruction, which also brittly aborts the entire disassembly process.
--

--- Patch [2]: [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction disassembly mismatch with objdump ---
commit d33f174b93415a7430f8bf32e53bbf77ad2fce9a
Author: Tengda Wu <[email protected]>

perf llvm: Fix arm64 adrp instruction disassembly mismatch with objdump

This commit addresses an inconsistency where libllvm parses arm64 adrp
operands as raw immediates instead of the address format used by objdump.
It extracts the target address and resolves the symbol, reformatting the
disassembly output to match objdump's expectations.

> diff --git a/tools/perf/util/llvm.c b/tools/perf/util/llvm.c
> index a0deb742a7331..6b53b283205ab 100644
> --- a/tools/perf/util/llvm.c
> +++ b/tools/perf/util/llvm.c

[ ... ]

> @@ -230,10 +242,40 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
>  		if (storage.pcrel_load_addr != 0) {
>  			char *name = llvm_name_for_data(dso, filename,
>  							storage.pcrel_load_addr);
> -			disasm_len += scnprintf(disasm_buf + disasm_len,
> -						sizeof(disasm_buf) - disasm_len,
> -						"  # %#"PRIx64,
> -						storage.pcrel_load_addr);
> +
> +			if (arch__is_arm64(args->arch)) {
> +				/*
> +				 * For arm64, replace the immediate operand with
> +				 * the resolved address to match objdump's output.
> +				 *
> +				 * Example conversion:
> +				 *   From: adrp  x18, 8014
> +				 *   To:   adrp  x18, ffff800081f5f000
> +				 */
> +				char *s = strchr(disasm_buf, ',');
> +
> +				if (s == NULL)
> +					goto err;

[Severity: Medium]
Does this leak name if a comma isn't found in the disassembled instruction?

Looking at symbol__disassemble_llvm(), name is allocated earlier in this
block via llvm_name_for_data(). If strchr() fails and the code jumps to the
err label, name appears to be leaked since the err cleanup path only frees
code_buf and line_storage.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2