perf arm64: Support data type profiling

In print_capstone_detail(), maps__find() is used to locate the kernel map. This function increments the reference count of the found map object. However, the current implementation fails to call map__put() after the map is no longer needed, leading to a reference count leak.

Fix this by adding a map__put(map) call to properly release the reference after use.

Fixes 92dfc59463d5 ("perf annotate: Add symbol name when using capstone")

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/capstone.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

tools/perf/util/capstone.c +9 -4

index 5ad537fea436..9bba78ee0c5a 100644
@@ -302,6 +302,7 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
 	for (i = 0; i < insn->detail->x86.op_count; i++) {
 		struct cs_x86_op *op = &insn->detail->x86.operands[i];
 		u64 orig_addr;
+		struct map *found_map = NULL;
 
 		if (op->type != X86_OP_MEM)
 			continue;

@@ -317,19 +318,22 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
 		if (dso__kernel(map__dso(map))) {
 			/*
 			 * The kernel maps can be split into sections, let's
-			 * find the map first and the search the symbol.
+			 * find the map first and then search the symbol.
 			 */
-			map = maps__find(map__kmaps(map), addr);
-			if (map == NULL)
+			found_map = maps__find(map__kmaps(map), addr);
+			if (found_map == NULL)
 				continue;
+			map = found_map;
 		}
 
 		/* convert it to map-relative address for search */
 		addr = map__map_ip(map, addr);
 
 		sym = map__find_symbol(map, addr);
-		if (sym == NULL)
+		if (sym == NULL) {
+			map__put(found_map);
 			continue;
+		}
 
 		if (addr == sym->start) {
 			scnprintf(buf, len, "\t# %"PRIx64" <%s>",

@@ -338,6 +342,7 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
 			scnprintf(buf, len, "\t# %"PRIx64" <%s+%#"PRIx64">",
 				  orig_addr, sym->name, addr - sym->start);
 		}
+		map__put(found_map);
 		break;
 	}
 }

Signature · 1 line

2.34.1

Quoted 5 lines · Tengda Wu

In print_capstone_detail(), maps__find() is used to locate the kernel map. This function increments the reference count of the found map object. However, the current implementation fails to call map__put() after the map is no longer needed, leading to a reference count leak.

Quoted 5 lines

Fix this by adding a map__put(map) call to properly release the reference after use.

Fixes 92dfc59463d5 ("perf annotate: Add symbol name when using capstone")

Signed-off-by Tengda Wu <[email protected]>

I'll pick this up separately.

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Symbol resolution fails for relocated kernels (KASLR) and user-space shared libraries due to mismatched address spaces.
- [Low] Build failure due to undeclared machine variable and undefined machine__normalized_is() function in capstone_init().
- [Low] Missing <stdlib.h> inclusion causes musl libc build failure.
- [Low] Usage of ARM64_GRP_BRANCH_RELATIVE causes build failures with Capstone versions prior to 5.0.
--

commit 1f9744f04e456b1209677658edb7f30a5b26e56d
Author: Tengda Wu <[email protected]>

perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump

The patch implements extended formatting for arm64 jump and adrp instructions during symbol__disassemble_capstone() to match objdump's expected output style. It adds instruction detail resolution in capstone_init() and formats the output with correct symbol and offset mappings.

Quoted 4 lines

tools/perf/util/capstone.c +0 -0

index 9bba78ee0c5a2..a9c0a1d2eb036 100644

[ ... ]

