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

On 2026/8/8 21:03, [email protected] wrote:
> 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.
> 

Sorry, that was a careless mistake. Will fix it right away.

-- Tengda