tools/perf/util/capstone.c static int capstone_init(uint16_t e_machine, csh *cs_handle, bool is64, bool is_ · line 221

Quoted 5 lines

tools/perf/util/capstone.c +1 -0

@@ -218,6 +218,12 @@ static int capstone_init(uint16_t e_machine, csh *cs_handle, bool is64, bool is_
 		 * on x86 by investigating instruction details.
 		 */
 		perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
+	} else if (machine__normalized_is(machine, "arm64")) {

[Severity: Low]
Does this code cause a build failure? The variable machine is not declared in capstone_init(), and the function machine__normalized_is() does not appear to be defined in the codebase.

Quoted 8 lines

tools/perf/util/capstone.c +5 -0

+		/*
+		 * Same as x86: arm64 needs instruction details to resolve
+		 * symbolic addresses
+		 */
+		perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
 	}
 
 	return 0;

[ ... ]

Quoted 12 lines

tools/perf/util/capstone.c +12 -0

+static bool is_pc_relative_insn(struct cs_insn *insn)
+{
+	int i;
+
+	if (insn->id == ARM64_INS_ADR || insn->id == ARM64_INS_ADRP)
+		return true;
+
+	if (insn->detail == NULL)
+		return false;
+
+	for (i = 0; i < insn->detail->groups_count; i++) {
+		if (insn->detail->groups[i] == ARM64_GRP_JUMP ||

tools/perf/util/capstone.c static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len, · line 384

Quoted 2 lines

tools/perf/util/capstone.c +2 -0

+		    insn->detail->groups[i] == ARM64_GRP_CALL ||
+		    insn->detail->groups[i] == ARM64_GRP_BRANCH_RELATIVE)

[Severity: Low]
Will this break compilation against Capstone versions prior to 5.0? ARM64_GRP_BRANCH_RELATIVE was introduced in Capstone 5.0. Since the perf tool's build feature detection doesn't mandate a specific Capstone version, relying on this enum might cause build failures on older setups.

Quoted 10 lines

tools/perf/util/capstone.c +10 -0

+			return true;
+	}
+
+	return false;
+}
+
+static void format_capstone_insn_arm64(struct cs_insn *insn, char *buf,
+				       size_t len, struct annotate_args *args,
+				       u64 addr)
+{

[ ... ]

tools/perf/util/capstone.c static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len, · line 415

Quoted 7 lines

tools/perf/util/capstone.c +7 -0

+	if (is_pc_relative_insn(insn)) {
+		/* Extract last immediate value as address */
+		last_imm = strrchr(buf, '#');
+		if (!last_imm)
+			return;
+
+		orig_addr = strtoull(last_imm + 1, &endptr, 16);

[Severity: Low]
Can this cause a build failure on musl libc? The POSIX standard function strtoull() is used here, but stdlib.h is not explicitly included in the file. Relying on implicit inclusions often works on glibc but breaks on musl libc.

tools/perf/util/capstone.c static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len, · line 428

Quoted 13 lines

tools/perf/util/capstone.c +13 -0

+		if (endptr == last_imm + 1)
+			return;
+
+		/* Relocate map that contains the address */
+		if (dso__kernel(map__dso(map))) {
+			found_map = maps__find(map__kmaps(map), orig_addr);
+			if (found_map == NULL)
+				return;
+			map = found_map;
+		}
+
+		/* Convert it to map-relative address for search */
+		addr = map__map_ip(map, orig_addr);

[Severity: High]
Does this code fail to resolve symbols for relocated kernels (KASLR) or user-space shared libraries (PIE)?

The extracted orig_addr is in the objdump address space. However, maps__find() and map__map_ip() expect absolute memory addresses. By omitting the translation step via map__objdump_2mem(map, orig_addr) (which the x86 implementation handles correctly), KASLR relocated kernels will fail in maps__find(), and user-space shared libraries will underflow in map__map_ip().

Quoted 6 lines

tools/perf/util/capstone.c +6 -0

+
+		sym = map__find_symbol(map, addr);
+		if (sym == NULL) {
+			map__put(found_map);
+			return;
+		}

Quoted 24 lines · On 2026/7/1 12:07, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Symbol resolution fails for relocated kernels (KASLR) and user-space shared libraries due to mismatched address spaces.
- [Low] Build failure due to undeclared machine variable and undefined machine__normalized_is() function in capstone_init().
- [Low] Missing <stdlib.h> inclusion causes musl libc build failure.
- [Low] Usage of ARM64_GRP_BRANCH_RELATIVE causes build failures with Capstone versions prior to 5.0.
--

commit 1f9744f04e456b1209677658edb7f30a5b26e56d
Author: Tengda Wu <[email protected]>

perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump

The patch implements extended formatting for arm64 jump and adrp instructions during symbol__disassemble_capstone() to match objdump's expected output style. It adds instruction detail resolution in capstone_init() and formats the output with correct symbol and offset mappings.

Quoted 4 lines

tools/perf/util/capstone.c +0 -0

index 9bba78ee0c5a2..a9c0a1d2eb036 100644

[ ... ]

Quoted 11 lines

tools/perf/util/capstone.c static int capstone_init(uint16_t e_machine, csh *cs_handle, bool is64, bool is_ · line 221

Quoted 5 lines

tools/perf/util/capstone.c +1 -0

@@ -218,6 +218,12 @@ static int capstone_init(uint16_t e_machine, csh *cs_handle, bool is64, bool is_
 		 * on x86 by investigating instruction details.
 		 */
 		perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
+	} else if (machine__normalized_is(machine, "arm64")) {

[Severity: Low]
Does this code cause a build failure? The variable machine is not declared in capstone_init(), and the function machine__normalized_is() does not appear to be defined in the codebase.

Agreed. This is due to commit 12c4737f55f2 ("perf capstone: Determine architecture from e_machine"), which changed the capstone interface. I haven't rebased this part yet. The base-commit already fails to build with capstone, so I didn't notice this one.

Quoted 24 lines

Quoted 9 lines

tools/perf/util/capstone.c +5 -0

+		/*
+		 * Same as x86: arm64 needs instruction details to resolve
+		 * symbolic addresses
+		 */
+		perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
 	}
 
 	return 0;

[ ... ]

Quoted 12 lines

tools/perf/util/capstone.c +12 -0

+static bool is_pc_relative_insn(struct cs_insn *insn)
+{
+	int i;
+
+	if (insn->id == ARM64_INS_ADR || insn->id == ARM64_INS_ADRP)
+		return true;
+
+	if (insn->detail == NULL)
+		return false;
+
+	for (i = 0; i < insn->detail->groups_count; i++) {
+		if (insn->detail->groups[i] == ARM64_GRP_JUMP ||

Quoted 8 lines

tools/perf/util/capstone.c static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len, · line 384

Quoted 2 lines

tools/perf/util/capstone.c +2 -0

+		    insn->detail->groups[i] == ARM64_GRP_CALL ||
+		    insn->detail->groups[i] == ARM64_GRP_BRANCH_RELATIVE)

[Severity: Low]
Will this break compilation against Capstone versions prior to 5.0? ARM64_GRP_BRANCH_RELATIVE was introduced in Capstone 5.0. Since the perf tool's build feature detection doesn't mandate a specific Capstone version, relying on this enum might cause build failures on older setups.

Quoted 1 line

This enum was added in Capstone 5.0 over two years ago. I think we can safely assume most systems are on a newer version now. I'll leave it as-is.

Quoted 14 lines

Quoted 11 lines

tools/perf/util/capstone.c +10 -0

+			return true;
+	}
+
+	return false;
+}
+
+static void format_capstone_insn_arm64(struct cs_insn *insn, char *buf,
+				       size_t len, struct annotate_args *args,
+				       u64 addr)
+{

[ ... ]

Quoted 14 lines

tools/perf/util/capstone.c static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len, · line 415

Quoted 7 lines

tools/perf/util/capstone.c +7 -0

+	if (is_pc_relative_insn(insn)) {
+		/* Extract last immediate value as address */
+		last_imm = strrchr(buf, '#');
+		if (!last_imm)
+			return;
+
+		orig_addr = strtoull(last_imm + 1, &endptr, 16);

[Severity: Low]
Can this cause a build failure on musl libc? The POSIX standard function strtoull() is used here, but stdlib.h is not explicitly included in the file. Relying on implicit inclusions often works on glibc but breaks on musl libc.

I'd say we can ignore this. strtoull() is used all over the perf codebase and gets included indirectly. I don't think this is a real issue.

Quoted 19 lines

Quoted 4 lines

tools/perf/util/capstone.c +3 -0

+		if (endptr == last_imm + 1)
+			return;
+

tools/perf/util/capstone.c static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len, · line 428

Quoted 10 lines

tools/perf/util/capstone.c +10 -0

+		/* Relocate map that contains the address */
+		if (dso__kernel(map__dso(map))) {
+			found_map = maps__find(map__kmaps(map), orig_addr);
+			if (found_map == NULL)
+				return;
+			map = found_map;
+		}
+
+		/* Convert it to map-relative address for search */
+		addr = map__map_ip(map, orig_addr);

[Severity: High]
Does this code fail to resolve symbols for relocated kernels (KASLR) or user-space shared libraries (PIE)?

Quoted 7 lines

The extracted orig_addr is in the objdump address space. However, maps__find() and map__map_ip() expect absolute memory addresses. By omitting the translation step via map__objdump_2mem(map, orig_addr) (which the x86 implementation handles correctly), KASLR relocated kernels will fail in maps__find(), and user-space shared libraries will underflow in map__map_ip().

Agreed. I missed map__objdump_2mem() here.

Quoted 7 lines

Quoted 6 lines

tools/perf/util/capstone.c +6 -0

+
+		sym = map__find_symbol(map, addr);
+		if (sym == NULL) {
+			map__put(found_map);
+			return;
+		}

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing explicit inclusion of <string.h> for POSIX string functions. --

commit c3571a268605b8e2ad28ebe5943729438021e5fe
Author: Tengda Wu <[email protected]>

perf llvm: Fix arm64 adrp instruction disassembly mismatch with objdump

This commit addresses an inconsistency between libllvm and objdump when parsing arm64 'adrp' instructions. It extracts the target address during callbacks and formats it as an absolute address with symbol names in symbol__disassemble_llvm().

Quoted 4 lines

tools/perf/util/llvm.c +0 -0

index a0deb742a7331..533d47e8084d7 100644

[ ... ]

Quoted 9 lines

tools/perf/util/llvm.c +5 -0

@@ -227,6 +241,42 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 				free(name);
 			}
 		}
+		if (storage.pcrel_adrp_addr != 0) {
+			/*
+			 * ADRP (Address Page) instructions encode a 21-bit signed
+			 * immediate offset relative to the current PC's page.
+			 *

tools/perf/util/llvm.c int symbol__disassemble_llvm(const char *filename, struct symbol *sym, · line 258

Quoted 10 lines

tools/perf/util/llvm.c +10 -0

+			 * To maintain consistency with standard objdump output,
+			 * we truncate the raw encoded immediate at the comma
+			 * and replace it with the resolved absolute page address.
+			 *
+			 * Example conversion:
+			 * From: adrp x18, 8014
+			 * To:   adrp x18, ffff800081f5f000 <this_cpu_vector>
+			 */
+			char *name;
+			char *s = strchr(disasm_buf, ',');

[Severity: Low]
Does this file need to explicitly include <string.h>?

This patch introduces a new use of strchr() here in symbol__disassemble_llvm(), but tools/perf/util/llvm.c lacks a direct inclusion of <string.h> at the top of the file.

While this might compile under glibc due to implicit header inclusion, missing explicit POSIX headers can cause build failures on systems using musl libc, which enforces strict namespace separation.

Quoted 7 lines

tools/perf/util/llvm.c +7 -0

+
+			if (s == NULL)
+				goto err;
+
+			s++;
+			*s = '\0';
+			disasm_len = strlen(disasm_buf);

[ ... ]

The operands of 'adrp' instructions parsed by libllvm are currently represented as raw immediates rather than the "address <symbol+offset>" format used by objdump. This inconsistency causes arm64_mov__parse() to fail when parsing these instructions during post-processing.

Example of the mismatch:
Current: adrp x18, 8014
Fix: adrp x18, ffff800081f5f000 <this_cpu_vector>

Fix this by manually extracting the target address from the raw adrp instruction via symbol_lookup_callback(). The address is then converted to a specific symbol during symbol__disassemble_llvm() and formatted to match objdump's output, ensuring compatibility with existing parsers.

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/llvm.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

tools/perf/util/llvm.c +1 -0

index a0deb742a733..533d47e8084d 100644
@@ -94,6 +94,7 @@ static void init_llvm(void)
 struct symbol_lookup_storage {
 	u64 branch_addr;
 	u64 pcrel_load_addr;
+	u64 pcrel_adrp_addr;
 };
 
 static const char *

tools/perf/util/llvm.c +49 -0

+	else if (*ref_type == LLVMDisassembler_ReferenceType_In_ARM64_ADRP) {
+		uint64_t adrp_imm;
+
+		/* immhi (bits 23:5) and immlo (bits 30:29) */
+		adrp_imm = ((value & 0x00ffffe0) >> 3) | ((value >> 29) & 0x3);
+		/* Sign-extend the 21-bit immediate to 64-bit */
+		if (adrp_imm & (1ULL << 20))
+			adrp_imm |= ~((1ULL << 21) - 1);
+
+		/* Calculate the target page address */
+		storage->pcrel_adrp_addr = (address & ~0xFFFLL) + (adrp_imm << 12);
+	}
 	*ref_type = LLVMDisassembler_ReferenceType_InOut_None;
 	return NULL;
 }

@@ -204,6 +217,7 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 
 		storage.branch_addr = 0;
 		storage.pcrel_load_addr = 0;
+		storage.pcrel_adrp_addr = 0;
 
 		/*
 		 * LLVM's API has the code be disassembled as non-const, cast

@@ -227,6 +241,42 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 				free(name);
 			}
 		}
+		if (storage.pcrel_adrp_addr != 0) {
+			/*
+			 * ADRP (Address Page) instructions encode a 21-bit signed
+			 * immediate offset relative to the current PC's page.
+			 *
+			 * To maintain consistency with standard objdump output,
+			 * we truncate the raw encoded immediate at the comma
+			 * and replace it with the resolved absolute page address.
+			 *
+			 * Example conversion:
+			 * From: adrp x18, 8014
+			 * To:   adrp x18, ffff800081f5f000 <this_cpu_vector>
+			 */
+			char *name;
+			char *s = strchr(disasm_buf, ',');
+
+			if (s == NULL)
+				goto err;
+
+			s++;
+			*s = '\0';
+			disasm_len = strlen(disasm_buf);
+			disasm_len += scnprintf(disasm_buf + disasm_len,
+						sizeof(disasm_buf) - disasm_len,
+						" %"PRIx64,
+						storage.pcrel_adrp_addr);
+			name = llvm_name_for_data(dso, filename,
+						  storage.pcrel_adrp_addr);
+			if (name) {
+				disasm_len += scnprintf(disasm_buf + disasm_len,
+							sizeof(disasm_buf) -
+							disasm_len,
+							" <%s>", name);
+				free(name);
+			}
+		}
 		if (storage.pcrel_load_addr != 0) {
 			char *name = llvm_name_for_data(dso, filename,
 							storage.pcrel_load_addr);

Signature · 1 line

2.34.1

Quoted 33 lines · Tengda Wu

The operands of 'adrp' instructions parsed by libllvm are currently represented as raw immediates rather than the "address <symbol+offset>" format used by objdump. This inconsistency causes arm64_mov__parse() to fail when parsing these instructions during post-processing.

Example of the mismatch:
Current: adrp x18, 8014
Fix: adrp x18, ffff800081f5f000 <this_cpu_vector>

Fix this by manually extracting the target address from the raw adrp instruction via symbol_lookup_callback(). The address is then converted to a specific symbol during symbol__disassemble_llvm() and formatted to match objdump's output, ensuring compatibility with existing parsers.

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/llvm.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

tools/perf/util/llvm.c +1 -0

index a0deb742a733..533d47e8084d 100644
@@ -94,6 +94,7 @@ static void init_llvm(void)
 struct symbol_lookup_storage {
 	u64 branch_addr;
 	u64 pcrel_load_addr;
+	u64 pcrel_adrp_addr;
 };
 
 static const char *

tools/perf/util/llvm.c symbol_lookup_callback(void *disinfo, uint64_t value, · line 111

Quoted 3 lines

tools/perf/util/llvm.c +0 -0

@@ -108,6 +109,18 @@ symbol_lookup_callback(void *disinfo, uint64_t value,
 		storage->branch_addr = value;
 	else if (*ref_type == LLVMDisassembler_ReferenceType_In_PCrel_Load)
 		storage->pcrel_load_addr = value;

Is this used by arm64? If not, would be possible to reuse pcrel_load_addr unless it'd complicate the code significantly?

I think it's the common concept of PC-relative addressing while it requires two instructions to set up the final address on arm64.

Thanks,
Namhyung

Quoted 69 lines

tools/perf/util/llvm.c +49 -0

+	else if (*ref_type == LLVMDisassembler_ReferenceType_In_ARM64_ADRP) {
+		uint64_t adrp_imm;
+
+		/* immhi (bits 23:5) and immlo (bits 30:29) */
+		adrp_imm = ((value & 0x00ffffe0) >> 3) | ((value >> 29) & 0x3);
+		/* Sign-extend the 21-bit immediate to 64-bit */
+		if (adrp_imm & (1ULL << 20))
+			adrp_imm |= ~((1ULL << 21) - 1);
+
+		/* Calculate the target page address */
+		storage->pcrel_adrp_addr = (address & ~0xFFFLL) + (adrp_imm << 12);
+	}
 	*ref_type = LLVMDisassembler_ReferenceType_InOut_None;
 	return NULL;
 }

@@ -204,6 +217,7 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 
 		storage.branch_addr = 0;
 		storage.pcrel_load_addr = 0;
+		storage.pcrel_adrp_addr = 0;
 
 		/*
 		 * LLVM's API has the code be disassembled as non-const, cast

@@ -227,6 +241,42 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 				free(name);
 			}
 		}
+		if (storage.pcrel_adrp_addr != 0) {
+			/*
+			 * ADRP (Address Page) instructions encode a 21-bit signed
+			 * immediate offset relative to the current PC's page.
+			 *
+			 * To maintain consistency with standard objdump output,
+			 * we truncate the raw encoded immediate at the comma
+			 * and replace it with the resolved absolute page address.
+			 *
+			 * Example conversion:
+			 * From: adrp x18, 8014
+			 * To:   adrp x18, ffff800081f5f000 <this_cpu_vector>
+			 */
+			char *name;
+			char *s = strchr(disasm_buf, ',');
+
+			if (s == NULL)
+				goto err;
+
+			s++;
+			*s = '\0';
+			disasm_len = strlen(disasm_buf);
+			disasm_len += scnprintf(disasm_buf + disasm_len,
+						sizeof(disasm_buf) - disasm_len,
+						" %"PRIx64,
+						storage.pcrel_adrp_addr);
+			name = llvm_name_for_data(dso, filename,
+						  storage.pcrel_adrp_addr);
+			if (name) {
+				disasm_len += scnprintf(disasm_buf + disasm_len,
+							sizeof(disasm_buf) -
+							disasm_len,
+							" <%s>", name);
+				free(name);
+			}
+		}
 		if (storage.pcrel_load_addr != 0) {
 			char *name = llvm_name_for_data(dso, filename,
 							storage.pcrel_load_addr);

Signature · 1 line

2.34.1

Quoted 41 lines · Namhyung Kim

Quoted 33 lines · Tengda Wu

The operands of 'adrp' instructions parsed by libllvm are currently represented as raw immediates rather than the "address <symbol+offset>" format used by objdump. This inconsistency causes arm64_mov__parse() to fail when parsing these instructions during post-processing.

Example of the mismatch:
Current: adrp x18, 8014
Fix: adrp x18, ffff800081f5f000 <this_cpu_vector>

Fix this by manually extracting the target address from the raw adrp instruction via symbol_lookup_callback(). The address is then converted to a specific symbol during symbol__disassemble_llvm() and formatted to match objdump's output, ensuring compatibility with existing parsers.

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/llvm.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

tools/perf/util/llvm.c +1 -0

index a0deb742a733..533d47e8084d 100644
@@ -94,6 +94,7 @@ static void init_llvm(void)
 struct symbol_lookup_storage {
 	u64 branch_addr;
 	u64 pcrel_load_addr;
+	u64 pcrel_adrp_addr;
 };
 
 static const char *

tools/perf/util/llvm.c symbol_lookup_callback(void *disinfo, uint64_t value, · line 111

Quoted 3 lines

tools/perf/util/llvm.c +0 -0

@@ -108,6 +109,18 @@ symbol_lookup_callback(void *disinfo, uint64_t value,
 		storage->branch_addr = value;
 	else if (*ref_type == LLVMDisassembler_ReferenceType_In_PCrel_Load)
 		storage->pcrel_load_addr = value;

Is this used by arm64? If not, would be possible to reuse pcrel_load_addr unless it'd complicate the code significantly?

Quoted 6 lines

I think it's the common concept of PC-relative addressing while it requires two instructions to set up the final address on arm64.

Thanks,
Namhyung

Quoted 70 lines

Quoted 70 lines

tools/perf/util/llvm.c +49 -0

+	else if (*ref_type == LLVMDisassembler_ReferenceType_In_ARM64_ADRP) {
+		uint64_t adrp_imm;
+
+		/* immhi (bits 23:5) and immlo (bits 30:29) */
+		adrp_imm = ((value & 0x00ffffe0) >> 3) | ((value >> 29) & 0x3);
+		/* Sign-extend the 21-bit immediate to 64-bit */
+		if (adrp_imm & (1ULL << 20))
+			adrp_imm |= ~((1ULL << 21) - 1);
+
+		/* Calculate the target page address */
+		storage->pcrel_adrp_addr = (address & ~0xFFFLL) + (adrp_imm << 12);
+	}
 	*ref_type = LLVMDisassembler_ReferenceType_InOut_None;
 	return NULL;
 }

@@ -204,6 +217,7 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 
 		storage.branch_addr = 0;
 		storage.pcrel_load_addr = 0;
+		storage.pcrel_adrp_addr = 0;
 
 		/*
 		 * LLVM's API has the code be disassembled as non-const, cast

@@ -227,6 +241,42 @@ int symbol__disassemble_llvm(const char *filename, struct symbol *sym,
 				free(name);
 			}
 		}
+		if (storage.pcrel_adrp_addr != 0) {
+			/*
+			 * ADRP (Address Page) instructions encode a 21-bit signed
+			 * immediate offset relative to the current PC's page.
+			 *
+			 * To maintain consistency with standard objdump output,
+			 * we truncate the raw encoded immediate at the comma
+			 * and replace it with the resolved absolute page address.
+			 *
+			 * Example conversion:
+			 * From: adrp x18, 8014
+			 * To:   adrp x18, ffff800081f5f000 <this_cpu_vector>
+			 */
+			char *name;
+			char *s = strchr(disasm_buf, ',');
+
+			if (s == NULL)
+				goto err;
+
+			s++;
+			*s = '\0';
+			disasm_len = strlen(disasm_buf);
+			disasm_len += scnprintf(disasm_buf + disasm_len,
+						sizeof(disasm_buf) - disasm_len,
+						" %"PRIx64,
+						storage.pcrel_adrp_addr);
+			name = llvm_name_for_data(dso, filename,
+						  storage.pcrel_adrp_addr);
+			if (name) {
+				disasm_len += scnprintf(disasm_buf + disasm_len,
+							sizeof(disasm_buf) -
+							disasm_len,
+							" <%s>", name);
+				free(name);
+			}
+		}
 		if (storage.pcrel_load_addr != 0) {
 			char *name = llvm_name_for_data(dso, filename,
 							storage.pcrel_load_addr);

Signature · 1 line

2.34.1

Quoted 44 lines · Tengda Wu

Quoted 41 lines · Namhyung Kim

Quoted 33 lines · Tengda Wu

The operands of 'adrp' instructions parsed by libllvm are currently represented as raw immediates rather than the "address <symbol+offset>" format used by objdump. This inconsistency causes arm64_mov__parse() to fail when parsing these instructions during post-processing.

Example of the mismatch:
Current: adrp x18, 8014
Fix: adrp x18, ffff800081f5f000 <this_cpu_vector>

Fix this by manually extracting the target address from the raw adrp instruction via symbol_lookup_callback(). The address is then converted to a specific symbol during symbol__disassemble_llvm() and formatted to match objdump's output, ensuring compatibility with existing parsers.

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/llvm.c | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

tools/perf/util/llvm.c +1 -0

index a0deb742a733..533d47e8084d 100644
@@ -94,6 +94,7 @@ static void init_llvm(void)
 struct symbol_lookup_storage {
 	u64 branch_addr;
 	u64 pcrel_load_addr;
+	u64 pcrel_adrp_addr;
 };
 
 static const char *

tools/perf/util/llvm.c symbol_lookup_callback(void *disinfo, uint64_t value, · line 111

Quoted 3 lines

tools/perf/util/llvm.c +0 -0

@@ -108,6 +109,18 @@ symbol_lookup_callback(void *disinfo, uint64_t value,
 		storage->branch_addr = value;
 	else if (*ref_type == LLVMDisassembler_ReferenceType_In_PCrel_Load)
 		storage->pcrel_load_addr = value;

Is this used by arm64? If not, would be possible to reuse pcrel_load_addr unless it'd complicate the code significantly?

Quoted 6 lines

Quoted 6 lines

I think it's the common concept of PC-relative addressing while it requires two instructions to set up the final address on arm64.

Thanks,
Namhyung

I checked the LLVM internal implementation and conducted further experiments. You are correct, arm64 does not trigger LLVMDisassembler_ReferenceType_In_PCrel_Load at all. Instead, it utilizes its own architecture-specific reference types like LLVMDisassembler_ReferenceType_In_ARM64_ADRP, _LDRXl, etc. Therefore, 'pcrel_load_addr' is completely safe to reuse here.

I will reuse 'pcrel_load_addr' to avoid adding unnecessary fields and optimize this patch in the next version.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The parsing logic permanently corrupts the raw instruction string buffer (ops->raw) when restoring the < character after extracting a source symbol name. --

commit 13742d589ccf5a86dcfd0991f541a8f1d46f7e0f
Author: Tengda Wu <[email protected]>

perf annotate-arm64: Generalize arm64_mov__parse to support more instructions

This commit generalizes arm64_mov__parse() to parse source and target operands for a wider variety of arm64 instructions. It utilizes arm64__check_multi_regs() to identify multiple registers while attempting to preserve parsing behavior for instructions containing the addr <symbol> suffix.

Quoted 4 lines

tools/perf/util/annotate-arch/annotate-arm64.c +0 -0

index 33080fdca125f..a1667237114a6 100644

[ ... ]

Quoted 6 lines

tools/perf/util/annotate-arch/annotate-arm64.c +0 -0

@@ -14,59 +16,129 @@
 static int arm64_mov__parse(const struct arch *arch,
 			    struct ins_operands *ops,
 			    struct map_symbol *ms __maybe_unused,
 			    struct disasm_line *dl __maybe_unused)
 {

[ ... ]

Quoted 8 lines

tools/perf/util/annotate-arch/annotate-arm64.c +3 -2

+	/* Parse a symbol followed by an address, if present */
 	s = strchr(endptr, '<');
 	if (s == NULL)
-		goto out_free_target;
-	endptr = strchr(s + 1, '>');
+		return 0;
+	endptr = strrchr(s + 1, '>');
 	if (endptr == NULL)

tools/perf/util/annotate-arch/annotate-arm64.c struct arch_arm64 { · line 127

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-arm64.c +3 -2

-		goto out_free_target;
+		return 0;
 
 	*endptr = '\0';
 	*s = ' ';
-	ops->target.name = strdup(s);
+	s = skip_spaces(++s);
+	ops->source.name = strdup(s);
 	*s = '<';
 	*endptr = '>';

[Severity: Medium]
Does this code overwrite the first character of the symbol name?

Because s is advanced by skip_spaces(++s), s now points to the first character of the symbol name. When *s = '<' is executed immediately after, it appears to corrupt the ops->raw instruction string buffer instead of restoring the '<' character at its original position.

This could result in a visual glitch in the fallback raw instruction view where the first character of the symbol name is overwritten (for example, 0x1000 <sym> becomes 0x1000 <ym>).

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64_ldst__parse fails to parse PC-relative load instructions, breaking operand extraction for literal pool loads.
- [Medium] Failure to strip inline comments before parsing operands allows comments to corrupt multi_regs evaluation. --

commit 9939a67ebc615cf049eb33c68f83aa3646abb3f0
Author: Tengda Wu <[email protected]>

perf annotate-arm64: Handle load and store instructions

This commit adds ldst_ops to parse data types and offsets for ARM64 memory access instructions using regex prefix matching.

Quoted 4 lines

tools/perf/util/annotate-arch/annotate-arm64.c +0 -0

index a1667237114a6..451500def8c12 100644

[ ... ]

Quoted 1 line

tools/perf/util/annotate-arch/annotate-arm64.c +0 -0

@@ -141,6 +142,104 @@ static const struct ins_ops arm64_mov_ops = {

[ ... ]

tools/perf/util/annotate-arch/annotate-arm64.c static const struct ins_ops arm64_mov_ops = { · line 199

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-arm64.c +10 -0

+static int arm64_ldst__parse(const struct arch *arch,
+			     struct ins_operands *ops,
+			     struct map_symbol *ms __maybe_unused,
+			     struct disasm_line *dl)
+{
+	char *s, *left, *right;
+
+	right = s = strchr(ops->raw, arch->objdump.memory_ref_char);
+	if (!s)
+		return -1;

[Severity: Medium]
Will this fail to parse PC-relative load instructions?

When processing a PC-relative load instruction (e.g., ldr x0, 4005c0 <sym>), the 'ld' prefix matches the new ldst_insn regex in arch__new_arm64() and routes it to arm64_ldst__parse().

Since PC-relative loads lack a bracket ('['), strchr() will return NULL, causing the function to immediately return -1. Does this leave operands unpopulated and break annotation features like data type tracking for literal pool loads?

Quoted 17 lines

tools/perf/util/annotate-arch/annotate-arm64.c +17 -0

+
+	while (s > ops->raw && *s != ',')
+		--s;
+
+	if (s == ops->raw)
+		return -1;
+
+	*s = '\0';
+	left = strdup(ops->raw);
+
+	*s = ',';
+	if (!left)
+		return -1;
+
+	right = strdup(right);
+	if (!right) {
+		zfree(&left);

tools/perf/util/annotate-arch/annotate-arm64.c static const struct ins_ops arm64_mov_ops = { · line 226

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-arm64.c +10 -0

+		return -1;
+	}
+
+	if (arm64__insn_is_source_first(dl)) {
+		ops->source.raw = left;
+		ops->source.mem_ref = false;
+
+		ops->target.raw = right;
+		ops->target.mem_ref = true;
+		ops->target.multi_regs = arm64__check_multi_regs(arch, ops->target.raw);

[Severity: Medium]
Does this code incorrectly include inline comments when evaluating multi_regs?

The string duplicated into 'right' includes trailing comments from the raw operand string because they are not stripped prior to parsing.

If objdump outputs an inline comment containing a comma and a string resembling a register name (for example: // read, x2), could arm64__check_multi_regs() wrongly flag the operand as having multiple registers and corrupt data type profiling?

Quoted 11 lines

tools/perf/util/annotate-arch/annotate-arm64.c +11 -0

+	} else {
+		ops->source.raw = right;
+		ops->source.mem_ref = true;
+		ops->source.multi_regs = arm64__check_multi_regs(arch, ops->source.raw);
+
+		ops->target.raw = left;
+		ops->target.mem_ref = false;
+	}
+
+	return 0;
+}

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The zero registers ('wzr' and 'xzr') are incorrectly mapped to DWARF register 31.
- [Medium] The link register ('lr') alias is no longer recognized and will unexpectedly return -ENOENT.
- [Low] The functions strcmp and strtol are used without explicitly including <string.h> and <stdlib.h>. --

commit cf10eca9678152514cc1354dc6997be3eec9f436
Author: Tengda Wu <[email protected]>

perf dwarf-regs: Adapt get_dwarf_regnum() for arm64

This commit replaces the static string table lookup for arm64 DWARF registers with a programmatic implementation in __get_dwarf_regnum_arm64(). This allows resolving register names directly into their corresponding DWARF numbers during annotation.

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c line 3

Quoted 8 lines

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c +1 -0

index 593ca7d4fccca..be55fc2a4f385 100644
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <errno.h>
+#include <ctype.h>

[Severity: Low]
Does omitting the explicit includes for <string.h> and <stdlib.h> break the build on musl libc?

This file now uses strcmp() and strtol() below, and musl strictly separates declarations without relying on implicit inclusions.

Quoted 2 lines

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c +0 -0

 #include <dwarf-regs.h>
 #include "../../../arch/arm64/include/uapi/asm/perf_regs.h"

[ ... ]

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c int __get_dwarf_regnum_for_perf_regnum_arm64(int perf_regnum) · line 20

Quoted 6 lines

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c +6 -0

+int __get_dwarf_regnum_arm64(const char *name)
+{
+	int reg;
+
+	if (!strcmp(name, "sp") || !strcmp(name, "wzr") || !strcmp(name, "xzr"))
+		return 31;

[Severity: Medium]
Does this incorrectly map the zero registers (wzr and xzr) to DWARF register 31?

The AArch64 DWARF ABI assigns register 31 strictly to the stack pointer (sp), and zero registers have no valid DWARF mapping.

Mapping wzr and xzr to 31 might cause perf annotate to conflate operations on the zero register with stack pointer operations, leading to incorrect stack variable tracking and type profiling statistics.

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c int __get_dwarf_regnum_for_perf_regnum_arm64(int perf_regnum) · line 23

Quoted 3 lines

tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c +3 -0

+
+	if (*name != 'x' && *name != 'w')
+		return -ENOENT;

[Severity: Medium]
Will this strict prefix check reject the link register alias lr?

The previous static table lookup correctly mapped lr to register 30. Because lr does not start with x or w, it appears this will now return -ENOENT, which might break data type tracking for instructions that operate on the link register.

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Refactoring changes the error propagation behavior for memory operands with unrecognized registers, causing valid instructions to be incorrectly marked as bad.
- [Low] Missing explicit inclusion of <stdlib.h> for the POSIX libc function strtol(), breaking musl compatibility.
--

commit a79e03a1004b442317df26b7478d0a1daed1e563
Author: Tengda Wu <[email protected]>

perf annotate: Introduce extract_op_location callback for arch-specific parsing

This patch refactors operand parsing by moving it from the central annotate_get_insn_location() into architecture-specific callbacks. By introducing extract_op_location callbacks for PowerPC and x86, it aims to simplify the core logic and improve maintainability without making functional changes.

tools/perf/util/annotate-arch/annotate-x86.c line 8

Quoted 12 lines

tools/perf/util/annotate-arch/annotate-x86.c +1 -0

index 7e61365363938..6a4b4bdcd7a7c 100644
@@ -2,6 +2,7 @@
 #include <string.h>
 #include <linux/compiler.h>
 #include <linux/zalloc.h>
+#include <linux/ctype.h>
 #include <assert.h>
 #include <inttypes.h>
 #include "../annotate-data.h"

[Severity: Low]
Does this file need an explicit include of <stdlib.h> for musl libc compatibility?

It appears strtol() is used later in extract_op_location_x86() without the required <stdlib.h> header being directly included here, which might lead to implicit function declaration errors on musl-based systems.

[ ... ]

Quoted 13 lines

tools/perf/util/annotate-arch/annotate-x86.c +9 -0

@@ -809,6 +810,86 @@ static void update_insn_state_x86(struct type_state *state,
 }
 #endif
 
+/*
+ * Get register number and access offset from the given instruction.
+ * It assumes AT&T x86 asm format like OFFSET(REG).  Maybe it needs
+ * to revisit the format when it handles different architecture.
+ * Fills @reg and @offset when return 0.
+ */
+static int extract_reg_offset(const struct arch *arch, const char *str,
+			      struct annotated_op_loc *op_loc)
+{

[ ... ]

Quoted 4 lines

tools/perf/util/annotate-arch/annotate-x86.c +4 -0

+static int extract_op_location_x86(const struct arch *arch,
+				   struct disasm_line *dl __maybe_unused,
+				   const char *op_str, int op_idx __maybe_unused,
+				   struct annotated_op_loc *op_loc)

tools/perf/util/annotate-arch/annotate-x86.c static void update_insn_state_x86(struct type_state *state, · line 869

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-x86.c +10 -0

+{
+	const char *s = op_str;
+	char *p = NULL;
+
+	if (op_str == NULL)
+		return 0;
+
+	if (strchr(op_str, arch->objdump.memory_ref_char)) {
+		op_loc->mem_ref = true;
+		return extract_reg_offset(arch, op_str, op_loc);

[Severity: Medium]
Does returning the result of extract_reg_offset() here alter the error propagation behavior?

In the previous annotate_get_insn_location() logic, the return value of extract_reg_offset() was safely ignored:

        } else if (strchr(insn_str, arch->objdump.memory_ref_char)) {
                op_loc->mem_ref = true;
                op_loc->multi_regs = multi_regs;
                extract_reg_offset(arch, insn_str, op_loc);
        } else {

By returning it here, extract_op_location_x86() might return -1 if the register is unrecognized. This regression is now propagated back to the caller in annotate_get_insn_location():

        if (arch->extract_op_location) {
                ret = arch->extract_op_location(arch, dl, insn_str, i, op_loc);
                if (ret)
                        return ret;
        }

Could this cause valid instructions to be completely skipped and incorrectly marked as bad?

Quoted 7 lines

tools/perf/util/annotate-arch/annotate-x86.c +7 -0

+	}
+
+	/* FIXME: Handle other segment registers */
+	if (!strncmp(op_str, "%gs:", 4)) {
+		op_loc->segment = INSN_SEG_X86_GS;
+		op_loc->offset = strtol(op_str + 4,
+					&p, 0);

Quoted 33 lines · On 2026/7/1 12:06, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Refactoring changes the error propagation behavior for memory operands with unrecognized registers, causing valid instructions to be incorrectly marked as bad.
- [Low] Missing explicit inclusion of <stdlib.h> for the POSIX libc function strtol(), breaking musl compatibility.
--

commit a79e03a1004b442317df26b7478d0a1daed1e563
Author: Tengda Wu <[email protected]>

perf annotate: Introduce extract_op_location callback for arch-specific parsing

This patch refactors operand parsing by moving it from the central annotate_get_insn_location() into architecture-specific callbacks. By introducing extract_op_location callbacks for PowerPC and x86, it aims to simplify the core logic and improve maintainability without making functional changes.

tools/perf/util/annotate-arch/annotate-x86.c line 8

Quoted 12 lines

tools/perf/util/annotate-arch/annotate-x86.c +1 -0

index 7e61365363938..6a4b4bdcd7a7c 100644
@@ -2,6 +2,7 @@
 #include <string.h>
 #include <linux/compiler.h>
 #include <linux/zalloc.h>
+#include <linux/ctype.h>
 #include <assert.h>
 #include <inttypes.h>
 #include "../annotate-data.h"

[Severity: Low]
Does this file need an explicit include of <stdlib.h> for musl libc compatibility?

Quoted 4 lines

It appears strtol() is used later in extract_op_location_x86() without the required <stdlib.h> header being directly included here, which might lead to implicit function declaration errors on musl-based systems.

Agreed.

Quoted 50 lines

[ ... ]

Quoted 13 lines

tools/perf/util/annotate-arch/annotate-x86.c +9 -0

@@ -809,6 +810,86 @@ static void update_insn_state_x86(struct type_state *state,
 }
 #endif
 
+/*
+ * Get register number and access offset from the given instruction.
+ * It assumes AT&T x86 asm format like OFFSET(REG).  Maybe it needs
+ * to revisit the format when it handles different architecture.
+ * Fills @reg and @offset when return 0.
+ */
+static int extract_reg_offset(const struct arch *arch, const char *str,
+			      struct annotated_op_loc *op_loc)
+{

[ ... ]

Quoted 4 lines

tools/perf/util/annotate-arch/annotate-x86.c +4 -0

+static int extract_op_location_x86(const struct arch *arch,
+				   struct disasm_line *dl __maybe_unused,
+				   const char *op_str, int op_idx __maybe_unused,
+				   struct annotated_op_loc *op_loc)

tools/perf/util/annotate-arch/annotate-x86.c static void update_insn_state_x86(struct type_state *state, · line 869

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-x86.c +10 -0

+{
+	const char *s = op_str;
+	char *p = NULL;
+
+	if (op_str == NULL)
+		return 0;
+
+	if (strchr(op_str, arch->objdump.memory_ref_char)) {
+		op_loc->mem_ref = true;
+		return extract_reg_offset(arch, op_str, op_loc);

[Severity: Medium]
Does returning the result of extract_reg_offset() here alter the error propagation behavior?

In the previous annotate_get_insn_location() logic, the return value of extract_reg_offset() was safely ignored:

        } else if (strchr(insn_str, arch->objdump.memory_ref_char)) {
                op_loc->mem_ref = true;
                op_loc->multi_regs = multi_regs;
                extract_reg_offset(arch, insn_str, op_loc);
        } else {

By returning it here, extract_op_location_x86() might return -1 if the register is unrecognized. This regression is now propagated back to the caller in annotate_get_insn_location():

Quoted 9 lines

        if (arch->extract_op_location) {
                ret = arch->extract_op_location(arch, dl, insn_str, i, op_loc);
                if (ret)
                        return ret;
        }

Could this cause valid instructions to be completely skipped and incorrectly marked as bad?

I don't think this is a problem. If we don't return an error here, extract_reg_offset() would continue with an uninitialized or invalid offset, which would then cause update_insn_state() to make incorrect judgments. Returning -1 and marking it as bad is actually safer than silently propagating wrong information.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Post-index memory references with register offsets are incorrectly classified as SIGNED_OFFSET. --

commit 1b03950d2edd24d57bf3a7ad6271aea1980c92e3
Author: Tengda Wu <[email protected]>

perf annotate-arm64: Implement extract_op_location() callback

Implement the extract_op_location() callback for the arm64 architecture to handle its specific assembly syntax and addressing modes, enabling data type profiling for this architecture.

Quoted 4 lines

tools/perf/util/annotate-arch/annotate-arm64.c +0 -0

index 451500def8c12..77b6f4841c849 100644

[ ... ]

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-arm64.c +10 -0

+static int extract_op_location_arm64(const struct arch *arch,
+				     struct disasm_line *dl __maybe_unused,
+				     const char *op_str, int op_idx __maybe_unused,
+				     struct annotated_op_loc *op_loc)
+{
+	const char *s = op_str;
+	char *p = NULL;
+
+	if (op_str == NULL)
+		return 0;

[ ... ]

Quoted 7 lines

tools/perf/util/annotate-arch/annotate-arm64.c +7 -0

+	/*
+	 * Handle memory references (e.g., [x0, #8]), identify
+	 * arm64 specific addressing modes
+	 */
+	if (*s == arch->objdump.memory_ref_char) {
+		op_loc->mem_ref = true;
+

tools/perf/util/annotate-arch/annotate-arm64.c static const struct ins_ops *arm64__associate_instruction_ops(struct arch *arch, · line 299

Quoted 10 lines

tools/perf/util/annotate-arch/annotate-arm64.c +10 -0

+		p = strchr(s, ']');
+		if (p == NULL)
+			return -1;
+
+		/* Pre-index: [base, #imm]! */
+		if (p[1] == '!')
+			op_loc->addr_mode = PERF_ADDR_MODE_PRE_INDEX;
+		/* Post-index: [base], #imm */
+		else if (p[1] == ',' && strchr(p + 1, arch->objdump.imm_char))
+			op_loc->addr_mode = PERF_ADDR_MODE_POST_INDEX;

[Severity: Medium]
Does this incorrectly restrict post-index addressing classification to only immediate offsets?

In ARM64, a comma immediately following the closing bracket (p[1] == ',') is sufficient to identify a post-index addressing mode, regardless of whether the offset is an immediate or a register (for example, Advanced SIMD instructions like ld1 {v0.16b}, [x2], x5).

If the condition fails for register offsets, it appears it will incorrectly fall through to PERF_ADDR_MODE_SIGNED_OFFSET. Could this cause the base register write-back to be silently ignored in adjust_reg_index_state(), leading to stale and corrupted type state tracking for the base register?

Quoted 6 lines

tools/perf/util/annotate-arch/annotate-arm64.c +6 -0

+		/* Signed offset: [base{, #imm}] */
+		else
+			op_loc->addr_mode = PERF_ADDR_MODE_SIGNED_OFFSET;
+
+		s++;
+	}

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Invalid union access causes severe type punning, false positive event dropping, and logic failure.
- [Medium] Unprotected dereference of potentially NULL evsel->name causing a segmentation fault. --

commit 21f6f127e80c94aececac4065deebfce02da27b3
Author: Tengda Wu <[email protected]>

perf annotate: Deduplicate overlapping ARM SPE events for data type profiling

When data type profiling is enabled on ARM SPE, multiple overlapping events are synthesized for a single sampled instruction. This patch implements automatic deduplication in process_sample_event() by retaining only the "instructions" event when data type annotation is enabled and no explicit itrace options are configured.

Quoted 11 lines

tools/perf/builtin-annotate.c +1 -0

index 69cb72b2082a6..46b2bd3fb2013 100644
@@ -283,9 +283,22 @@ static int process_sample_event(const struct perf_tool *tool,
 				struct machine *machine)
 {
 	struct perf_annotate *ann = container_of(tool, struct perf_annotate, tool);
+	struct evsel *evsel = sample->evsel;
 	struct addr_location al;
 	int ret = 0;

tools/perf/builtin-annotate.c static int process_sample_event(const struct perf_tool *tool, · line 298

Quoted 10 lines

tools/perf/builtin-annotate.c +9 -0

 
+	/*
+	 * ARM SPE synthesizes multiple overlapping events (e.g., l1d-miss,
+	 * tlb-access) for a single sampled instruction. When data type is
+	 * enabled, counting all of these overlapping events would inflate
+	 * the profile and distort the data type statistics. To avoid this,
+	 * deduplicate by retaining only the comprehensive "instructions" event.
+	 */
+	if (ann->data_type && !ann->session->itrace_synth_opts->set &&
+	    event->auxtrace_info.type == PERF_AUXTRACE_ARM_SPE &&

[Severity: Medium]
Since event is a union of all perf events, does checking event->auxtrace_info.type on a PERF_RECORD_SAMPLE event read the sample data payload as if it were an auxtrace type?

If a normal event's first 32-bit payload (like PID 4 when PERF_SAMPLE_TID is the first element) equals 4 (PERF_AUXTRACE_ARM_SPE), could this falsely match and silently drop the event?

tools/perf/builtin-annotate.c static int process_sample_event(const struct perf_tool *tool, · line 299

Quoted 2 lines

tools/perf/builtin-annotate.c +1 -0

+	    strcmp(evsel->name, "instructions") != 0)

[Severity: Medium]
Could evsel->name be NULL here if the event is unnamed or if we are processing a perf.data file missing the HEADER_EVENT_DESC feature?

Passing NULL to strcmp() would cause a segmentation fault. Would it be safer to use the evsel__name(evsel) wrapper instead?

Quoted 4 lines

tools/perf/builtin-annotate.c +2 -0

+		return 0;
+
 	addr_location__init(&al);

[ ... ]

tools/perf/util/arm-spe.c static void arm_spe_prep_sample(struct arm_spe *spe, · line 406

Quoted 9 lines

tools/perf/util/arm-spe.c +1 -0

index 552f063f126e6..e027e3b200bfb 100644
@@ -403,6 +403,7 @@ static void arm_spe_prep_sample(struct arm_spe *spe,
 	event->sample.header.type = PERF_RECORD_SAMPLE;
 	event->sample.header.misc = sample->cpumode;
 	event->sample.header.size = sizeof(struct perf_event_header);
+	event->auxtrace_info.type = PERF_AUXTRACE_ARM_SPE;

[Severity: Medium]
Writing to auxtrace_info.type modifies offset 8 of the event union, which aliases the sample payload.

If trace injection is used, does perf_event__synthesize_sample() subsequently overwrite this offset with the actual sample payload (such as sample->id), erasing this magic value and breaking the deduplication entirely?

When using --itrace=M for data type profiling on arm64, the 'Percent' values in perf annotate output are all zero:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Percent     offset       size  field
    0.00          0       0x10  struct mmu_gather_batch  {
    0.00          0        0x8      struct mmu_gather_batch*    next;
    0.00        0x8        0x4      unsigned int        nr;
    0.00        0xc        0x4      unsigned int        max;
    0.00       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

However, adding the -n option reveals non-zero sample counts:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Samples     offset       size  field
      15          0       0x10  struct mmu_gather_batch  {
      13          0        0x8      struct mmu_gather_batch*    next;
       2        0x8        0x4      unsigned int        nr;
       0        0xc        0x4      unsigned int        max;
       0       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

The root cause is that when period is not explicitly specified in the --itrace option, it remains zero after itrace_do_parse_synth_opts(). The zero period then flows through annotated_data_type__update_samples() where h->period accumulates to zero, and print_annotated_data_type_value() calculates the 'Percent' as zero.

In itrace_do_parse_synth_opts(), non-'iy' options have their period type set to PERF_ITRACE_PERIOD_INSTRUCTIONS, but period remains zero. Since a zero period is meaningless for this type, default to 1 (one sample per instruction).

With this fix applied, the result is as follows:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Percent     offset       size  field
  100.00          0       0x10  struct mmu_gather_batch  {
   86.67          0        0x8      struct mmu_gather_batch*    next;
   13.33        0x8        0x4      unsigned int        nr;
    0.00        0xc        0x4      unsigned int        max;
    0.00       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/auxtrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/auxtrace.c +6 -0

index 0b851f32e98c..415b68a2bba9 100644
@@ -1759,6 +1759,12 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
 			synth_opts->period = PERF_ITRACE_DEFAULT_PERIOD;
 	}
 
+	if (!period_set &&
+	    synth_opts->period_type == PERF_ITRACE_PERIOD_INSTRUCTIONS) {
+		/* Indicates a sample is taken for every instruction. */
+		synth_opts->period = 1;
+	}
+
 	return 0;
 
 out_err:

Quoted 25 lines · Tengda Wu

When using --itrace=M for data type profiling on arm64, the 'Percent' values in perf annotate output are all zero:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Percent     offset       size  field
    0.00          0       0x10  struct mmu_gather_batch  {
    0.00          0        0x8      struct mmu_gather_batch*    next;
    0.00        0x8        0x4      unsigned int        nr;
    0.00        0xc        0x4      unsigned int        max;
    0.00       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

However, adding the -n option reveals non-zero sample counts:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Samples     offset       size  field
      15          0       0x10  struct mmu_gather_batch  {
      13          0        0x8      struct mmu_gather_batch*    next;
       2        0x8        0x4      unsigned int        nr;
       0        0xc        0x4      unsigned int        max;
       0       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

Quoted 5 lines

The root cause is that when period is not explicitly specified in the --itrace option, it remains zero after itrace_do_parse_synth_opts(). The zero period then flows through annotated_data_type__update_samples() where h->period accumulates to zero, and print_annotated_data_type_value() calculates the 'Percent' as zero.

The period is for instructions samples i.e. options 'i' or 'y'. Why is it being used in the 'M' case? What samples are being synthesized in that case?

Quoted 40 lines

In itrace_do_parse_synth_opts(), non-'iy' options have their period type set to PERF_ITRACE_PERIOD_INSTRUCTIONS, but period remains zero. Since a zero period is meaningless for this type, default to 1 (one sample per instruction).

With this fix applied, the result is as follows:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Percent     offset       size  field
  100.00          0       0x10  struct mmu_gather_batch  {
   86.67          0        0x8      struct mmu_gather_batch*    next;
   13.33        0x8        0x4      unsigned int        nr;
    0.00        0xc        0x4      unsigned int        max;
    0.00       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/auxtrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/auxtrace.c +6 -0

index 0b851f32e98c..415b68a2bba9 100644
@@ -1759,6 +1759,12 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
 			synth_opts->period = PERF_ITRACE_DEFAULT_PERIOD;
 	}
 
+	if (!period_set &&
+	    synth_opts->period_type == PERF_ITRACE_PERIOD_INSTRUCTIONS) {
+		/* Indicates a sample is taken for every instruction. */
+		synth_opts->period = 1;
+	}
+
 	return 0;
 
 out_err:

Hi Adrian,

Quoted 32 lines · Adrian Hunter

Quoted 25 lines · Tengda Wu

When using --itrace=M for data type profiling on arm64, the 'Percent' values in perf annotate output are all zero:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Percent     offset       size  field
    0.00          0       0x10  struct mmu_gather_batch  {
    0.00          0        0x8      struct mmu_gather_batch*    next;
    0.00        0x8        0x4      unsigned int        nr;
    0.00        0xc        0x4      unsigned int        max;
    0.00       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

However, adding the -n option reveals non-zero sample counts:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Samples     offset       size  field
      15          0       0x10  struct mmu_gather_batch  {
      13          0        0x8      struct mmu_gather_batch*    next;
       2        0x8        0x4      unsigned int        nr;
       0        0xc        0x4      unsigned int        max;
       0       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

Quoted 5 lines

The root cause is that when period is not explicitly specified in the --itrace option, it remains zero after itrace_do_parse_synth_opts(). The zero period then flows through annotated_data_type__update_samples() where h->period accumulates to zero, and print_annotated_data_type_value() calculates the 'Percent' as zero.

Quoted 4 lines

The period is for instructions samples i.e. options 'i' or 'y'. Why is it being used in the 'M' case? What samples are being synthesized in that case?

The 'M' case is just one example. On arm64, 't' and 'f' also have the same percent-zero issue.

How ARM SPE synthesizes events:

ARM SPE can synthesize multiple event types from a single instruction. For example, the 'M' case targets instructions that cause memory interaction events, while the 't' case targets instructions that trigger TLB access/miss events. Users can specify the desired event types via the itrace option to obtain corresponding data type profiling results.

How data type computes Percent:

Data type profiling computes Percent based on the period. The flow is as follows:

arm_spe_prep_sample():

        sample->period = spe->synth_opts.period;

__hists__add_entry():

        he.stat.period = sample->period;

annotated_data_type__update_samples():

        h->period += period;
        h->addr[offset].period += period;

print_annotated_data_value():

        double percent = h->period ? (100.0 * period / h->period) : 0;

Therefore, if synth_opts.period is not specified (remains zero), the final Percent calculation results in zero.

As you pointed out, if period is intended to be limited to only the 'i' and 'y' options, then setting period to 1 in the current patch may not be appropriate.

In fact, within print_annotated_data_value(), Percent could also be computed from the sample count. Moreover, I noticed that 'perf annotate' already has a --percent-type parameter that allows users to specify whether Percent is based on period or hit (which I understand to be the sample count). Perhaps it would be more reasonable to adapt print_annotated_data_value() to honor the --percent-type parameter instead.

Thanks,
Tengda

Quoted 41 lines

Quoted 40 lines

In itrace_do_parse_synth_opts(), non-'iy' options have their period type set to PERF_ITRACE_PERIOD_INSTRUCTIONS, but period remains zero. Since a zero period is meaningless for this type, default to 1 (one sample per instruction).

With this fix applied, the result is as follows:

Annotate type: 'struct mmu_gather_batch' in [kernel.kallsyms] (15 samples):
============================================================================

 Percent     offset       size  field
  100.00          0       0x10  struct mmu_gather_batch  {
   86.67          0        0x8      struct mmu_gather_batch*    next;
   13.33        0x8        0x4      unsigned int        nr;
    0.00        0xc        0x4      unsigned int        max;
    0.00       0x10          0      struct encoded_page*[]      encoded_pages;
                                };

Signed-off-by Tengda Wu <[email protected]>

---
 tools/perf/util/auxtrace.c | 6 ++++++
 1 file changed, 6 insertions(+)

tools/perf/util/auxtrace.c +6 -0

index 0b851f32e98c..415b68a2bba9 100644
@@ -1759,6 +1759,12 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
 			synth_opts->period = PERF_ITRACE_DEFAULT_PERIOD;
 	}
 
+	if (!period_set &&
+	    synth_opts->period_type == PERF_ITRACE_PERIOD_INSTRUCTIONS) {
+		/* Indicates a sample is taken for every instruction. */
+		synth_opts->period = 1;
+	}
+
 	return 0;
 
 out_err: