* [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-10 13:08 ` Shuai Xue
2026-08-08 12:23 ` [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction " Tengda Wu
` (21 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
The jump and adrp instructions parsed by libcapstone currently lack
symbolic representation and use a '#' prefix for addresses. This
format is inconsistent with objdump's output, which causes subsequent
parsing in jump__parse() and arm64_mov__parse() to fail.
Example mismatch:
Current: b #0xffff8000800114c8
Fix: b ffff8000800114c8 <el0t_64_sync+0x108>
Current: adrp x18, #0xffff800081f5f000
Fix: adrp x18, ffff800081f5f000 <this_cpu_vector>
Fix this by implementing extended formatting for these arm64
instructions during symbol__disassemble_capstone(). This ensures
the output matches objdump's expected style, including the raw
address and the associated <symbol+offset> suffix.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/capstone.c | 136 +++++++++++++++++++++++++++++++++----
tools/perf/util/disasm.c | 5 ++
tools/perf/util/disasm.h | 1 +
3 files changed, 130 insertions(+), 12 deletions(-)
diff --git a/tools/perf/util/capstone.c b/tools/perf/util/capstone.c
index 74213daf8786..fb8a2bc5558f 100644
--- a/tools/perf/util/capstone.c
+++ b/tools/perf/util/capstone.c
@@ -3,6 +3,7 @@
#include <errno.h>
#include <inttypes.h>
+#include <stdlib.h>
#include <string.h>
#include <dlfcn.h>
@@ -31,6 +32,10 @@
#define CS_MODE_RISCVC 4
#endif
+#if CS_VERSION_MAJOR < 4
+#define ARM64_GRP_BRANCH_RELATIVE 7
+#endif
+
#ifdef LIBCAPSTONE_DLOPEN
static void *perf_cs_dll_handle(void)
{
@@ -225,6 +230,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 (arch == CS_ARCH_ARM64) {
+ /*
+ * Same as x86: arm64 needs instruction details to resolve
+ * symbolic addresses.
+ */
+ perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
}
return 0;
@@ -299,10 +310,6 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
struct map *map = args->ms->map;
struct symbol *sym;
- /* TODO: support more architectures */
- if (!arch__is_x86(args->arch))
- return;
-
if (insn->detail == NULL)
return;
@@ -354,6 +361,116 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
}
}
+static int print_default_format(struct cs_insn *insn, char *buf, size_t len)
+{
+ return scnprintf(buf, len, " %-7s %s",
+ insn->mnemonic, insn->op_str);
+}
+
+static void format_capstone_insn_x86(struct cs_insn *insn, char *buf,
+ size_t len, struct annotate_args *args,
+ u64 addr)
+{
+ int printed;
+
+ printed = print_default_format(insn, buf, len);
+ buf += printed;
+ len -= printed;
+
+ print_capstone_detail(insn, buf, len, args, addr);
+}
+
+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 ||
+ insn->detail->groups[i] == ARM64_GRP_CALL ||
+ insn->detail->groups[i] == ARM64_GRP_BRANCH_RELATIVE)
+ return true;
+ }
+
+ return false;
+}
+
+static void format_capstone_insn_arm64(struct cs_insn *insn, char *buf,
+ size_t len, struct annotate_args *args)
+{
+ struct map *map = args->ms->map;
+ struct symbol *sym;
+ char *last_imm, *endptr;
+ u64 orig_addr, addr;
+ struct map *found_map = NULL;
+
+ print_default_format(insn, buf, len);
+ /*
+ * Adjust instructions to keep the existing behavior with objdump.
+ *
+ * Example conversion:
+ * From: b #0xffff8000800114c8
+ * To: b ffff8000800114c8 <el0t_64_sync+0x108>
+ */
+ 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);
+ if (endptr == last_imm + 1)
+ return;
+
+ addr = map__objdump_2mem(map, orig_addr);
+
+ /* Relocate map that contains the address */
+ if (dso__kernel(map__dso(map))) {
+ found_map = maps__find(map__kmaps(map), addr);
+ if (found_map == NULL)
+ return;
+ 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) {
+ map__put(found_map);
+ return;
+ }
+
+ /* Symbolize the resolved address */
+ len = len - (last_imm - buf);
+ if (addr == sym->start) {
+ scnprintf(last_imm, len, "%"PRIx64" <%s>",
+ orig_addr, sym->name);
+ } else {
+ scnprintf(last_imm, len, "%"PRIx64" <%s+%#"PRIx64">",
+ orig_addr, sym->name, addr - sym->start);
+ }
+ map__put(found_map);
+ }
+}
+
+static void format_capstone_insn(struct cs_insn *insn, char *buf, size_t len,
+ struct annotate_args *args, u64 addr)
+{
+ /* TODO: support more architectures */
+ if (arch__is_x86(args->arch))
+ format_capstone_insn_x86(insn, buf, len, args, addr);
+ else if (arch__is_arm64(args->arch))
+ format_capstone_insn_arm64(insn, buf, len, args);
+ else
+ print_default_format(insn, buf, len);
+}
+
struct find_file_offset_data {
u64 ip;
u64 offset;
@@ -446,14 +563,9 @@ int symbol__disassemble_capstone(const char *filename, struct symbol *sym,
free_count = count = perf_cs_disasm(handle, buf, buf_len, start, buf_len, &insn);
for (i = 0, offset = 0; i < count; i++) {
- int printed;
-
- printed = scnprintf(disasm_buf, sizeof(disasm_buf),
- " %-7s %s",
- insn[i].mnemonic, insn[i].op_str);
- print_capstone_detail(&insn[i], disasm_buf + printed,
- sizeof(disasm_buf) - printed, args,
- start + offset);
+ format_capstone_insn(&insn[i], disasm_buf,
+ sizeof(disasm_buf), args,
+ start + offset);
args->offset = offset;
args->line = disasm_buf;
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 6cfdbabbb8c7..0ba38f70fe1a 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -203,6 +203,11 @@ bool arch__is_powerpc(const struct arch *arch)
return arch->id.e_machine == EM_PPC || arch->id.e_machine == EM_PPC64;
}
+bool arch__is_arm64(const struct arch *arch)
+{
+ return arch->id.e_machine == EM_AARCH64;
+}
+
static void ins_ops__delete(struct ins_operands *ops)
{
if (ops == NULL)
diff --git a/tools/perf/util/disasm.h b/tools/perf/util/disasm.h
index 25756e3f47e4..06c664fd4fc2 100644
--- a/tools/perf/util/disasm.h
+++ b/tools/perf/util/disasm.h
@@ -111,6 +111,7 @@ struct annotate_args {
const struct arch *arch__find(uint16_t e_machine, uint32_t e_flags, const char *cpuid);
bool arch__is_x86(const struct arch *arch);
bool arch__is_powerpc(const struct arch *arch);
+bool arch__is_arm64(const struct arch *arch);
extern const struct ins_ops call_ops;
extern const struct ins_ops dec_ops;
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump
2026-08-08 12:23 ` [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump Tengda Wu
@ 2026-08-10 13:08 ` Shuai Xue
0 siblings, 0 replies; 42+ messages in thread
From: Shuai Xue @ 2026-08-10 13:08 UTC (permalink / raw)
To: Tengda Wu, Namhyung Kim, james.clark, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm
On 8/8/26 8:23 PM, Tengda Wu wrote:
> The jump and adrp instructions parsed by libcapstone currently lack
> symbolic representation and use a '#' prefix for addresses. This
> format is inconsistent with objdump's output, which causes subsequent
> parsing in jump__parse() and arm64_mov__parse() to fail.
>
> Example mismatch:
> Current: b #0xffff8000800114c8
> Fix: b ffff8000800114c8 <el0t_64_sync+0x108>
>
> Current: adrp x18, #0xffff800081f5f000
> Fix: adrp x18, ffff800081f5f000 <this_cpu_vector>
>
> Fix this by implementing extended formatting for these arm64
> instructions during symbol__disassemble_capstone(). This ensures
> the output matches objdump's expected style, including the raw
> address and the associated <symbol+offset> suffix.
>
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
> tools/perf/util/capstone.c | 136 +++++++++++++++++++++++++++++++++----
> tools/perf/util/disasm.c | 5 ++
> tools/perf/util/disasm.h | 1 +
> 3 files changed, 130 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/capstone.c b/tools/perf/util/capstone.c
> index 74213daf8786..fb8a2bc5558f 100644
> --- a/tools/perf/util/capstone.c
> +++ b/tools/perf/util/capstone.c
> @@ -3,6 +3,7 @@
>
> #include <errno.h>
> #include <inttypes.h>
> +#include <stdlib.h>
> #include <string.h>
>
> #include <dlfcn.h>
> @@ -31,6 +32,10 @@
> #define CS_MODE_RISCVC 4
> #endif
>
> +#if CS_VERSION_MAJOR < 4
> +#define ARM64_GRP_BRANCH_RELATIVE 7
Please add a comment explaining where '7' comes from
(CS_GRP_BRANCH_RELATIVE in capstone v3), otherwise it reads like an
arbitrary magic number.
> +#endif
> +
> #ifdef LIBCAPSTONE_DLOPEN
> static void *perf_cs_dll_handle(void)
> {
> @@ -225,6 +230,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 (arch == CS_ARCH_ARM64) {
> + /*
> + * Same as x86: arm64 needs instruction details to resolve
> + * symbolic addresses.
> + */
> + perf_cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
> }
>
> return 0;
> @@ -299,10 +310,6 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
> struct map *map = args->ms->map;
> struct symbol *sym;
>
> - /* TODO: support more architectures */
> - if (!arch__is_x86(args->arch))
> - return;
> -
> if (insn->detail == NULL)
> return;
>
> @@ -354,6 +361,116 @@ static void print_capstone_detail(struct cs_insn *insn, char *buf, size_t len,
> }
> }
>
> +static int print_default_format(struct cs_insn *insn, char *buf, size_t len)
> +{
> + return scnprintf(buf, len, " %-7s %s",
> + insn->mnemonic, insn->op_str);
> +}
> +
> +static void format_capstone_insn_x86(struct cs_insn *insn, char *buf,
> + size_t len, struct annotate_args *args,
> + u64 addr)
> +{
> + int printed;
> +
> + printed = print_default_format(insn, buf, len);
> + buf += printed;
> + len -= printed;
> +
> + print_capstone_detail(insn, buf, len, args, addr);
> +}
> +
> +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 ||
> + insn->detail->groups[i] == ARM64_GRP_CALL ||
> + insn->detail->groups[i] == ARM64_GRP_BRANCH_RELATIVE)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void format_capstone_insn_arm64(struct cs_insn *insn, char *buf,
> + size_t len, struct annotate_args *args)
> +{
> + struct map *map = args->ms->map;
> + struct symbol *sym;
> + char *last_imm, *endptr;
> + u64 orig_addr, addr;
> + struct map *found_map = NULL;
> +
> + print_default_format(insn, buf, len);
> + /*
> + * Adjust instructions to keep the existing behavior with objdump.
> + *
> + * Example conversion:
> + * From: b #0xffff8000800114c8
> + * To: b ffff8000800114c8 <el0t_64_sync+0x108>
> + */
> + 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);
> + if (endptr == last_imm + 1)
> + return;
> +
> + addr = map__objdump_2mem(map, orig_addr);
> +
> + /* Relocate map that contains the address */
> + if (dso__kernel(map__dso(map))) {
> + found_map = maps__find(map__kmaps(map), addr);
> + if (found_map == NULL)
> + return;
> + 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) {
> + map__put(found_map);
> + return;
> + }
> +
> + /* Symbolize the resolved address */
> + len = len - (last_imm - buf);
> + if (addr == sym->start) {
> + scnprintf(last_imm, len, "%"PRIx64" <%s>",
> + orig_addr, sym->name);
> + } else {
> + scnprintf(last_imm, len, "%"PRIx64" <%s+%#"PRIx64">",
> + orig_addr, sym->name, addr - sym->start);
> + }
> + map__put(found_map);
This whole sequence (objdump_2mem -> kmaps relocation -> map_ip ->
find_symbol -> scnprintf the "<sym+off>" string) is almost identical to
what print_capstone_detail() does for x86 RIP-relative operands. Could
you factor out a common helper so the two paths don't diverge over time?
Thanks.
Shuai
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction disassembly mismatch with objdump
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
2026-08-08 12:23 ` [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:03 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions Tengda Wu
` (20 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, 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 <wutengda@huaweicloud.com>
---
tools/perf/util/llvm.c | 50 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/llvm.c b/tools/perf/util/llvm.c
index a0deb742a733..6b53b283205a 100644
--- a/tools/perf/util/llvm.c
+++ b/tools/perf/util/llvm.c
@@ -9,8 +9,10 @@
#include "symbol.h"
#include <errno.h>
#include <fcntl.h>
+#include <string.h>
#include <unistd.h>
#include <linux/zalloc.h>
+#include <linux/bitops.h>
#ifdef HAVE_LIBLLVM_SUPPORT
#include "llvm-c-helpers.h"
@@ -108,6 +110,16 @@ 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;
+ 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);
+ adrp_imm = sign_extend64(adrp_imm, 20);
+
+ /* Calculate the target page address */
+ storage->pcrel_load_addr = (address & ~0xfffULL) + (adrp_imm << 12);
+ }
*ref_type = LLVMDisassembler_ReferenceType_InOut_None;
return NULL;
}
@@ -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;
+
+ s++;
+ *s = '\0';
+ disasm_len = strlen(disasm_buf);
+ disasm_len += scnprintf(disasm_buf + disasm_len,
+ sizeof(disasm_buf) - disasm_len,
+ " %"PRIx64,
+ storage.pcrel_load_addr);
+ } else {
+ /*
+ * For other archs, append the resolved address
+ * as an inline comment.
+ */
+ disasm_len += scnprintf(disasm_buf + disasm_len,
+ sizeof(disasm_buf) - disasm_len,
+ " # %#"PRIx64,
+ storage.pcrel_load_addr);
+ }
+
+ /* Append the resolved symbol name if available */
if (name) {
disasm_len += scnprintf(disasm_buf + disasm_len,
sizeof(disasm_buf) -
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction disassembly mismatch with objdump
2026-08-08 12:23 ` [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction " Tengda Wu
@ 2026-08-08 13:03 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:03 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
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 <wutengda@huaweicloud.com>
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/20260808122400.2961238-1-wutengda@huaweicloud.com?part=2
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
2026-08-08 12:23 ` [PATCH v4 01/23] perf capstone: Fix arm64 jump/adrp disassembly mismatch with objdump Tengda Wu
2026-08-08 12:23 ` [PATCH v4 02/23] perf llvm: Fix arm64 adrp instruction " Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:05 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions Tengda Wu
` (19 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
As the default parser for arm64 instructions, arm64_mov__parse()
currently only supports parsing instructions with the 'addr <symbol>'
suffix. Other instructions are not supported, causing a lack of
source and target information for most regular instructions.
Generalize arm64_mov__parse to extend its parsing capabilities:
1. Parse 'ops->target.raw' and 'ops->source.raw' by default.
2. Utilize arm64__check_multi_regs() to identify if source and
target contain multiple registers.
3. Maintain backward compatibility for instructions containing the
'addr <symbol>' suffix.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 158 ++++++++++++++----
1 file changed, 126 insertions(+), 32 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 33080fdca125..5e824fd617ee 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -3,6 +3,8 @@
#include <errno.h>
#include <stdlib.h>
#include <string.h>
+#include <linux/ctype.h>
+#include <linux/string.h>
#include <linux/zalloc.h>
#include <regex.h>
#include "../annotate.h"
@@ -14,59 +16,151 @@ struct arch_arm64 {
regex_t jump_insn;
};
-static int arm64_mov__parse(const struct arch *arch __maybe_unused,
+static bool arm64__is_reg(const char *op)
+{
+ if (!op || !*op)
+ return false;
+
+ /*
+ * General-purpose registers: x0-x30, w0-w30.
+ * Check for 'x' or 'w' prefix followed by a numeric index.
+ */
+ if ((op[0] == 'x' || op[0] == 'w') && isdigit(op[1]))
+ return true;
+
+ /* Special-purpose registers: sp. */
+ if (!strncmp(op, "sp", 2))
+ return true;
+
+ /* TODO: Support more registers. */
+ return false;
+}
+
+static bool arm64__check_multi_regs(const struct arch *arch, const char *op)
+{
+ const char *p = op;
+ int reg_count = 0;
+
+ while (p && *p) {
+ p = skip_spaces(p);
+ if (*p == arch->objdump.memory_ref_char)
+ p++;
+
+ if (arm64__is_reg(p))
+ reg_count++;
+
+ if (reg_count >= 2)
+ return true;
+
+ /* Move to next operand after comma */
+ p = strchr(p, ',');
+ if (p)
+ p++;
+ }
+
+ return false;
+}
+
+/*
+ * Duplicate @insn, stripping the comment and trailing whitespace.
+ * Returns a newly allocated string which the caller must free(),
+ * or NULL on allocation failure or if @insn is NULL.
+ */
+static char *rstrip_space_and_comment(const char *insn, char comment_char)
+{
+ const char *end, *comment;
+ size_t len;
+ char *result;
+
+ if (insn == NULL)
+ return NULL;
+
+ comment = strchr(insn, comment_char);
+ if (comment != NULL)
+ end = comment;
+ else
+ end = insn + strlen(insn);
+
+ while (end > insn && isspace(end[-1]))
+ --end;
+
+ len = end - insn;
+ result = malloc(len + 1);
+ if (result == NULL)
+ return NULL;
+
+ memcpy(result, insn, len);
+ result[len] = '\0';
+
+ return result;
+}
+
+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)
{
- char *s = strchr(ops->raw, ','), *target, *endptr;
+ char *s = strchr(ops->raw, ','), *source, *endptr;
if (s == NULL)
return -1;
+ /* Parse target */
*s = '\0';
- ops->source.raw = strdup(ops->raw);
+ ops->target.raw = strdup(ops->raw);
*s = ',';
- if (ops->source.raw == NULL)
- return -1;
-
- target = ++s;
- ops->target.raw = strdup(target);
if (ops->target.raw == NULL)
- goto out_free_source;
+ return -1;
- ops->target.addr = strtoull(target, &endptr, 16);
- if (endptr == target)
- goto out_free_target;
+ ops->target.multi_regs = arm64__check_multi_regs(arch, ops->target.raw);
- s = strchr(endptr, '<');
- if (s == NULL)
- goto out_free_target;
- endptr = strchr(s + 1, '>');
- if (endptr == NULL)
- goto out_free_target;
-
- *endptr = '\0';
- *s = ' ';
- ops->target.name = strdup(s);
- *s = '<';
- *endptr = '>';
- if (ops->target.name == NULL)
- goto out_free_target;
+ /* Parse source, stripping comment if present */
+ source = skip_spaces(++s);
+ ops->source.raw = rstrip_space_and_comment(source, arch->objdump.comment_char);
+ if (ops->source.raw == NULL) {
+ zfree(&ops->target.raw);
+ return -1;
+ }
+
+ ops->source.multi_regs = arm64__check_multi_regs(arch, ops->source.raw);
+
+ /* Parse 'addr <symbol>' from source (if any) */
+ ops->source.addr = strtoull(ops->source.raw, &endptr, 16);
+ if (endptr != ops->source.raw) {
+ s = strchr(endptr, '<');
+ if (s == NULL) {
+ ops->source.addr = 0;
+ return 0;
+ }
+ endptr = strrchr(s + 1, '>');
+ if (endptr == NULL) {
+ ops->source.addr = 0;
+ return 0;
+ }
+
+ *endptr = '\0';
+ ops->source.name = strdup(s + 1);
+ *endptr = '>';
+ if (ops->source.name == NULL) {
+ ops->source.addr = 0;
+ return 0;
+ }
+ }
return 0;
+}
-out_free_target:
- zfree(&ops->target.raw);
-out_free_source:
- zfree(&ops->source.raw);
- return -1;
+static int arm64_mov__scnprintf(const struct ins *ins, char *bf, size_t size,
+ struct ins_operands *ops, int max_ins_name)
+{
+ return scnprintf(bf, size, "%-*s %s, %s", max_ins_name, ins->name,
+ ops->target.raw, ops->source.name ?: ops->source.raw);
}
static const struct ins_ops arm64_mov_ops = {
.parse = arm64_mov__parse,
- .scnprintf = mov__scnprintf,
+ .scnprintf = arm64_mov__scnprintf,
};
static const struct ins_ops *arm64__associate_instruction_ops(struct arch *arch, const char *name)
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions
2026-08-08 12:23 ` [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions Tengda Wu
@ 2026-08-08 13:05 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:05 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] arch->objdump.memory_ref_char is uninitialized (0) for ARM64, causing arm64__check_multi_regs() to fail parsing memory operands and creating an out-of-bounds read risk.
--
--- Patch [3]: [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions ---
commit 0546b270cb844bdab4fcf529e313bb0ead4fc3f8
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Generalize arm64_mov__parse to support more instructions
As the default parser for arm64 instructions, arm64_mov__parse()
currently only supports parsing instructions with the 'addr <symbol>'
suffix. Other instructions are not supported, causing a lack of
source and target information for most regular instructions.
Generalize arm64_mov__parse to extend its parsing capabilities:
1. Parse 'ops->target.raw' and 'ops->source.raw' by default.
2. Utilize arm64__check_multi_regs() to identify if source and
target contain multiple registers.
3. Maintain backward compatibility for instructions containing the
'addr <symbol>' suffix.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 33080fdca125f..5e824fd617ee5 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> +static bool arm64__check_multi_regs(const struct arch *arch, const char *op)
> +{
> + const char *p = op;
> + int reg_count = 0;
> +
> + while (p && *p) {
> + p = skip_spaces(p);
> + if (*p == arch->objdump.memory_ref_char)
> + p++;
[Severity: Medium]
Could this result in an out-of-bounds read?
Since arch->objdump.memory_ref_char is not explicitly initialized for ARM64,
it defaults to 0. If the operand string contains trailing spaces,
skip_spaces() will advance p to the null terminator.
This would cause the following check to match:
if (*p == arch->objdump.memory_ref_char) /* 0 == 0 */
p++;
This increments p past the null terminator, and the subsequent call to
arm64__is_reg(p) would access unallocated memory:
static bool arm64__is_reg(const char *op)
{
...
if ((op[0] == 'x' || op[0] == 'w') && isdigit(op[1]))
...
Should memory_ref_char be initialized to '[' for ARM64 so that memory
operands are correctly parsed?
> +
> + if (arm64__is_reg(p))
> + reg_count++;
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=3
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (2 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 03/23] perf annotate-arm64: Generalize arm64_mov__parse to support more instructions Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:07 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64 Tengda Wu
` (18 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Add ldst_ops to handle load and store instructions in order to parse
the data types and offsets associated with PMU events for memory access
instructions. There are many variants of load and store instructions in
arm64, making it difficult to match all of these instruction names
completely. Therefore, only the instruction prefixes are matched. The
prefix 'ld|st' covers most of the memory access instructions, 'cas|swp'
matches atomic instructions, and 'prf' matches memory prefetch
instructions.
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 119 ++++++++++++++++++
1 file changed, 119 insertions(+)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 5e824fd617ee..32752a3bc756 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -14,6 +14,7 @@ struct arch_arm64 {
struct arch arch;
regex_t call_insn;
regex_t jump_insn;
+ regex_t ldst_insn; /* load and store instruction */
};
static bool arm64__is_reg(const char *op)
@@ -163,6 +164,108 @@ static const struct ins_ops arm64_mov_ops = {
.scnprintf = arm64_mov__scnprintf,
};
+static bool arm64__insn_is_source_on_left(struct disasm_line *dl)
+{
+ /*
+ * Store instructions invert the standard syntax by placing the source
+ * register before the destination memory address.
+ */
+ return !strncmp(dl->ins.name, "st", 2);
+}
+
+/*
+ * This function is used to parse arm64 load/store instructions into
+ * instruction operands.
+ *
+ * Typical instructions and their parsing logic:
+ *
+ * 1. Immediate offset:
+ * ldr x2, [x0] -> target="x2", source="[x0]"
+ * ldr x2, [x0, #24] -> target="x2", source="[x0, #24]"
+ * ldp x19, x20, [sp, #16] -> target="x19, x20", source="[sp, #16]"
+ *
+ * 2. Pre-index addressing:
+ * stp x29, x30, [sp, #-64]! -> target="[sp, #-64]!", source="x29, x30"
+ *
+ * 3. Post-index addressing:
+ * str x1, [x0], #8 -> target="[x0], #8", source="x1"
+ * ldr w1, [x21], #4 -> target="w1", source="[x21], #4"
+ * ldp x29, x30, [sp], #32 -> target="x29, x30", source="[sp], #32"
+ *
+ * 4. Register offset / extension:
+ * ldr x0, [x1, w0, sxtw #3] -> target="x0", source="[x1, w0, sxtw #3]"
+ * ldr x0, [x1, x0, lsl #3] -> target="x0", source="[x1, x0, lsl #3]"
+ *
+ * 5. Atomic operations:
+ * cas w3, w1, [x0] -> target="w3, w1", source="[x0]"
+ * swp x3, x0, [x2] -> target="x3, x0", source="[x2]"
+ *
+ * 6. Prefetch memory:
+ * prfm pstl1strm, [x4] -> target="pstl1strm", source="[x4]"
+ *
+ * 7. PC-relative loads (No bracket found):
+ * ldr x0, ffff800080f40c68 <__kvm_nvhe_$d> -> Fallback to default parser
+ *
+ * Parsing strategy:
+ * Use the '[' bracket as the boundary to split the operands into left
+ * and right sides. For non-store instructions, the left side is the
+ * target and the right side is the source. For store instructions, the
+ * roles are reversed.
+ */
+static int arm64_ldst__parse(const struct arch *arch,
+ struct ins_operands *ops,
+ struct map_symbol *ms,
+ struct disasm_line *dl)
+{
+ char *s, *left, *right;
+
+ right = s = strchr(ops->raw, arch->objdump.memory_ref_char);
+ if (!s) {
+ /* Fallback to default parser for PC-relative loads. */
+ return arm64_mov__parse(arch, ops, ms, dl);
+ }
+
+ while (s > ops->raw && *s != ',')
+ --s;
+
+ if (s == ops->raw)
+ return -1;
+
+ *s = '\0';
+ left = strdup(ops->raw);
+
+ *s = ',';
+ if (!left)
+ return -1;
+
+ right = rstrip_space_and_comment(right, arch->objdump.comment_char);
+ if (!right) {
+ zfree(&left);
+ return -1;
+ }
+
+ if (arm64__insn_is_source_on_left(dl)) {
+ ops->source.raw = left;
+ ops->source.mem_ref = false;
+ ops->target.raw = right;
+ ops->target.mem_ref = true;
+ } else {
+ ops->source.raw = right;
+ ops->source.mem_ref = true;
+ ops->target.raw = left;
+ ops->target.mem_ref = false;
+ }
+
+ ops->source.multi_regs = arm64__check_multi_regs(arch, ops->source.raw);
+ ops->target.multi_regs = arm64__check_multi_regs(arch, ops->target.raw);
+
+ return 0;
+}
+
+static struct ins_ops arm64_ldst_ops = {
+ .parse = arm64_ldst__parse,
+};
+
static const struct ins_ops *arm64__associate_instruction_ops(struct arch *arch, const char *name)
{
struct arch_arm64 *arm = container_of(arch, struct arch_arm64, arch);
@@ -173,6 +276,8 @@ static const struct ins_ops *arm64__associate_instruction_ops(struct arch *arch,
ops = &jump_ops;
else if (!regexec(&arm->call_insn, name, 2, match, 0))
ops = &call_ops;
+ else if (!regexec(&arm->ldst_insn, name, 2, match, 0))
+ ops = &arm64_ldst_ops;
else if (!strcmp(name, "ret"))
ops = &ret_ops;
else
@@ -197,6 +302,8 @@ const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
arch->id = *id;
arch->objdump.comment_char = '/';
arch->objdump.skip_functions_char = '+';
+ arch->objdump.memory_ref_char = '[';
+ arch->objdump.imm_char = '#';
arch->associate_instruction_ops = arm64__associate_instruction_ops;
/* bl, blr */
@@ -210,8 +317,20 @@ const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
if (err)
goto out_free_call;
+ /*
+ * The ARM64 architecture has many variants of load/store instructions.
+ * It is quite challenging to match all of them completely. Here, we
+ * only match the prefixes of these instructions.
+ */
+ err = regcomp(&arm->ldst_insn, "^(ld|st|cas|prf|swp)",
+ REG_EXTENDED);
+ if (err)
+ goto out_free_jump;
+
return arch;
+out_free_jump:
+ regfree(&arm->jump_insn);
out_free_call:
regfree(&arm->call_insn);
out_free_arm:
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions
2026-08-08 12:23 ` [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions Tengda Wu
@ 2026-08-08 13:07 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:07 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] PC-relative loads can be incorrectly parsed and truncated if their associated symbol name or trailing comment contains a bracket '[' (e.g., C++ operator[]).
- [Medium] The arm64_ldst_ops struct omits a .scnprintf callback, breaking UI formatting for PC-relative loads and discarding resolved symbols.
--
--- Patch [4]: [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions ---
commit e23857fdcfc409dfdff0cd84b9fe8f31c4283050
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Handle load and store instructions
Add ldst_ops to handle load and store instructions in order to parse
the data types and offsets associated with PMU events for memory access
instructions. Matches instruction prefixes to handle arm64 variants.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 5e824fd617ee5..32752a3bc756b 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> @@ -163,6 +164,108 @@ static const struct ins_ops arm64_mov_ops = {
> .scnprintf = arm64_mov__scnprintf,
> };
>
> +static bool arm64__insn_is_source_on_left(struct disasm_line *dl)
> +{
[ ... ]
> +static int arm64_ldst__parse(const struct arch *arch,
> + struct ins_operands *ops,
> + struct map_symbol *ms,
> + struct disasm_line *dl)
> +{
> + char *s, *left, *right;
> +
> + right = s = strchr(ops->raw, arch->objdump.memory_ref_char);
> + if (!s) {
> + /* Fallback to default parser for PC-relative loads. */
> + return arm64_mov__parse(arch, ops, ms, dl);
> + }
[Severity: Medium]
For PC-relative loads, if the associated symbol name contains a bracket '['
(such as a C++ operator[] or Objective-C method) or if there is an unstripped
comment containing a bracket, will this strchr() incorrectly match it instead
of falling back to arm64_mov__parse()?
If so, it appears the subsequent string manipulation would use this incorrect
bracket position, dropping the characters in between and potentially corrupting
the operands.
[ ... ]
> + ops->source.multi_regs = arm64__check_multi_regs(arch, ops->source.raw);
> + ops->target.multi_regs = arm64__check_multi_regs(arch, ops->target.raw);
> +
> + return 0;
> +}
> +
> +static struct ins_ops arm64_ldst_ops = {
> + .parse = arm64_ldst__parse,
> +};
[Severity: Medium]
Does omitting a .scnprintf callback here break UI formatting for PC-relative
loads?
Because this structure lacks .scnprintf, it will fall back to
ins__raw_scnprintf(). For PC-relative loads that successfully fall back to
arm64_mov__parse() and resolve the symbol into ops->source.name, the raw
formatter will ignore the resolved symbol and just print ops->raw. Could this
result in the UI discarding the resolved symbol and displaying raw hex
addresses instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=4
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (3 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 04/23] perf annotate-arm64: Handle load and store instructions Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:12 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() " Tengda Wu
` (17 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
The current arm64 DWARF register lookup relies on 'aarch64_regstr_tbl',
a static string table. While this works for kprobe-tracer where register
names start with '%', it is insufficient for parsing register numbers
directly from raw instructions (e.g., extracting '6' from 'x6' or 'w6')
during annotation.
Since get_dwarf_regnum() is currently used only by 'perf annotate' and
does not affect kprobe-tracer, replace the limited table-based lookup
with a programmatic implementation in __get_dwarf_regnum_arm64(). This
allows resolving arm64 register names (x0-x30, w0-w30, sp, etc.) directly
into their corresponding DWARF register numbers.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../util/dwarf-regs-arch/dwarf-regs-arm64.c | 22 +++++++++++++++++++
tools/perf/util/dwarf-regs.c | 2 +-
tools/perf/util/include/dwarf-regs.h | 1 +
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c b/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c
index 593ca7d4fccc..720c863d1869 100644
--- a/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c
+++ b/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c
@@ -1,5 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
#include <errno.h>
+#include <ctype.h>
+#include <stdlib.h>
+#include <string.h>
#include <dwarf-regs.h>
#include "../../../arch/arm64/include/uapi/asm/perf_regs.h"
@@ -10,3 +13,22 @@ int __get_dwarf_regnum_for_perf_regnum_arm64(int perf_regnum)
return perf_regnum;
}
+
+int __get_dwarf_regnum_arm64(const char *name)
+{
+ int reg;
+
+ if (!strcmp(name, "sp"))
+ return 31;
+
+ if (*name != 'x' && *name != 'w')
+ return -ENOENT;
+
+ name++;
+ if (!isdigit(*name))
+ return -ENOENT;
+
+ reg = strtol(name, NULL, 10);
+
+ return reg >= 0 && reg <= 30 ? reg : -ENOENT;
+}
diff --git a/tools/perf/util/dwarf-regs.c b/tools/perf/util/dwarf-regs.c
index 797f455eba0d..bacf5c13c3bc 100644
--- a/tools/perf/util/dwarf-regs.c
+++ b/tools/perf/util/dwarf-regs.c
@@ -114,7 +114,7 @@ int get_dwarf_regnum(const char *name, unsigned int machine, unsigned int flags)
reg = _get_dwarf_regnum(arm_regstr_tbl, name);
break;
case EM_AARCH64:
- reg = _get_dwarf_regnum(aarch64_regstr_tbl, name);
+ reg = __get_dwarf_regnum_arm64(name);
break;
case EM_CSKY:
reg = __get_csky_regnum(name, flags);
diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h
index 46a764cf322f..a25f038bbff2 100644
--- a/tools/perf/util/include/dwarf-regs.h
+++ b/tools/perf/util/include/dwarf-regs.h
@@ -105,6 +105,7 @@ int __get_dwarf_regnum_x86_64(const char *name);
int __get_dwarf_regnum_for_perf_regnum_i386(int perf_regnum);
int __get_dwarf_regnum_for_perf_regnum_x86_64(int perf_regnum);
+int __get_dwarf_regnum_arm64(const char *name);
int __get_dwarf_regnum_for_perf_regnum_arm(int perf_regnum);
int __get_dwarf_regnum_for_perf_regnum_arm64(int perf_regnum);
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64
2026-08-08 12:23 ` [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64 Tengda Wu
@ 2026-08-08 13:12 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:12 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The programmatic DWARF register lookup __get_dwarf_regnum_arm64() fails to support the standard AArch64 'lr' (link register) and 'fp' (frame pointer) aliases.
--
--- Patch [5]: [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64 ---
commit 7e82be98e18ee79f31ad73b727e21d33b0a6f30d
Author: Tengda Wu <wutengda@huaweicloud.com>
perf dwarf-regs: Adapt get_dwarf_regnum() for arm64
This patch replaces the static string table lookup for arm64 DWARF registers
with a programmatic implementation. This change allows resolving arm64
register names directly into their corresponding DWARF register numbers
during annotation parsing.
> diff --git a/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c b/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c
> index 593ca7d4fccca..720c863d18696 100644
> --- a/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c
> +++ b/tools/perf/util/dwarf-regs-arch/dwarf-regs-arm64.c
[ ... ]
> @@ -10,3 +13,22 @@ int __get_dwarf_regnum_for_perf_regnum_arm64(int perf_regnum)
>
> return perf_regnum;
> }
> +
> +int __get_dwarf_regnum_arm64(const char *name)
> +{
> + int reg;
> +
> + if (!strcmp(name, "sp"))
> + return 31;
> +
> + if (*name != 'x' && *name != 'w')
> + return -ENOENT;
[Severity: Medium]
Will this strict prefix check inadvertently reject standard AArch64 aliases
like lr and fp?
These architectural aliases are frequently emitted by objdump and were
previously handled by the old static table. When perf annotate attempts
to look up lr or fp, this programmatic DWARF register lookup in
__get_dwarf_regnum_arm64() will fail because the name does not start with
x or w.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=5
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() for arm64
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (4 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 05/23] perf dwarf-regs: Adapt get_dwarf_regnum() for arm64 Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:07 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing Tengda Wu
` (16 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Currently, arch__dwarf_regnum() assumes that all architectures use a
register prefix character (e.g., '%' for x86) defined by
arch->objdump.register_char, and uses it to match register names in
objdump output. However, this assumption does not hold for arm64,
where assembly syntax uses bare register names like 'x0', 'w1'
without any prefix.
As a result, arm64 builds may fail to correctly recognize register
names from objdump disassembly, leading to incomplete or incorrect
annotation output.
To address this:
- Make the register prefix check optional, allowing architectures
without a prefix character to be parsed correctly.
- Extend the delimiter set in strpbrk() to include the closing square
bracket ']'. In arm64 assembly, memory operands often use bracketed
syntax such as '[x1, #16]' or '[x2]'. Adding ']' ensures clean
extraction of register names like 'x2' without trailing characters.
- Remove the 'static' qualifier from arch__dwarf_regnum() so that it
can be reused by other architecture-specific profiling components
in future changes.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/annotate.c | 14 ++++++++------
tools/perf/util/annotate.h | 2 ++
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index df70e95a8470..9d8b4d6b859b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2472,21 +2472,23 @@ int annotate_check_args(void)
return 0;
}
-static int arch__dwarf_regnum(const struct arch *arch, const char *str)
+int arch__dwarf_regnum(const struct arch *arch, const char *str)
{
- const char *p;
+ const char *p = str;
char *regname, *q;
int reg;
- p = strchr(str, arch->objdump.register_char);
- if (p == NULL)
- return -1;
+ if (arch->objdump.register_char) {
+ p = strchr(str, arch->objdump.register_char);
+ if (p == NULL)
+ return -1;
+ }
regname = strdup(p);
if (regname == NULL)
return -1;
- q = strpbrk(regname, ",) ");
+ q = strpbrk(regname, ",)] ");
if (q)
*q = '\0';
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index fa08d09b80f7..11b6e4780c02 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -492,6 +492,8 @@ int annotate_parse_percent_type(const struct option *opt, const char *_str,
int annotate_check_args(void);
+int arch__dwarf_regnum(const struct arch *arch, const char *str);
+
/**
* struct annotated_op_loc - Location info of instruction operand
* @reg1: First register in the operand
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() for arm64
2026-08-08 12:23 ` [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() " Tengda Wu
@ 2026-08-08 13:07 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:07 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch's fix to enable arm64 register parsing is unreachable and ineffective because the callers still unconditionally require a register prefix.
- [Medium] The new optional prefix logic in `arch__dwarf_regnum()` fails to strip the opening bracket from memory operands.
--
--- Patch [6]: [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() for arm64 ---
commit 1ed2e0e5199a9033fdb64817a106aecaa0871796
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate: Adapt arch__dwarf_regnum() for arm64
This commit makes the register prefix check in arch__dwarf_regnum() optional
to support arm64 assembly syntax. It also extends the delimiter set in
strpbrk() to handle bracketed memory operands and removes the static
qualifier from arch__dwarf_regnum().
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index df70e95a84704..9d8b4d6b859b8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
[ ... ]
> @@ -2472,21 +2472,23 @@ int annotate_check_args(void)
> return 0;
> }
>
> -static int arch__dwarf_regnum(const struct arch *arch, const char *str)
> +int arch__dwarf_regnum(const struct arch *arch, const char *str)
[Severity: Medium]
Does this change fully enable arm64 register parsing?
Looking at the callers of arch__dwarf_regnum(), they appear to still
unconditionally enforce the presence of a register prefix character.
For example, extract_reg_offset() checks:
if (arch->objdump.register_char == 0)
return -1;
And annotate_get_insn_location() strictly requires a match:
if (*s == arch->objdump.register_char) {
op_loc->reg1 = arch__dwarf_regnum(arch, s);
}
Will these un-updated callers prevent arm64 registers from actually being
parsed?
> {
> - const char *p;
> + const char *p = str;
> char *regname, *q;
> int reg;
>
> - p = strchr(str, arch->objdump.register_char);
> - if (p == NULL)
> - return -1;
> + if (arch->objdump.register_char) {
> + p = strchr(str, arch->objdump.register_char);
> + if (p == NULL)
> + return -1;
> + }
>
> regname = strdup(p);
> if (regname == NULL)
> return -1;
>
> - q = strpbrk(regname, ",) ");
> + q = strpbrk(regname, ",)] ");
[Severity: Medium]
Since the prefix check is now optional, will p point to the opening bracket
for arm64 memory operands like [x2, #8]?
This strpbrk() strips the closing bracket ], but is there logic to strip the
leading [ before the string is passed to get_dwarf_regnum()? Can this pass
[x2 as the register name and cause it to be rejected?
> if (q)
> *q = '\0';
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=6
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (5 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 06/23] perf annotate: Adapt arch__dwarf_regnum() " Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:11 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 08/23] perf annotate-arm64: Implement extract_op_location() callback Tengda Wu
` (15 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Assembly syntax for operands varies significantly across different
architectures, which prevents the operand location (op_loc) parsing
logic in annotate_get_insn_location() from being directly reused.
To simplify the core logic and improve maintainability, move the
operand parsing inside the for_each_insn_op_loc loop into arch-specific
extract_op_location callbacks. This refactoring is primarily a cleanup,
but includes a minor x86-specific change: errors from extract_reg_offset()
are now propagated, so unrecognized operands are correctly rejected
instead of being silently ignored, , and testing confirms no functional
regression from this change.
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../util/annotate-arch/annotate-powerpc.c | 10 ++
tools/perf/util/annotate-arch/annotate-x86.c | 82 ++++++++++++++++
tools/perf/util/annotate.c | 94 ++-----------------
tools/perf/util/disasm.h | 4 +
4 files changed, 103 insertions(+), 87 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-powerpc.c b/tools/perf/util/annotate-arch/annotate-powerpc.c
index 218207b52581..8d0b8def5955 100644
--- a/tools/perf/util/annotate-arch/annotate-powerpc.c
+++ b/tools/perf/util/annotate-arch/annotate-powerpc.c
@@ -390,6 +390,15 @@ static void update_insn_state_powerpc(struct type_state *state,
}
#endif /* HAVE_LIBDW_SUPPORT */
+static int extract_op_location_powerpc(const struct arch *arch __maybe_unused,
+ struct disasm_line *dl,
+ const char *op_str __maybe_unused, int op_idx,
+ struct annotated_op_loc *op_loc)
+{
+ get_powerpc_regs(dl->raw.raw_insn, !op_idx, op_loc);
+ return 0;
+}
+
const struct arch *arch__new_powerpc(const struct e_machine_and_e_flags *id,
const char *cpuid __maybe_unused)
{
@@ -406,5 +415,6 @@ const struct arch *arch__new_powerpc(const struct e_machine_and_e_flags *id,
#ifdef HAVE_LIBDW_SUPPORT
arch->update_insn_state = update_insn_state_powerpc;
#endif
+ arch->extract_op_location = extract_op_location_powerpc;
return arch;
}
diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
index 7e6136536393..6044a68966f1 100644
--- a/tools/perf/util/annotate-arch/annotate-x86.c
+++ b/tools/perf/util/annotate-arch/annotate-x86.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
#include <string.h>
+#include <stdlib.h>
#include <linux/compiler.h>
+#include <linux/ctype.h>
#include <linux/zalloc.h>
#include <assert.h>
#include <inttypes.h>
@@ -809,6 +811,85 @@ 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).
+ * 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)
+{
+ char *p;
+
+ if (arch->objdump.register_char == 0)
+ return -1;
+
+ /*
+ * It should start from offset, but it's possible to skip 0
+ * in the asm. So 0(%rax) should be same as (%rax).
+ *
+ * However, it also start with a segment select register like
+ * %gs:0x18(%rbx). In that case it should skip the part.
+ */
+ if (*str == arch->objdump.register_char) {
+ /* FIXME: Handle other segment registers */
+ if (!strncmp(str, "%gs:", 4))
+ op_loc->segment = INSN_SEG_X86_GS;
+
+ while (*str && !isdigit(*str) &&
+ *str != arch->objdump.memory_ref_char)
+ str++;
+ }
+
+ op_loc->offset = strtol(str, &p, 0);
+ op_loc->reg1 = arch__dwarf_regnum(arch, p);
+ if (op_loc->reg1 == -1)
+ return -1;
+
+ /* Get the second register */
+ if (op_loc->multi_regs)
+ op_loc->reg2 = arch__dwarf_regnum(arch, p + 1);
+
+ return 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)
+{
+ 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);
+ }
+
+ /* 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);
+ if (p && p != op_str + 4)
+ op_loc->imm = true;
+ return 0;
+ }
+
+ if (*s == arch->objdump.register_char) {
+ op_loc->reg1 = arch__dwarf_regnum(arch, s);
+ } else if (*s == arch->objdump.imm_char) {
+ op_loc->offset = strtol(s + 1, &p, 0);
+ if (p && p != s + 1)
+ op_loc->imm = true;
+ }
+
+ return 0;
+}
+
const struct arch *arch__new_x86(const struct e_machine_and_e_flags *id, const char *cpuid)
{
struct arch *arch = zalloc(sizeof(*arch));
@@ -848,5 +929,6 @@ const struct arch *arch__new_x86(const struct e_machine_and_e_flags *id, const c
#ifdef HAVE_LIBDW_SUPPORT
arch->update_insn_state = update_insn_state_x86;
#endif
+ arch->extract_op_location = extract_op_location_x86;
return arch;
}
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 9d8b4d6b859b..20be609ebed2 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2497,51 +2497,6 @@ int arch__dwarf_regnum(const struct arch *arch, const char *str)
return reg;
}
-/*
- * 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)
-{
- char *p;
-
- if (arch->objdump.register_char == 0)
- return -1;
-
- /*
- * It should start from offset, but it's possible to skip 0
- * in the asm. So 0(%rax) should be same as (%rax).
- *
- * However, it also start with a segment select register like
- * %gs:0x18(%rbx). In that case it should skip the part.
- */
- if (*str == arch->objdump.register_char) {
- if (arch__is_x86(arch)) {
- /* FIXME: Handle other segment registers */
- if (!strncmp(str, "%gs:", 4))
- op_loc->segment = INSN_SEG_X86_GS;
- }
-
- while (*str && !isdigit(*str) &&
- *str != arch->objdump.memory_ref_char)
- str++;
- }
-
- op_loc->offset = strtol(str, &p, 0);
- op_loc->reg1 = arch__dwarf_regnum(arch, p);
- if (op_loc->reg1 == -1)
- return -1;
-
- /* Get the second register */
- if (op_loc->multi_regs)
- op_loc->reg2 = arch__dwarf_regnum(arch, p + 1);
-
- return 0;
-}
-
/**
* annotate_get_insn_location - Get location of instruction
* @arch: the architecture info
@@ -2572,6 +2527,7 @@ int annotate_get_insn_location(const struct arch *arch, struct disasm_line *dl,
struct ins_operands *ops;
struct annotated_op_loc *op_loc;
int i;
+ int ret;
if (ins__is_lock(&dl->ins))
ops = dl->ops.locked.ops;
@@ -2597,49 +2553,13 @@ int annotate_get_insn_location(const struct arch *arch, struct disasm_line *dl,
/* Invalidate the register by default */
op_loc->reg1 = -1;
op_loc->reg2 = -1;
+ op_loc->mem_ref = mem_ref;
+ op_loc->multi_regs = multi_regs;
- if (insn_str == NULL) {
- if (!arch__is_powerpc(arch))
- continue;
- }
-
- /*
- * For powerpc, call get_powerpc_regs function which extracts the
- * required fields for op_loc, ie reg1, reg2, offset from the
- * raw instruction.
- */
- if (arch__is_powerpc(arch)) {
- op_loc->mem_ref = mem_ref;
- op_loc->multi_regs = multi_regs;
- get_powerpc_regs(dl->raw.raw_insn, !i, op_loc);
- } 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 {
- const char *s = insn_str;
- char *p = NULL;
-
- if (arch__is_x86(arch)) {
- /* FIXME: Handle other segment registers */
- if (!strncmp(insn_str, "%gs:", 4)) {
- op_loc->segment = INSN_SEG_X86_GS;
- op_loc->offset = strtol(insn_str + 4,
- &p, 0);
- if (p && p != insn_str + 4)
- op_loc->imm = true;
- continue;
- }
- }
-
- if (*s == arch->objdump.register_char) {
- op_loc->reg1 = arch__dwarf_regnum(arch, s);
- }
- else if (*s == arch->objdump.imm_char) {
- op_loc->offset = strtol(s + 1, &p, 0);
- if (p && p != s + 1)
- op_loc->imm = true;
- }
+ if (arch->extract_op_location) {
+ ret = arch->extract_op_location(arch, dl, insn_str, i, op_loc);
+ if (ret)
+ return ret;
}
}
diff --git a/tools/perf/util/disasm.h b/tools/perf/util/disasm.h
index 06c664fd4fc2..8796f92769f9 100644
--- a/tools/perf/util/disasm.h
+++ b/tools/perf/util/disasm.h
@@ -16,6 +16,7 @@ struct symbol;
struct data_loc_info;
struct type_state;
struct disasm_line;
+struct annotated_op_loc;
struct e_machine_and_e_flags {
uint32_t e_flags;
@@ -49,6 +50,9 @@ struct arch {
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl);
#endif
+ int (*extract_op_location)(const struct arch *arch, struct disasm_line *dl,
+ const char *op_str, int op_idx,
+ struct annotated_op_loc *op_loc);
};
struct ins {
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing
2026-08-08 12:23 ` [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing Tengda Wu
@ 2026-08-08 13:11 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:11 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Propagating errors from extract_reg_offset aborts instruction state processing, breaking register invalidation, and uses a flawed condition.
--
--- Patch [7]: [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing ---
commit ea4cc06c47d3d877b7277db8368241769c5b60d3
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate: Introduce extract_op_location callback for arch-specific parsing
Assembly syntax for operands varies significantly across different
architectures, which prevents the operand location (op_loc) parsing
logic in annotate_get_insn_location() from being directly reused.
To simplify the core logic and improve maintainability, move the
operand parsing inside the for_each_insn_op_loc loop into arch-specific
extract_op_location callbacks.
> This refactoring is primarily a cleanup,
> but includes a minor x86-specific change: errors from extract_reg_offset()
> are now propagated, so unrecognized operands are correctly rejected
> instead of being silently ignored, , and testing confirms no functional
> regression from this change.
[Severity: Medium]
Are we certain this change does not introduce functional regressions?
Propagating errors out of extract_reg_offset() appears to break type state
tracking for absolute memory references.
> diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> index 7e61365363938..6044a68966f14 100644
> --- a/tools/perf/util/annotate-arch/annotate-x86.c
> +++ b/tools/perf/util/annotate-arch/annotate-x86.c
[ ... ]
> @@ -809,6 +811,85 @@ static void update_insn_state_x86(struct type_state *state,
[ ... ]
> +static int extract_reg_offset(const struct arch *arch, const char *str,
> + struct annotated_op_loc *op_loc)
> +{
[ ... ]
> + op_loc->reg1 = arch__dwarf_regnum(arch, p);
> + if (op_loc->reg1 == -1)
> + return -1;
[Severity: Medium]
Will this error check correctly identify unrecognized registers?
If an instruction contains an unrecognized register like '%zmm0',
arch__dwarf_regnum() returns -2 (-ENOENT). The check for -1 will miss
unrecognized registers, but it will trigger for absolute memory references
like 'mov (0x1234), %rax' which lack a '%' character entirely.
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 9d8b4d6b859b8..20be609ebed24 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
[ ... ]
> @@ -2572,6 +2527,7 @@ int annotate_get_insn_location(const struct arch *arch, struct disasm_line *dl,
[ ... ]
> + if (arch->extract_op_location) {
> + ret = arch->extract_op_location(arch, dl, insn_str, i, op_loc);
> + if (ret)
> + return ret;
> }
> }
[Severity: Medium]
Does propagating this failure cause issues in update_insn_state_x86()?
When absolute memory references return -1 and trigger this early return,
annotate_get_insn_location() will fail.
If update_insn_state_x86() bails out early because of this failure:
tools/perf/util/annotate-arch/annotate-x86.c:update_insn_state_x86() {
...
if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0)
return;
...
}
Won't this bypass the fallback logic that invalidates the destination register?
This could cause the analyzer to incorrectly assume the destination register
retains its previous type state, when it actually contains unknown data.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=7
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 08/23] perf annotate-arm64: Implement extract_op_location() callback
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (6 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 07/23] perf annotate: Introduce extract_op_location callback for arch-specific parsing Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling Tengda Wu
` (14 subsequent siblings)
22 siblings, 0 replies; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Implement the extract_op_location() callback for the arm64 architecture
to handle its specific assembly syntax and addressing modes.
The extractor handles:
1. Standalone immediate operands (e.g., #0x10).
2. Memory references with diverse addressing modes:
- Signed offset: [base{, #imm|reg}]
- Pre-index: [base, #imm]!
- Post-index: [base], #imm|reg
3. Multi-register operands and primary/secondary register extraction.
With this callback in place, 'perf annotate' can now resolve memory
locations and register types on arm64, marking the initial enablement
of data type profiling for this architecture.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 67 +++++++++++++++++++
tools/perf/util/annotate.h | 10 +++
2 files changed, 77 insertions(+)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 32752a3bc756..6b4f1228afa9 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -287,6 +287,72 @@ static const struct ins_ops *arm64__associate_instruction_ops(struct arch *arch,
return ops;
}
+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;
+
+ /* Handle standalone immediate operands (e.g., #0x10) */
+ if (*s == arch->objdump.imm_char) {
+ op_loc->offset = strtol(s + 1, &p, 0);
+ if (p && p != s + 1)
+ op_loc->imm = true;
+ return 0;
+ }
+
+ /*
+ * Handle memory references, identify arm64 specific addressing modes.
+ * Reference: Arm Architecture Reference Manual
+ * (DDI 0487), Chapter C1.3.3: Load/store addressing modes.
+ */
+ if (*s == arch->objdump.memory_ref_char) {
+ op_loc->mem_ref = true;
+
+ 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|reg */
+ else if (p[1] == ',' &&
+ (strchr(p + 1, arch->objdump.imm_char) ||
+ arm64__is_reg(skip_spaces(p + 2))))
+ op_loc->addr_mode = PERF_ADDR_MODE_POST_INDEX;
+ /* Signed offset: [base{, #imm|reg}] */
+ else
+ op_loc->addr_mode = PERF_ADDR_MODE_SIGNED_OFFSET;
+
+ s++;
+ }
+
+ /* Extract the primary register */
+ op_loc->reg1 = arch__dwarf_regnum(arch, s);
+ if (op_loc->reg1 == -1)
+ return -1;
+
+ /* Move to the next symbol of the operand, if any */
+ s = strchr(s, ',');
+ if (s == NULL)
+ return 0;
+ s = skip_spaces(s + 1);
+
+ /* Extract secondary register or immediate offset */
+ if (op_loc->multi_regs)
+ op_loc->reg2 = arch__dwarf_regnum(arch, s);
+ else if (*s == arch->objdump.imm_char)
+ op_loc->offset = strtol(s + 1, &p, 0);
+
+ return 0;
+}
+
const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
const char *cpuid __maybe_unused)
{
@@ -305,6 +371,7 @@ const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
arch->objdump.memory_ref_char = '[';
arch->objdump.imm_char = '#';
arch->associate_instruction_ops = arm64__associate_instruction_ops;
+ arch->extract_op_location = extract_op_location_arm64;
/* bl, blr */
err = regcomp(&arm->call_insn, "^blr?$", REG_EXTENDED);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 11b6e4780c02..a44c933f55f5 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -494,12 +494,21 @@ int annotate_check_args(void);
int arch__dwarf_regnum(const struct arch *arch, const char *str);
+enum annotated_addr_mode {
+ PERF_ADDR_MODE_NONE = 0,
+
+ PERF_ADDR_MODE_SIGNED_OFFSET,
+ PERF_ADDR_MODE_PRE_INDEX,
+ PERF_ADDR_MODE_POST_INDEX,
+};
+
/**
* struct annotated_op_loc - Location info of instruction operand
* @reg1: First register in the operand
* @reg2: Second register in the operand
* @offset: Memory access offset in the operand
* @segment: Segment selector register
+ * @addr_mode: Addressing mode, only valid if @mem_ref is true
* @mem_ref: Whether the operand accesses memory
* @multi_regs: Whether the second register is used
* @imm: Whether the operand is an immediate value (in offset)
@@ -509,6 +518,7 @@ struct annotated_op_loc {
int reg2;
int offset;
u8 segment;
+ u8 addr_mode;
bool mem_ref;
bool multi_regs;
bool imm;
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (7 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 08/23] perf annotate-arm64: Implement extract_op_location() callback Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-10 6:57 ` Adrian Hunter
2026-08-08 12:23 ` [PATCH v4 10/23] perf arm-spe: Set default synthesized event period to 1 Tengda Wu
` (13 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
When data type profiling is enabled on ARM SPE, multiple overlapping
events (e.g., l1d-miss, tlb-access) are synthesized for a single sampled
instruction, as shown below:
Available samples
0 arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=30/
0 dummy:u
84K l1d-miss
95K l1d-access
77K llc-miss
58K llc-access
9K tlb-miss
108K tlb-access
0 branch
13K remote-access
108K memory
108K instructions
While 'perf report' provides an interactive menu for users to select a
specific event to prevent duplicate counting, 'perf annotate' lacks such
a mechanism. Consequently, it counts all instructions across these
overlapping events, which inflates the profile and distorts the data
type statistics.
Although using the '--itrace' option can work around this issue (e.g.:
perf annotate --data-type --stdio --itrace=i1i), it is inconvenient for
users to specify this explicitly every time.
To address this, introduce itrace_synth_opts.dont_overlap. Set this to true
when data type profiling is enabled and the user has not explicitly
specified an itrace option. Then, during arm_spe_process_auxtrace_info(),
adjust the synthesized event options based on the dont_overlap value to
only enable instruction event synthesis, thereby achieving automatic
deduplication.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/builtin-annotate.c | 8 ++++++++
tools/perf/util/arm-spe.c | 17 +++++++++++++++++
tools/perf/util/auxtrace.h | 2 ++
3 files changed, 27 insertions(+)
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 69cb72b2082a..616f54bf4868 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -873,6 +873,14 @@ int cmd_annotate(int argc, const char **argv)
annotate.session = perf_session__new(&data, &annotate.tool);
if (IS_ERR(annotate.session))
return PTR_ERR(annotate.session);
+ /*
+ * Hardware tracing (e.g.: ARM SPE) may generate overlapping events
+ * per instruction. When data type profiling is enabled, enable
+ * dont_overlap to deduplicate them to avoid skewed stats, but only
+ * if user hasn't specified itrace options (respect user override).
+ */
+ if (annotate.data_type && !itrace_synth_opts.set)
+ itrace_synth_opts.dont_overlap = true;
annotate.session->itrace_synth_opts = &itrace_synth_opts;
diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 401aab529309..1721882423f6 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -2033,6 +2033,9 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
/* Default nanoseconds period not supported */
spe->synth_opts.period_type = PERF_ITRACE_PERIOD_INSTRUCTIONS;
spe->synth_opts.period = 1;
+
+ if (session->itrace_synth_opts)
+ spe->synth_opts.dont_overlap = session->itrace_synth_opts->dont_overlap;
}
if (spe->synth_opts.period_type != PERF_ITRACE_PERIOD_INSTRUCTIONS) {
@@ -2044,6 +2047,20 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
ui__warning("Arm SPE has a hardware-based sampling period.\n\n"
"--itrace periods > 1i downsample by an interval of n SPE samples rather than n instructions.\n");
+ if (spe->synth_opts.dont_overlap) {
+ /*
+ * The 'instructions' event is the most comprehensive,
+ * synthesize it exclusively.
+ */
+ spe->synth_opts.flc = false;
+ spe->synth_opts.llc = false;
+ spe->synth_opts.tlb = false;
+ spe->synth_opts.branches = false;
+ spe->synth_opts.remote_access = false;
+ spe->synth_opts.mem = false;
+ spe->synth_opts.instructions = true;
+ }
+
err = arm_spe_synth_events(spe, session);
if (err)
goto err_free_queues;
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 6947f3f284c0..ebb4e9e8e574 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -80,6 +80,7 @@ enum itrace_period_type {
* @intr_events: whether to synthesize interrupt events
* @errors: whether to synthesize decoder error events
* @dont_decode: whether to skip decoding entirely
+ * @dont_overlap: whether to deduplicate overlapping events
* @log: write a decoding log
* @calls: limit branch samples to calls (can be combined with @returns)
* @returns: limit branch samples to returns (can be combined with @calls)
@@ -128,6 +129,7 @@ struct itrace_synth_opts {
bool intr_events;
bool errors;
bool dont_decode;
+ bool dont_overlap;
bool log;
bool calls;
bool returns;
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling
2026-08-08 12:23 ` [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling Tengda Wu
@ 2026-08-10 6:57 ` Adrian Hunter
0 siblings, 0 replies; 42+ messages in thread
From: Adrian Hunter @ 2026-08-10 6:57 UTC (permalink / raw)
To: Tengda Wu, Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Zecheng Li,
linux-perf-users, linux-kernel, llvm
On 08/08/2026 15:23, Tengda Wu wrote:
> When data type profiling is enabled on ARM SPE, multiple overlapping
> events (e.g., l1d-miss, tlb-access) are synthesized for a single sampled
> instruction, as shown below:
>
> Available samples
> 0 arm_spe_0/ts_enable=1,pa_enable=1,load_filter=1,store_filter=1,min_latency=30/
> 0 dummy:u
> 84K l1d-miss
> 95K l1d-access
> 77K llc-miss
> 58K llc-access
> 9K tlb-miss
> 108K tlb-access
> 0 branch
> 13K remote-access
> 108K memory
> 108K instructions
>
> While 'perf report' provides an interactive menu for users to select a
> specific event to prevent duplicate counting, 'perf annotate' lacks such
> a mechanism. Consequently, it counts all instructions across these
> overlapping events, which inflates the profile and distorts the data
> type statistics.
>
> Although using the '--itrace' option can work around this issue (e.g.:
> perf annotate --data-type --stdio --itrace=i1i), it is inconvenient for
> users to specify this explicitly every time.
>
> To address this, introduce itrace_synth_opts.dont_overlap. Set this to true
> when data type profiling is enabled and the user has not explicitly
> specified an itrace option. Then, during arm_spe_process_auxtrace_info(),
> adjust the synthesized event options based on the dont_overlap value to
> only enable instruction event synthesis, thereby achieving automatic
> deduplication.
>
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
> tools/perf/builtin-annotate.c | 8 ++++++++
> tools/perf/util/arm-spe.c | 17 +++++++++++++++++
> tools/perf/util/auxtrace.h | 2 ++
> 3 files changed, 27 insertions(+)
>
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 69cb72b2082a..616f54bf4868 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -873,6 +873,14 @@ int cmd_annotate(int argc, const char **argv)
> annotate.session = perf_session__new(&data, &annotate.tool);
> if (IS_ERR(annotate.session))
> return PTR_ERR(annotate.session);
> + /*
> + * Hardware tracing (e.g.: ARM SPE) may generate overlapping events
> + * per instruction. When data type profiling is enabled, enable
> + * dont_overlap to deduplicate them to avoid skewed stats, but only
> + * if user hasn't specified itrace options (respect user override).
> + */
> + if (annotate.data_type && !itrace_synth_opts.set)
> + itrace_synth_opts.dont_overlap = true;
>
> annotate.session->itrace_synth_opts = &itrace_synth_opts;
>
> diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
> index 401aab529309..1721882423f6 100644
> --- a/tools/perf/util/arm-spe.c
> +++ b/tools/perf/util/arm-spe.c
> @@ -2033,6 +2033,9 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
> /* Default nanoseconds period not supported */
> spe->synth_opts.period_type = PERF_ITRACE_PERIOD_INSTRUCTIONS;
> spe->synth_opts.period = 1;
> +
> + if (session->itrace_synth_opts)
> + spe->synth_opts.dont_overlap = session->itrace_synth_opts->dont_overlap;
> }
>
> if (spe->synth_opts.period_type != PERF_ITRACE_PERIOD_INSTRUCTIONS) {
> @@ -2044,6 +2047,20 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
> ui__warning("Arm SPE has a hardware-based sampling period.\n\n"
> "--itrace periods > 1i downsample by an interval of n SPE samples rather than n instructions.\n");
>
> + if (spe->synth_opts.dont_overlap) {
> + /*
> + * The 'instructions' event is the most comprehensive,
> + * synthesize it exclusively.
> + */
> + spe->synth_opts.flc = false;
> + spe->synth_opts.llc = false;
> + spe->synth_opts.tlb = false;
> + spe->synth_opts.branches = false;
> + spe->synth_opts.remote_access = false;
> + spe->synth_opts.mem = false;
> + spe->synth_opts.instructions = true;
> + }
> +
> err = arm_spe_synth_events(spe, session);
> if (err)
> goto err_free_queues;
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 6947f3f284c0..ebb4e9e8e574 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -80,6 +80,7 @@ enum itrace_period_type {
> * @intr_events: whether to synthesize interrupt events
> * @errors: whether to synthesize decoder error events
> * @dont_decode: whether to skip decoding entirely
> + * @dont_overlap: whether to deduplicate overlapping events
This doesn't seem to match what you are actually doing, which
seems to be to choose particular default itrace options for
'perf annotate --data-type'
I wonder if this should really be handled by
itrace_synth_opts__set_default()?
> * @log: write a decoding log
> * @calls: limit branch samples to calls (can be combined with @returns)
> * @returns: limit branch samples to returns (can be combined with @calls)
> @@ -128,6 +129,7 @@ struct itrace_synth_opts {
> bool intr_events;
> bool errors;
> bool dont_decode;
> + bool dont_overlap;
> bool log;
> bool calls;
> bool returns;
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/23] perf arm-spe: Set default synthesized event period to 1
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (8 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 09/23] perf annotate: Deduplicate overlapping ARM SPE events for data type profiling Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 11/23] perf annotate-data: Extract invalidate_reg_state() as a common helper Tengda Wu
` (12 subsequent siblings)
22 siblings, 0 replies; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, 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 (show-nr-samples) 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 --itrace is used with non-i/y options
(e.g., --itrace=M), the period remains zero after parsing. During
subsequent propagation, the final percentage calculation results
in zero:
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;
Fix this by defaulting the period to 1 in the ARM SPE auxtrace info
processing path if it is not explicitly specified. This ensures that
downstream tool components can correctly accumulate periods and compute
event percentages for all synthesized SPE events.
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 <wutengda@huaweicloud.com>
---
tools/perf/util/arm-spe.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 1721882423f6..0f656398f2a8 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -2043,6 +2043,17 @@ int arm_spe_process_auxtrace_info(union perf_event *event,
err = -EINVAL;
goto err_free_queues;
}
+
+ /*
+ * When --itrace is used with non-i/y options (e.g., --itrace=M),
+ * the period remains 0 because the parser does not apply a default
+ * for those paths. However, synthesized SPE events such as memory
+ * accesses, TLB walks, and cache misses still require a valid
+ * sample->period to correctly accumulate periods and compute event
+ * percentages. Set it to 1 to ensure proper accounting.
+ */
+ spe->synth_opts.period = spe->synth_opts.period ?: 1;
+
if (spe->synth_opts.period > 1)
ui__warning("Arm SPE has a hardware-based sampling period.\n\n"
"--itrace periods > 1i downsample by an interval of n SPE samples rather than n instructions.\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v4 11/23] perf annotate-data: Extract invalidate_reg_state() as a common helper
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (9 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 10/23] perf arm-spe: Set default synthesized event period to 1 Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support Tengda Wu
` (11 subsequent siblings)
22 siblings, 0 replies; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
The invalidate_reg_state() function is currently static and only used
within the x86 data type profiling implementation. To prepare for adding
arm64 instruction tracking support, extract it as a common helper that
can be shared across architectures.
No functional change intended.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/annotate-arch/annotate-x86.c | 9 ---------
tools/perf/util/annotate-data.c | 9 +++++++++
tools/perf/util/annotate-data.h | 1 +
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
index 6044a68966f1..ee4e3e7f3209 100644
--- a/tools/perf/util/annotate-arch/annotate-x86.c
+++ b/tools/perf/util/annotate-arch/annotate-x86.c
@@ -207,15 +207,6 @@ static int x86__cpuid_parse(struct arch *arch, const char *cpuid)
}
#ifdef HAVE_LIBDW_SUPPORT
-static void invalidate_reg_state(struct type_state_reg *reg)
-{
- reg->kind = TSR_KIND_INVALID;
- reg->ok = false;
- reg->lifetime_active = false;
- reg->lifetime_end = 0;
- reg->copied_from = -1;
-}
-
static void update_insn_state_x86(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 4e4c58764082..104b80d471f1 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -638,6 +638,15 @@ struct type_state_stack *findnew_stack_state(struct type_state *state,
return stack;
}
+void invalidate_reg_state(struct type_state_reg *reg)
+{
+ reg->kind = TSR_KIND_INVALID;
+ reg->ok = false;
+ reg->lifetime_active = false;
+ reg->lifetime_end = 0;
+ reg->copied_from = -1;
+}
+
/* Maintain a cache for quick global variable lookup */
struct global_var_entry {
struct rb_node node;
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index c26130744260..453e13bbe3e2 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -258,6 +258,7 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
Dwarf_Die *type_die, int ptr_offset);
struct type_state_stack *find_stack_state(struct type_state *state,
int offset);
+void invalidate_reg_state(struct type_state_reg *reg);
bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
u64 ip, u64 var_addr, int *var_offset,
Dwarf_Die *type_die);
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (10 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 11/23] perf annotate-data: Extract invalidate_reg_state() as a common helper Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:22 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions Tengda Wu
` (10 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Enable basic instruction tracking for arm64 by implementing three
essential functions in the find_data_type_block() call path:
find_data_type_block
-> arch_supports_insn_tracking (1)
-> find_data_type_insn
-> init_type_state (2)
-> update_var_state
-> update_insn_state (3)
Changes:
* arch_supports_insn_tracking(): add arm64 to the list of supported
architectures, allowing find_data_type_block() to proceed with data
type analysis.
* init_type_state(): correctly identify ret_reg, stack_reg, and
caller-saved registers for arm64 during type state initialization.
* update_insn_state(): add the update_insn_state_arm64() callback with
conservative register invalidation for call instructions and those with
destination register(s), preventing stale type propagation. Full
instruction-level analysis support will be added incrementally in
later patches.
With these changes, arm64 gains support for basic variable type
inference during instruction tracking.
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 66 +++++++++++++++++++
tools/perf/util/annotate-data.c | 11 +++-
2 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 6b4f1228afa9..44daa0176e51 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -9,6 +9,10 @@
#include <regex.h>
#include "../annotate.h"
#include "../disasm.h"
+#include "../annotate-data.h"
+#include "../debug.h"
+#include "../map.h"
+#include "../symbol.h"
struct arch_arm64 {
struct arch arch;
@@ -353,6 +357,65 @@ static int extract_op_location_arm64(const struct arch *arch,
return 0;
}
+#ifdef HAVE_LIBDW_SUPPORT
+static void update_insn_state_arm64(struct type_state *state,
+ struct data_loc_info *dloc, Dwarf_Die *cu_die __maybe_unused,
+ struct disasm_line *dl)
+{
+ struct annotated_insn_loc loc;
+ struct annotated_op_loc *dst = &loc.ops[INSN_OP_TARGET];
+ u32 insn_offset = dl->al.offset;
+
+ if (annotate_get_insn_location(dloc->arch, dl, &loc) < 0)
+ return;
+
+ /*
+ * Invalidate caller-saved registers on function calls per ARM64 AAPCS64
+ * ABI, unless DWARF location info indicates the register remains valid
+ * beyond the call address.
+ */
+ if (ins__is_call(&dl->ins)) {
+ struct symbol *func = dl->ops.target.sym;
+ const char *call_name;
+ u64 call_addr;
+
+ call_name = func ? func->name : dl->ops.target.name;
+ pr_debug_dtp("call [%x] %s\n", insn_offset, call_name ?: "<unknown>");
+
+ /* Invalidate caller-saved registers after call */
+ call_addr = map__rip_2objdump(dloc->ms->map,
+ dloc->ms->sym->start + dl->al.offset);
+ for (unsigned int i = 0; i < ARRAY_SIZE(state->regs); i++) {
+ struct type_state_reg *reg = &state->regs[i];
+
+ if (!reg->caller_saved)
+ continue;
+ /* Keep register valid within DWARF location lifetime */
+ if (reg->lifetime_active && call_addr < reg->lifetime_end)
+ continue;
+ invalidate_reg_state(reg);
+ }
+ return;
+ }
+
+ /*
+ * Invalidate destination register(s) for unsupported instructions to
+ * prevent stale type info from propagating to subsequent instructions.
+ */
+ if (has_reg_type(state, dst->reg1)) {
+ pr_debug_dtp("%s [%x] invalidate reg%d",
+ dl->ins.name, insn_offset, dst->reg1);
+ invalidate_reg_state(&state->regs[dst->reg1]);
+ if (dst->multi_regs) {
+ pr_debug_dtp(" and reg%d", dst->reg2);
+ invalidate_reg_state(&state->regs[dst->reg2]);
+ }
+ pr_debug_dtp("\n");
+ return;
+ }
+}
+#endif
+
const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
const char *cpuid __maybe_unused)
{
@@ -372,6 +435,9 @@ const struct arch *arch__new_arm64(const struct e_machine_and_e_flags *id,
arch->objdump.imm_char = '#';
arch->associate_instruction_ops = arm64__associate_instruction_ops;
arch->extract_op_location = extract_op_location_arm64;
+#ifdef HAVE_LIBDW_SUPPORT
+ arch->update_insn_state = update_insn_state_arm64;
+#endif
/* bl, blr */
err = regcomp(&arm->call_insn, "^blr?$", REG_EXTENDED);
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 104b80d471f1..c51b20a7af9b 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -28,6 +28,7 @@
/* register number of the stack pointer */
#define X86_REG_SP 7
+#define ARM64_REG_SP 31
static void delete_var_types(struct die_var_type *var_types);
@@ -178,6 +179,13 @@ static void init_type_state(struct type_state *state, const struct arch *arch)
state->regs[11].caller_saved = true;
state->ret_reg = 0;
state->stack_reg = X86_REG_SP;
+ } else if (arch__is_arm64(arch)) {
+ int i;
+
+ for (i = 0; i < 18; i++)
+ state->regs[i].caller_saved = true;
+ state->ret_reg = 0;
+ state->stack_reg = ARM64_REG_SP;
}
}
@@ -1437,7 +1445,8 @@ static enum type_match_result find_data_type_insn(struct data_loc_info *dloc,
static int arch_supports_insn_tracking(struct data_loc_info *dloc)
{
- if ((arch__is_x86(dloc->arch)) || (arch__is_powerpc(dloc->arch)))
+ if (arch__is_x86(dloc->arch) || arch__is_powerpc(dloc->arch) ||
+ arch__is_arm64(dloc->arch))
return 1;
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support
2026-08-08 12:23 ` [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support Tengda Wu
@ 2026-08-08 13:22 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:22 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing bounds check on the secondary destination register (`dst->reg2`) leads to an out-of-bounds stack write in `update_insn_state_arm64()`.
- [Medium] Incorrect invalidation of base pointer type information during memory store instructions in `update_insn_state_arm64()`.
--
--- Patch [12]: [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support ---
commit f55f5ca2b14376eae146cdb6373ed5a53a114257
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Enable instruction tracking support
Enable basic instruction tracking for arm64 by implementing three
essential functions in the find_data_type_block() call path.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 6b4f1228afa9b..44daa0176e512 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
> @@ -353,6 +357,65 @@ static int extract_op_location_arm64(const struct arch *arch,
[ ... ]
> + /*
> + * Invalidate destination register(s) for unsupported instructions to
> + * prevent stale type info from propagating to subsequent instructions.
> + */
> + if (has_reg_type(state, dst->reg1)) {
> + pr_debug_dtp("%s [%x] invalidate reg%d",
> + dl->ins.name, insn_offset, dst->reg1);
> + invalidate_reg_state(&state->regs[dst->reg1]);
[Severity: Medium]
Does this incorrectly invalidate the base pointer type information during memory
store instructions?
If dst->mem_ref is true (e.g., for a store instruction like str x1, [x0]),
this fallback lacks a !dst->mem_ref check. Will it incorrectly invalidate the
extracted base register (x0), assuming it is being overwritten?
> + if (dst->multi_regs) {
> + pr_debug_dtp(" and reg%d", dst->reg2);
> + invalidate_reg_state(&state->regs[dst->reg2]);
[Severity: High]
Can this cause an out-of-bounds write on the stack-allocated type_state
structure?
If an instruction parses with multiple registers where the second is invalid
(e.g., ldp x0, x99, [sp]), extract_op_location_arm64() sets op_loc->reg2
to a negative error code since it cannot find the register name.
Because there is no check for dst->reg2 >= 0 here, does this result in a
negative index being used for the regs array?
> + }
> + pr_debug_dtp("\n");
> + return;
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=12
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (11 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 12/23] perf annotate-arm64: Enable instruction tracking support Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:05 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking Tengda Wu
` (9 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Enable data type profiling to track function return values in arm64
annotations by looking up the DWARF return type of called functions.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/annotate-arch/annotate-arm64.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 44daa0176e51..acff14ca01e0 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -359,7 +359,7 @@ static int extract_op_location_arm64(const struct arch *arch,
#ifdef HAVE_LIBDW_SUPPORT
static void update_insn_state_arm64(struct type_state *state,
- struct data_loc_info *dloc, Dwarf_Die *cu_die __maybe_unused,
+ struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
{
struct annotated_insn_loc loc;
@@ -378,6 +378,8 @@ static void update_insn_state_arm64(struct type_state *state,
struct symbol *func = dl->ops.target.sym;
const char *call_name;
u64 call_addr;
+ struct type_state_reg *tsr;
+ Dwarf_Die type_die;
call_name = func ? func->name : dl->ops.target.name;
pr_debug_dtp("call [%x] %s\n", insn_offset, call_name ?: "<unknown>");
@@ -395,6 +397,20 @@ static void update_insn_state_arm64(struct type_state *state,
continue;
invalidate_reg_state(reg);
}
+
+ /* Update register with the return type (if any) */
+ if (call_name && die_find_func_rettype(cu_die, call_name, &type_die)) {
+ tsr = &state->regs[state->ret_reg];
+ tsr->copied_from = -1;
+ tsr->type = type_die;
+ tsr->kind = TSR_KIND_TYPE;
+ tsr->offset = 0;
+ tsr->ok = true;
+
+ pr_debug_dtp("call [%x] return -> reg%d",
+ insn_offset, state->ret_reg);
+ pr_debug_type_name(&type_die, tsr->kind);
+ }
return;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions
2026-08-08 12:23 ` [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions Tengda Wu
@ 2026-08-08 13:05 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:05 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing invalidation of DWARF lifetime tracking when a caller-saved return register is updated with a function's return type.
--
--- Patch [13]: [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions ---
commit 001dcc403f6841490dc601cf7b406aa4f0f30321
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Track return type after call instructions
This commit enables data type profiling to track function return values in
arm64 annotations by looking up the DWARF return type of called functions.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 44daa0176e512..acff14ca01e00 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> @@ -395,6 +397,20 @@ static void update_insn_state_arm64(struct type_state *state,
> continue;
> invalidate_reg_state(reg);
> }
> +
> + /* Update register with the return type (if any) */
> + if (call_name && die_find_func_rettype(cu_die, call_name, &type_die)) {
> + tsr = &state->regs[state->ret_reg];
> + tsr->copied_from = -1;
> + tsr->type = type_die;
> + tsr->kind = TSR_KIND_TYPE;
> + tsr->offset = 0;
> + tsr->ok = true;
[Severity: Medium]
Does this code need to clear the existing dynamic lifetime tracking fields
(such as lifetime_active and lifetime_end) before updating the return
register?
If the return register (e.g., x0) previously held a variable with an active
DWARF lifetime that spanned across this call, failing to call
invalidate_reg_state() or manually clear those fields could leave stale
lifetime flags in the register state.
If a subsequent function call occurs before that original lifetime ends, and
it does not return a known type, the return register might skip invalidation
due to the stale lifetime flag. This could erroneously preserve the first
call's return type and corrupt the data type profiling statistics.
> +
> + pr_debug_dtp("call [%x] return -> reg%d",
> + insn_offset, state->ret_reg);
> + pr_debug_type_name(&type_die, tsr->kind);
> + }
> return;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=13
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (12 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 13/23] perf annotate-arm64: Track return type after call instructions Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:08 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 15/23] perf annotate-arm64: Support store " Tengda Wu
` (8 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state_arm64() to handle LDR instructions, tracking
register state changes when data is loaded from memory to registers.
The implementation handles the three primary arm64 addressing modes:
1. Signed offset: [base, #imm|reg]
2. Pre-index: [base, #imm]!
3. Post-index: [base], #imm
Before updating, check the addressing mode via get_reg_index_offset() to
obtain the actual source's reg_offset, and then propagate the type.
Since a load instruction may have two destination registers (in ldp cases),
introduce propagate_load_reg_state() to propagate the type for a specified
destination register using a given reg_offset. The respective reg_offset
values for the two registers are as follows:
dst->reg1: reg_offset = get_reg_index_offset()
dst->reg2: reg_offset = get_reg_index_offset() + reg_size(dst->reg1)
Finally, handle the side effects of pre-index and post-index addressing
via adjust_reg_index_state().
A real-world example is shown below:
ffff80008011f5b0 <pick_task_stop>:
ffff80008011f5b8: ldr x0, [x0, #2712] // x0: struct rq* -> task_struct*
* ffff80008011f5c0: ldr w1, [x0, #104]
Before this commit, the type of x0 was incorrectly inferred as 'struct rq':
find data type for 0x68(reg0) at pick_task_stop+0x10
var [8] reg0 offset 0 type='struct rq*'
chk [10] reg0 offset=0x68 ok=1 kind=1 (struct rq*) : Good!
final result: type='struct rq'
After this commit, the type of x0 is correctly inferred as 'struct task_struct':
find data type for 0x68(reg0) at pick_task_stop+0x10
var [8] reg0 offset 0 type='struct rq*'
ldr [8] 0xa98(reg0) -> reg0 type='struct task_struct*'
chk [10] reg0 offset=0x68 ok=1 kind=1 (struct task_struct*) : Good!
final result: type='struct task_struct'
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 148 +++++++++++++++++-
1 file changed, 147 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index acff14ca01e0..6557c0ad11b2 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -358,11 +358,152 @@ static int extract_op_location_arm64(const struct arch *arch,
}
#ifdef HAVE_LIBDW_SUPPORT
+static int arm64__reg_size(const char *reg)
+{
+ if (!reg || !*reg || !arm64__is_reg(reg))
+ return -1;
+
+ if (reg[0] == 'w')
+ return 4;
+
+ if (reg[0] == 'x' || !strncmp(reg, "sp", 2))
+ return 8;
+
+ return -1;
+}
+
+static int get_reg_index_offset(struct annotated_op_loc *op_loc)
+{
+ return op_loc->addr_mode == PERF_ADDR_MODE_POST_INDEX ? 0 : op_loc->offset;
+}
+
+/* Apply addressing mode (pre-index, post-index) to register state */
+static void adjust_reg_index_state(struct type_state *state,
+ struct annotated_op_loc *op_loc,
+ const char *insn_name, u32 insn_offset)
+{
+ struct type_state_reg *tsr;
+ int reg = op_loc->reg1;
+
+ if (op_loc->addr_mode != PERF_ADDR_MODE_PRE_INDEX &&
+ op_loc->addr_mode != PERF_ADDR_MODE_POST_INDEX)
+ return;
+
+ if (!has_reg_type(state, reg) || !state->regs[reg].ok)
+ return;
+
+ tsr = &state->regs[reg];
+ tsr->copied_from = -1;
+ tsr->offset = op_loc->offset + tsr->offset;
+
+ pr_debug_dtp("%s [%x] %s-index %#x(reg%d) -> reg%d", insn_name,
+ insn_offset, op_loc->addr_mode == PERF_ADDR_MODE_PRE_INDEX ?
+ "pre" : "post", op_loc->offset, reg, reg);
+ pr_debug_type_name(&tsr->type, tsr->kind);
+}
+
+/*
+ * For load insns: propagate type from @src to @dreg, applying @reg_offset
+ * to the source struct's field offset.
+ */
+static int propagate_load_reg_state(struct type_state *state,
+ struct disasm_line *dl, int dreg,
+ struct annotated_op_loc *src,
+ int reg_offset, const char *insn_name)
+{
+ struct type_state_reg *tsr;
+ struct type_state_reg src_tsr;
+ Dwarf_Die type_die;
+ u32 insn_offset = dl->al.offset;
+ int sreg = src->reg1;
+
+ if (!has_reg_type(state, dreg))
+ return -1;
+
+ tsr = &state->regs[dreg];
+ tsr->copied_from = -1;
+
+retry:
+ if (!has_reg_type(state, sreg) || !state->regs[sreg].ok)
+ return -1;
+
+ src_tsr = state->regs[sreg];
+
+ /* Dereference the pointer if it has one */
+ if (src_tsr.kind == TSR_KIND_TYPE &&
+ die_deref_ptr_type(&src_tsr.type,
+ src_tsr.offset + reg_offset, &type_die)) {
+ tsr->type = type_die;
+ tsr->kind = TSR_KIND_TYPE;
+ tsr->offset = 0;
+ tsr->ok = true;
+
+ if (src->multi_regs) {
+ pr_debug_dtp("%s [%x] %#x(reg%d, reg%d) -> reg%d",
+ insn_name, insn_offset, reg_offset,
+ src->reg1, src->reg2, dreg);
+ } else {
+ pr_debug_dtp("%s [%x] %#x(reg%d) -> reg%d",
+ insn_name, insn_offset, reg_offset,
+ sreg, dreg);
+ }
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ return 0;
+ }
+ /* Or try another register if any */
+ else if (src->multi_regs && src->reg1 != src->reg2 && sreg != src->reg2) {
+ sreg = src->reg2;
+ goto retry;
+ }
+
+ return -1;
+}
+
+static void update_load_insn_state(struct type_state *state,
+ struct disasm_line *dl,
+ struct annotated_op_loc *src,
+ struct annotated_op_loc *dst)
+{
+ int reg_offset = get_reg_index_offset(src);
+ const char *insn_name = dst->multi_regs ? "ldp" : "ldr";
+
+ if (!has_reg_type(state, dst->reg1) ||
+ (dst->multi_regs && !has_reg_type(state, dst->reg2)))
+ goto out_err_adjust;
+
+ /* Handle the first destination register */
+ if (propagate_load_reg_state(state, dl, dst->reg1, src,
+ reg_offset, insn_name))
+ goto out_err_adjust;
+
+ /* Handle the second destination register (ldp only) */
+ if (dst->multi_regs) {
+ int reg_size = arm64__reg_size(dl->ops.target.raw);
+
+ if (reg_size < 0 ||
+ propagate_load_reg_state(state, dl, dst->reg2, src,
+ reg_offset + reg_size, insn_name))
+ goto out_err_adjust;
+ }
+
+out_adjust:
+ adjust_reg_index_state(state, src, insn_name, dl->al.offset);
+ return;
+
+out_err_adjust:
+ if (has_reg_type(state, dst->reg1))
+ invalidate_reg_state(&state->regs[dst->reg1]);
+ if (dst->multi_regs && has_reg_type(state, dst->reg2))
+ invalidate_reg_state(&state->regs[dst->reg2]);
+ goto out_adjust;
+}
+
static void update_insn_state_arm64(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
{
struct annotated_insn_loc loc;
+ struct annotated_op_loc *src = &loc.ops[INSN_OP_SOURCE];
struct annotated_op_loc *dst = &loc.ops[INSN_OP_TARGET];
u32 insn_offset = dl->al.offset;
@@ -418,7 +559,8 @@ static void update_insn_state_arm64(struct type_state *state,
* Invalidate destination register(s) for unsupported instructions to
* prevent stale type info from propagating to subsequent instructions.
*/
- if (has_reg_type(state, dst->reg1)) {
+ if (has_reg_type(state, dst->reg1) &&
+ strncmp(dl->ins.name, "ld", 2)) {
pr_debug_dtp("%s [%x] invalidate reg%d",
dl->ins.name, insn_offset, dst->reg1);
invalidate_reg_state(&state->regs[dst->reg1]);
@@ -429,6 +571,10 @@ static void update_insn_state_arm64(struct type_state *state,
pr_debug_dtp("\n");
return;
}
+
+ /* Memory to register transfers */
+ if (!strncmp(dl->ins.name, "ld", 2))
+ update_load_insn_state(state, dl, src, dst);
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking
2026-08-08 12:23 ` [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking Tengda Wu
@ 2026-08-08 13:08 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:08 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] `update_load_insn_state` incorrectly uses the newly loaded type as the base pointer for the second register in `ldp` instructions when the base register overlaps with the first destination register (e.g., `ldp x0, x1, [x0]`).
- [Medium] `update_insn_state_arm64` blindly invalidates the base pointer type state for store instructions because it fails to check `!dst->mem_ref` before invalidating `dst->reg1` for non-load instructions.
- [Medium] Incorrect memory offset calculation for the second register in `LDPSW` (Load Pair of Signed Words) instructions due to reliance on the destination register prefix for size calculation.
- [Medium] Incomplete register recognition in `arm64__is_reg()` prevents processing or defensive invalidation of the second destination register in `ldp` instructions involving zero registers (`xzr`) or SIMD registers.
- [Medium] `arm64_ldst__parse` incorrectly parses PC-relative memory loads if an end-of-line comment contains a bracket (`[`), improperly interpreting the comment as the memory operand.
--
--- Patch [14]: [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking ---
commit d3c58ddf95c5da043aecdd1f99723182e79fd318
Author: Tengda Wu <wutengda@huaweicloud.com>
Subject: perf annotate-arm64: Support load instruction tracking
This commit extends update_insn_state_arm64() to handle LDR and LDP
instructions. It tracks register state changes when data is loaded from
memory into registers by propagating the data type of the base register
to the destination register, handling various addressing modes.
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
> @@ -358,11 +358,152 @@ static int extract_op_location_arm64(const struct arch *arch,
> }
>
> #ifdef HAVE_LIBDW_SUPPORT
> +static int arm64__reg_size(const char *reg)
> +{
> + if (!reg || !*reg || !arm64__is_reg(reg))
[Severity: Medium]
Since arm64__is_reg() does not recognize zero registers like xzr or SIMD
registers, can this cause us to incorrectly skip processing the second
destination register in ldp instructions?
If the first register is unrecognized (for example, in ldp xzr, x19, [sp]),
arm64__check_multi_regs() might evaluate multi_regs to false. This would
cause update_load_insn_state() to skip the second register entirely, leaving
its stale type state intact.
> + return -1;
[ ... ]
> +static void update_load_insn_state(struct type_state *state,
> + struct disasm_line *dl,
> + struct annotated_op_loc *src,
> + struct annotated_op_loc *dst)
> +{
> + int reg_offset = get_reg_index_offset(src);
> + const char *insn_name = dst->multi_regs ? "ldp" : "ldr";
> +
> + if (!has_reg_type(state, dst->reg1) ||
> + (dst->multi_regs && !has_reg_type(state, dst->reg2)))
> + goto out_err_adjust;
> +
> + /* Handle the first destination register */
> + if (propagate_load_reg_state(state, dl, dst->reg1, src,
> + reg_offset, insn_name))
> + goto out_err_adjust;
[Severity: Medium]
Can this inadvertently corrupt the type tracking state when the base register
and the first destination register overlap?
If we process an instruction like ldp x0, x1, [x0], the first call to
propagate_load_reg_state() updates state->regs[dst->reg1] (which is x0).
When the second call to propagate_load_reg_state() runs for the second
destination register:
if (dst->multi_regs) {
int reg_size = arm64__reg_size(dl->ops.target.raw);
...
propagate_load_reg_state(state, dl, dst->reg2, src,
it will read from src->reg1 (which is also x0). Because the state for x0
was already overwritten in the first call, the second call would use the
newly loaded type as the base pointer instead of the original base pointer
type.
> +
> + /* Handle the second destination register (ldp only) */
> + if (dst->multi_regs) {
> + int reg_size = arm64__reg_size(dl->ops.target.raw);
[Severity: Medium]
Does arm64__reg_size() return the correct memory offset for ldpsw
instructions?
For a Load Pair of Signed Words (ldpsw), the instruction loads two 32-bit
values from memory into 64-bit registers. Using arm64__reg_size() on the
destination register (which starts with 'x') returns 8, but the memory
spacing should be 4 bytes.
This would result in passing reg_offset + 8 instead of reg_offset + 4 to
propagate_load_reg_state() for the second register, leading to an incorrect
struct field offset lookup.
> +
> + if (reg_size < 0 ||
> + propagate_load_reg_state(state, dl, dst->reg2, src,
> + reg_offset + reg_size, insn_name))
> + goto out_err_adjust;
> + }
[ ... ]
> @@ -418,7 +559,8 @@ static void update_insn_state_arm64(struct type_state *state,
> * Invalidate destination register(s) for unsupported instructions to
> * prevent stale type info from propagating to subsequent instructions.
> */
> - if (has_reg_type(state, dst->reg1)) {
> + if (has_reg_type(state, dst->reg1) &&
> + strncmp(dl->ins.name, "ld", 2)) {
[Severity: Medium]
Should we check if the destination is actually a memory reference before
invalidating the register?
For store instructions (like str x0, [x1, #16]), arm64_ldst__parse() reverses
the operands so that dst becomes the memory operand and dst->reg1 is the
base pointer (x1).
By unconditionally invalidating dst->reg1 for any instruction that doesn't
start with "ld", we would erroneously destroy the type state of the base
pointer used in the store.
> pr_debug_dtp("%s [%x] invalidate reg%d",
> dl->ins.name, insn_offset, dst->reg1);
> invalidate_reg_state(&state->regs[dst->reg1]);
[ ... ]
> + /* Memory to register transfers */
> + if (!strncmp(dl->ins.name, "ld", 2))
> + update_load_insn_state(state, dl, src, dst);
> }
> #endif
[Severity: Medium]
Could PC-relative memory loads end up with missing operands here if their
end-of-line comments contain a bracket?
In arm64_ldst__parse(), which runs before we reach this point, strchr()
searches for a bracket. If a PC-relative load lacks brackets in its operands
but contains one in the comment (for example, ldr x0, 1234 <sym> // [foo]),
the parser matches the comment bracket and parses backwards.
This drops the actual PC-relative target entirely, which would prevent
update_load_insn_state() from successfully tracking the type when evaluated
here.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=14
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 15/23] perf annotate-arm64: Support store instruction tracking
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (13 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 14/23] perf annotate-arm64: Support load instruction tracking Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:11 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64 Tengda Wu
` (7 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to handle store (STR) instructions.
Unlike load instructions, a store instruction sets a value in the struct
within the memory where the destination register resides, and does not
alter its type. Therefore, no processing is required for the transfer.
The only point to note is that store instructions support pre-index and
post-index addressing modes, so calling adjust_reg_index_state() is still
necessary to handle their addressing.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/annotate-arch/annotate-arm64.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 6557c0ad11b2..ed0f0ef2877d 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -560,7 +560,7 @@ static void update_insn_state_arm64(struct type_state *state,
* prevent stale type info from propagating to subsequent instructions.
*/
if (has_reg_type(state, dst->reg1) &&
- strncmp(dl->ins.name, "ld", 2)) {
+ strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {
pr_debug_dtp("%s [%x] invalidate reg%d",
dl->ins.name, insn_offset, dst->reg1);
invalidate_reg_state(&state->regs[dst->reg1]);
@@ -575,6 +575,17 @@ static void update_insn_state_arm64(struct type_state *state,
/* Memory to register transfers */
if (!strncmp(dl->ins.name, "ld", 2))
update_load_insn_state(state, dl, src, dst);
+ /* Register to memory transfers */
+ else if (!strncmp(dl->ins.name, "st", 2)) {
+ /*
+ * Ignore transfers since it'd set a value in a struct
+ * and won't change the type.
+ *
+ * Needs to update the pre-index and post-index addressing
+ * modes for the destination register.
+ */
+ adjust_reg_index_state(state, dst, "str", insn_offset);
+ }
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 15/23] perf annotate-arm64: Support store instruction tracking
2026-08-08 12:23 ` [PATCH v4 15/23] perf annotate-arm64: Support store " Tengda Wu
@ 2026-08-08 13:11 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:11 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] For post-index addressing modes using a register offset (e.g., `ldr x0, [x1], x2` or `str x1, [x0], x2`), the base register's type state is incorrectly preserved instead of being invalidated.
--
--- Patch [15]: [PATCH v4 15/23] perf annotate-arm64: Support store instruction tracking ---
commit 5069bd6d8cc957f98ba504ae076e8ee35699d61a
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Support store instruction tracking
This commit extends the arm64 update_insn_state() function to handle
store (STR) instructions. It ignores the transfer itself since it won't
change the type, but calls adjust_reg_index_state() to handle addressing
modes for the destination register.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> @@ -575,6 +575,17 @@ static void update_insn_state_arm64(struct type_state *state,
> /* Memory to register transfers */
> if (!strncmp(dl->ins.name, "ld", 2))
> update_load_insn_state(state, dl, src, dst);
> + /* Register to memory transfers */
> + else if (!strncmp(dl->ins.name, "st", 2)) {
> + /*
> + * Ignore transfers since it'd set a value in a struct
> + * and won't change the type.
> + *
> + * Needs to update the pre-index and post-index addressing
> + * modes for the destination register.
> + */
> + adjust_reg_index_state(state, dst, "str", insn_offset);
[Severity: Medium]
When using post-index addressing modes with a register offset (e.g.,
"str x1, [x0], x2"), does adjust_reg_index_state() correctly invalidate
the base register's state?
Looking at adjust_reg_index_state() in the same file, when op_loc->multi_regs
is true (indicating a dynamic register offset), op_loc->offset is 0:
tools/perf/util/annotate-arch/annotate-arm64.c:adjust_reg_index_state() {
...
tsr->offset = op_loc->offset + tsr->offset;
...
}
Since op_loc->offset is 0 in this case, this seems to retain the old static
offset instead of clearing the state via invalidate_reg_state().
Can this cause incorrect struct field attribution in the perf annotate
--data-type output for subsequent accesses using the modified base register?
> + }
> }
> #endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=15
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (14 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 15/23] perf annotate-arm64: Support store " Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:17 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 17/23] perf annotate-data: Track imm_value for stack variables Tengda Wu
` (6 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
The imm_value in struct type_state_reg is defined as u32, which limits
the size of values it can pass.
Promote imm_value from u32 to u64 and adjust the print format specifier
in pr_debug_dtp() accordingly.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/annotate-arch/annotate-x86.c | 2 +-
tools/perf/util/annotate-data.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
index ee4e3e7f3209..eec3d8ce00b8 100644
--- a/tools/perf/util/annotate-arch/annotate-x86.c
+++ b/tools/perf/util/annotate-arch/annotate-x86.c
@@ -540,7 +540,7 @@ static void update_insn_state_x86(struct type_state *state,
tsr->offset = 0;
tsr->ok = true;
- pr_debug_dtp("mov [%x] imm=%#x -> reg%d\n",
+ pr_debug_dtp("mov [%x] imm=%#"PRIx64" -> reg%d\n",
insn_offset, tsr->imm_value, dst->reg1);
return;
}
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 453e13bbe3e2..91b83e94c51b 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -173,7 +173,7 @@ extern struct annotated_data_stat ann_data_stat;
*/
struct type_state_reg {
Dwarf_Die type;
- u32 imm_value;
+ u64 imm_value;
/*
* The offset within the struct that the register points to.
* A value of 0 means the register points to the beginning.
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64
2026-08-08 12:23 ` [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64 Tengda Wu
@ 2026-08-08 13:17 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:17 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Expanding `imm_value` to `u64` fails to support 64-bit immediates and introduces a critical sign-extension bug for 32-bit constants due to an intermediate 32-bit field.
--
--- Patch [16]: [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64 ---
commit df449a8cd3aeb4d61dac5a7e48f7afaddbe8b587
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-data: Expand type_state_reg imm_value to u64
The imm_value in struct type_state_reg is promoted from u32 to u64 to
support larger immediate values. The print format specifier is adjusted
accordingly.
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 453e13bbe3e2c..91b83e94c51b9 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -173,7 +173,7 @@ extern struct annotated_data_stat ann_data_stat;
> */
> struct type_state_reg {
> Dwarf_Die type;
> - u32 imm_value;
> + u64 imm_value;
[Severity: Medium]
Does this change introduce a sign-extension issue for 32-bit constants?
The offset field in struct annotated_op_loc (like src->offset) is currently
a 32-bit signed int. When we assign imm_value = src->offset in
update_insn_state_x86(), a 32-bit immediate like 0xdeadbeef will now be
sign-extended into this new u64 field instead of zero-extended.
Additionally, because the intermediate offset is still an int, does this
fail to support 64-bit immediates? They would be truncated during parsing
when extract_op_location_x86() assigns op_loc->offset = strtol(...).
> /*
> * The offset within the struct that the register points to.
> * A value of 0 means the register points to the beginning.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=16
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 17/23] perf annotate-data: Track imm_value for stack variables
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (15 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 16/23] perf annotate-data: Expand type_state_reg imm_value to u64 Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking Tengda Wu
` (5 subsequent siblings)
22 siblings, 0 replies; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Currently, imm_value are tracked in registers but dropped when values
are saved to or loaded from stack variables during type profiling.
If a register of type TSR_KIND_CONST is stored to the stack, its value
will be lost.
Add an imm_value field to struct type_state_stack and update
set_stack_state() and findnew_stack_state() to accept and preserve the
immediate value when updating stack state.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
tools/perf/util/annotate-arch/annotate-x86.c | 8 ++++++--
tools/perf/util/annotate-data.c | 13 +++++++------
tools/perf/util/annotate-data.h | 5 +++--
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
index eec3d8ce00b8..995117525e7d 100644
--- a/tools/perf/util/annotate-arch/annotate-x86.c
+++ b/tools/perf/util/annotate-arch/annotate-x86.c
@@ -591,6 +591,7 @@ static void update_insn_state_x86(struct type_state *state,
tsr->type = stack->type;
tsr->kind = stack->kind;
tsr->offset = stack->ptr_offset;
+ tsr->imm_value = stack->imm_value;
tsr->ok = true;
} else if (die_get_member_type(&stack->type,
offset - stack->offset,
@@ -598,6 +599,7 @@ static void update_insn_state_x86(struct type_state *state,
tsr->type = type_die;
tsr->kind = TSR_KIND_TYPE;
tsr->offset = 0;
+ tsr->imm_value = 0;
tsr->ok = true;
} else {
invalidate_reg_state(tsr);
@@ -773,10 +775,12 @@ static void update_insn_state_x86(struct type_state *state,
*/
if (!stack->compound)
set_stack_state(stack, offset, tsr->kind,
- &tsr->type, tsr->offset);
+ &tsr->type, tsr->offset,
+ tsr->imm_value);
} else {
findnew_stack_state(state, offset, tsr->kind,
- &tsr->type, tsr->offset);
+ &tsr->type, tsr->offset,
+ tsr->imm_value);
}
if (dst->reg1 == fbreg) {
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index c51b20a7af9b..9dcdc3c7e688 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -590,7 +590,7 @@ struct type_state_stack *find_stack_state(struct type_state *state,
}
void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
- Dwarf_Die *type_die, int ptr_offset)
+ Dwarf_Die *type_die, int ptr_offset, u64 imm_value)
{
int tag;
Dwarf_Word size;
@@ -607,8 +607,9 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
stack->offset = offset;
stack->ptr_offset = ptr_offset;
stack->kind = kind;
+ stack->imm_value = imm_value;
- if (kind == TSR_KIND_POINTER) {
+ if (kind == TSR_KIND_POINTER || kind == TSR_KIND_CONST) {
stack->compound = false;
return;
}
@@ -629,18 +630,18 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
struct type_state_stack *findnew_stack_state(struct type_state *state,
int offset, u8 kind,
Dwarf_Die *type_die,
- int ptr_offset)
+ int ptr_offset, u64 imm_value)
{
struct type_state_stack *stack = find_stack_state(state, offset);
if (stack) {
- set_stack_state(stack, offset, kind, type_die, ptr_offset);
+ set_stack_state(stack, offset, kind, type_die, ptr_offset, imm_value);
return stack;
}
stack = malloc(sizeof(*stack));
if (stack) {
- set_stack_state(stack, offset, kind, type_die, ptr_offset);
+ set_stack_state(stack, offset, kind, type_die, ptr_offset, imm_value);
list_add(&stack->list, &state->stack_vars);
}
return stack;
@@ -935,7 +936,7 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
continue;
findnew_stack_state(state, offset, TSR_KIND_TYPE,
- &mem_die, /*ptr_offset=*/0);
+ &mem_die, /*ptr_offset=*/0, /*imm_value=*/0);
if (var->reg == state->stack_reg) {
pr_debug_dtp("var [%"PRIx64"] %#x(reg%d)",
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 91b83e94c51b..06fc4dbfb35c 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -199,6 +199,7 @@ struct type_state_stack {
int size;
bool compound;
u8 kind;
+ u64 imm_value;
};
/*
@@ -253,9 +254,9 @@ bool has_reg_type(struct type_state *state, int reg);
struct type_state_stack *findnew_stack_state(struct type_state *state,
int offset, u8 kind,
Dwarf_Die *type_die,
- int ptr_offset);
+ int ptr_offset, u64 imm_value);
void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
- Dwarf_Die *type_die, int ptr_offset);
+ Dwarf_Die *type_die, int ptr_offset, u64 imm_value);
struct type_state_stack *find_stack_state(struct type_state *state,
int offset);
void invalidate_reg_state(struct type_state_reg *reg);
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (16 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 17/23] perf annotate-data: Track imm_value for stack variables Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:25 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking Tengda Wu
` (4 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to track data types stored on the
stack. This allows 'perf annotate' to maintain type information for
local variables that are spilled to or loaded from stack slots.
The implementation handles:
1. Stack Loads (LDR): Identify when a register is loaded from a stack
slot and update the register's type state based on the tracked
stack content or compound member types.
2. Stack Stores (STR): Update or create new stack state entries when
a tracked register type is stored to the stack.
Similar to loads, stores also support saving one or two registers
(in the case of 'stp'). Therefore, propagate_store_reg_state() is
introduced. The overall offset calculation follows the same logic as
the load implementation:
src->reg1: reg_offset = get_reg_index_offset()
src->reg2: reg_offset = get_reg_index_offset() + reg_size(src->reg1)
With these changes, the instruction tracker can now follow data types
as they move between registers and memory, specifically for function
local variables and compiler-spilled values on arm64.
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 161 ++++++++++++++++--
1 file changed, 148 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index ed0f0ef2877d..6e09e9707256 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -407,6 +407,7 @@ static void adjust_reg_index_state(struct type_state *state,
* to the source struct's field offset.
*/
static int propagate_load_reg_state(struct type_state *state,
+ struct data_loc_info *dloc,
struct disasm_line *dl, int dreg,
struct annotated_op_loc *src,
int reg_offset, const char *insn_name)
@@ -416,6 +417,8 @@ static int propagate_load_reg_state(struct type_state *state,
Dwarf_Die type_die;
u32 insn_offset = dl->al.offset;
int sreg = src->reg1;
+ int fbreg = dloc->fbreg;
+ int fboff = 0;
if (!has_reg_type(state, dreg))
return -1;
@@ -423,7 +426,52 @@ static int propagate_load_reg_state(struct type_state *state,
tsr = &state->regs[dreg];
tsr->copied_from = -1;
+ if (dloc->fb_cfa) {
+ u64 ip = dloc->ms->sym->start + dl->al.offset;
+ u64 pc = map__rip_2objdump(dloc->ms->map, ip);
+
+ if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fboff) < 0)
+ fbreg = -1;
+ }
+
retry:
+ /* Check stack variables with offset */
+ if (sreg == fbreg || sreg == state->stack_reg) {
+ struct type_state_stack *stack;
+ int offset = sreg == fbreg ? reg_offset - fboff : reg_offset;
+
+ stack = find_stack_state(state, offset);
+ if (stack == NULL) {
+ return -1;
+ } else if (!stack->compound) {
+ tsr->type = stack->type;
+ tsr->kind = stack->kind;
+ tsr->offset = stack->ptr_offset;
+ tsr->imm_value = stack->imm_value;
+ tsr->ok = true;
+ } else if (die_get_member_type(&stack->type,
+ offset - stack->offset,
+ &type_die)) {
+ tsr->type = type_die;
+ tsr->kind = TSR_KIND_TYPE;
+ tsr->offset = 0;
+ tsr->imm_value = 0;
+ tsr->ok = true;
+ } else {
+ return -1;
+ }
+
+ if (sreg == fbreg) {
+ pr_debug_dtp("%s [%x] -%#x(stack) -> reg%d",
+ insn_name, insn_offset, -offset, dreg);
+ } else {
+ pr_debug_dtp("%s [%x] %#x(reg%d) -> reg%d",
+ insn_name, insn_offset, offset, sreg, dreg);
+ }
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ return 0;
+ }
+
if (!has_reg_type(state, sreg) || !state->regs[sreg].ok)
return -1;
@@ -460,6 +508,7 @@ static int propagate_load_reg_state(struct type_state *state,
}
static void update_load_insn_state(struct type_state *state,
+ struct data_loc_info *dloc,
struct disasm_line *dl,
struct annotated_op_loc *src,
struct annotated_op_loc *dst)
@@ -472,7 +521,7 @@ static void update_load_insn_state(struct type_state *state,
goto out_err_adjust;
/* Handle the first destination register */
- if (propagate_load_reg_state(state, dl, dst->reg1, src,
+ if (propagate_load_reg_state(state, dloc, dl, dst->reg1, src,
reg_offset, insn_name))
goto out_err_adjust;
@@ -481,7 +530,7 @@ static void update_load_insn_state(struct type_state *state,
int reg_size = arm64__reg_size(dl->ops.target.raw);
if (reg_size < 0 ||
- propagate_load_reg_state(state, dl, dst->reg2, src,
+ propagate_load_reg_state(state, dloc, dl, dst->reg2, src,
reg_offset + reg_size, insn_name))
goto out_err_adjust;
}
@@ -498,6 +547,100 @@ static void update_load_insn_state(struct type_state *state,
goto out_adjust;
}
+/*
+ * For store insns: propagate type from @sreg to the memory location
+ * referenced by @dreg, applying @reg_offset to the destination memory offset.
+ */
+static int propagate_store_reg_state(struct type_state *state,
+ struct data_loc_info *dloc,
+ struct disasm_line *dl, int sreg, int dreg,
+ int reg_offset, const char *insn_name)
+{
+ struct type_state_reg *tsr;
+ u32 insn_offset = dl->al.offset;
+ int fbreg = dloc->fbreg;
+ int fboff = 0;
+
+ if (!has_reg_type(state, sreg) || !state->regs[sreg].ok)
+ return -1;
+
+ if (dloc->fb_cfa) {
+ u64 ip = dloc->ms->sym->start + dl->al.offset;
+ u64 pc = map__rip_2objdump(dloc->ms->map, ip);
+
+ if (die_get_cfa(dloc->di->dbg, pc, &fbreg, &fboff) < 0)
+ fbreg = -1;
+ }
+
+ /* Check stack variables with offset */
+ if (dreg == fbreg || dreg == state->stack_reg) {
+ struct type_state_stack *stack;
+ int offset = dreg == fbreg ? reg_offset - fboff : reg_offset;
+
+ tsr = &state->regs[sreg];
+
+ stack = find_stack_state(state, offset);
+ if (stack) {
+ if (!stack->compound)
+ set_stack_state(stack, offset, tsr->kind, &tsr->type,
+ tsr->offset, tsr->imm_value);
+ /*
+ * If it's a compound type, it means attempting to
+ * write to a member value of the compound type without
+ * changing the compound type itself, so do nothing.
+ */
+ } else {
+ findnew_stack_state(state, offset, tsr->kind, &tsr->type,
+ tsr->offset, tsr->imm_value);
+ }
+
+ if (dreg == fbreg) {
+ pr_debug_dtp("%s [%x] reg%d -> -%#x(stack)",
+ insn_name, insn_offset, sreg, -offset);
+ } else {
+ pr_debug_dtp("%s [%x] reg%d -> %#x(reg%d)",
+ insn_name, insn_offset, sreg, offset, dreg);
+ }
+ if (tsr->offset != 0) {
+ pr_debug_dtp(" reg%d offset %#x ->",
+ sreg, tsr->offset);
+ }
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ }
+ /*
+ * Ignore other transfers since it'd set a value in a struct
+ * and won't change the type.
+ */
+
+ return 0;
+}
+
+static void update_store_insn_state(struct type_state *state,
+ struct data_loc_info *dloc,
+ struct disasm_line *dl,
+ struct annotated_op_loc *src,
+ struct annotated_op_loc *dst)
+{
+ int reg_offset = get_reg_index_offset(dst);
+ const char *insn_name = src->multi_regs ? "stp" : "str";
+
+ /* Handle the first source register */
+ propagate_store_reg_state(state, dloc, dl, src->reg1, dst->reg1,
+ reg_offset, insn_name);
+
+ /* Handle the second source register (stp only) */
+ if (src->multi_regs) {
+ int reg_size = arm64__reg_size(dl->ops.source.raw);
+
+ if (reg_size >= 0)
+ propagate_store_reg_state(state, dloc, dl, src->reg2,
+ dst->reg1, reg_offset + reg_size,
+ insn_name);
+ }
+
+ adjust_reg_index_state(state, dst, insn_name, dl->al.offset);
+}
+
static void update_insn_state_arm64(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
@@ -574,18 +717,10 @@ static void update_insn_state_arm64(struct type_state *state,
/* Memory to register transfers */
if (!strncmp(dl->ins.name, "ld", 2))
- update_load_insn_state(state, dl, src, dst);
+ update_load_insn_state(state, dloc, dl, src, dst);
/* Register to memory transfers */
- else if (!strncmp(dl->ins.name, "st", 2)) {
- /*
- * Ignore transfers since it'd set a value in a struct
- * and won't change the type.
- *
- * Needs to update the pre-index and post-index addressing
- * modes for the destination register.
- */
- adjust_reg_index_state(state, dst, "str", insn_offset);
- }
+ else if (!strncmp(dl->ins.name, "st", 2))
+ update_store_insn_state(state, dloc, dl, src, dst);
}
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking
2026-08-08 12:23 ` [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking Tengda Wu
@ 2026-08-08 13:25 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:25 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect register mapping and stack state propagation for exclusive store instructions (stxr, stxp).
- [Medium] Failure to invalidate stack state when storing an unknown register type.
--
--- Patch [18]: [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking ---
commit af17741623908e66e62f48c13604a02748d39721
Author: Tengda Wu <wutengda@huaweicloud.com>
Subject: perf annotate-arm64: Support stack variable tracking
Extend update_insn_state() for arm64 to track data types stored on the
stack. This allows 'perf annotate' to maintain type information for
local variables that are spilled to or loaded from stack slots.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index ed0f0ef2877d0..6e09e97072568 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> @@ -498,6 +547,100 @@ static void update_load_insn_state(struct type_state *state,
> goto out_adjust;
> }
>
> +/*
> + * For store insns: propagate type from @sreg to the memory location
> + * referenced by @dreg, applying @reg_offset to the destination memory offset.
> + */
> +static int propagate_store_reg_state(struct type_state *state,
> + struct data_loc_info *dloc,
> + struct disasm_line *dl, int sreg, int dreg,
> + int reg_offset, const char *insn_name)
> +{
> + struct type_state_reg *tsr;
> + u32 insn_offset = dl->al.offset;
> + int fbreg = dloc->fbreg;
> + int fboff = 0;
> +
> + if (!has_reg_type(state, sreg) || !state->regs[sreg].ok)
> + return -1;
[Severity: Medium]
Will this early return fail to invalidate the stack state when storing an
unknown register type?
If a register without a known type is stored to a stack location that
previously held a known type, this early return appears to bypass stack
invalidation. This would leave stale type information intact for that
stack offset, causing subsequent loads from that slot to erroneously
propagate the old type. While similar logic exists in the x86
implementation, does it need to be corrected here to prevent stale types?
[ ... ]
> +static void update_store_insn_state(struct type_state *state,
> + struct data_loc_info *dloc,
> + struct disasm_line *dl,
> + struct annotated_op_loc *src,
> + struct annotated_op_loc *dst)
> +{
> + int reg_offset = get_reg_index_offset(dst);
> + const char *insn_name = src->multi_regs ? "stp" : "str";
> +
> + /* Handle the first source register */
> + propagate_store_reg_state(state, dloc, dl, src->reg1, dst->reg1,
> + reg_offset, insn_name);
> +
> + /* Handle the second source register (stp only) */
> + if (src->multi_regs) {
> + int reg_size = arm64__reg_size(dl->ops.source.raw);
> +
> + if (reg_size >= 0)
> + propagate_store_reg_state(state, dloc, dl, src->reg2,
> + dst->reg1, reg_offset + reg_size,
> + insn_name);
> + }
[Severity: Medium]
Does this logic correctly handle exclusive store instructions like stxr or
stxp?
When the instruction tracker parses arm64 exclusive store instructions, it
appears arm64__check_multi_regs() sets src->multi_regs to true because of
the comma-separated registers.
Would update_store_insn_state() then incorrectly assume the instruction is
stp, and propagate the type of the status register to the base memory
offset, while assigning the true source register to the offset + reg_size?
> +
> + adjust_reg_index_state(state, dst, insn_name, dl->al.offset);
> +}
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=18
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (17 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 18/23] perf annotate-arm64: Support stack variable tracking Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:20 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 20/23] perf annotate-arm64: Support 'add' " Tengda Wu
` (3 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to support register-to-register and
immediate-to-register 'mov' instructions.
For register-to-register 'mov' (e.g., mov dreg, sreg), propagate data type
information from the source register to the destination register.
For immediate-to-register 'mov' (e.g., mov dreg, #imm), store the immediate
value in the destination register's imm_value field and set its kind to
TSR_KIND_CONST, allowing subsequent instructions to resolve it as a
constant.
A real-world example is shown below:
ffff8000803eebf8 <get_vma_policy>:
ffff8000803eec20: mov x21, x0 // x0 (struct vm_area_struct*) -> x21
ffff8000803eec28: ldr x2, [x0, #112]
ffff8000803eec2c: cbz x2, ffff8000803eec94 <get_vma_policy+0x9c>
* ffff8000803eec94: ldr x0, [x21, #152]
Before this commit, the type of x21 was unknown, causing the subsequent
inference to fail:
var [0] reg0 offset 0 type='struct vm_area_struct*' size=0x8
chk [9c] reg21 offset=0x98 ok=0 kind=0 cfa : no type information
final result: no type information
After this commit, the type of x21 is correctly inferred as 'vm_area_struct':
var [0] reg0 offset 0 type='struct vm_area_struct*' size=0x8
mov [28] reg0 -> reg21 type='struct vm_area_struct*' size=0x8
chk [9c] reg21 offset=0x98 ok=1 kind=1 (struct vm_area_struct*) : Good!
found by insn track: 0x98(reg21) type-offset=0x98
final result: type='struct vm_area_struct' size=0xb0
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 53 ++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 6e09e9707256..7b780bad8c07 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/compiler.h>
#include <errno.h>
+#include <inttypes.h>
#include <stdlib.h>
#include <string.h>
#include <linux/ctype.h>
@@ -484,6 +485,7 @@ static int propagate_load_reg_state(struct type_state *state,
tsr->type = type_die;
tsr->kind = TSR_KIND_TYPE;
tsr->offset = 0;
+ tsr->imm_value = 0;
tsr->ok = true;
if (src->multi_regs) {
@@ -641,6 +643,51 @@ static void update_store_insn_state(struct type_state *state,
adjust_reg_index_state(state, dst, insn_name, dl->al.offset);
}
+static void update_mov_insn_state(struct type_state *state,
+ struct disasm_line *dl,
+ struct annotated_op_loc *src,
+ struct annotated_op_loc *dst)
+{
+ struct type_state_reg *tsr;
+ u32 insn_offset = dl->al.offset;
+ int sreg = src->reg1;
+ int dreg = dst->reg1;
+
+ if (!has_reg_type(state, dreg))
+ return;
+
+ tsr = &state->regs[dreg];
+ tsr->copied_from = -1;
+
+ if (src->imm) {
+ tsr->kind = TSR_KIND_CONST;
+ tsr->imm_value = src->offset;
+ tsr->offset = 0;
+ tsr->ok = true;
+
+ pr_debug_dtp("mov [%x] imm=%#"PRIx64" -> reg%d\n",
+ insn_offset, tsr->imm_value, dreg);
+ return;
+ }
+
+ if (!has_reg_type(state, sreg) || !state->regs[sreg].ok) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ tsr->type = state->regs[sreg].type;
+ tsr->kind = state->regs[sreg].kind;
+ tsr->imm_value = state->regs[sreg].imm_value;
+ tsr->offset = state->regs[sreg].offset;
+ tsr->ok = state->regs[sreg].ok;
+
+ if (tsr->kind == TSR_KIND_TYPE || tsr->kind == TSR_KIND_POINTER)
+ tsr->copied_from = sreg;
+
+ pr_debug_dtp("mov [%x] reg%d -> reg%d", insn_offset, sreg, dreg);
+ pr_debug_type_name(&tsr->type, tsr->kind);
+}
+
static void update_insn_state_arm64(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
@@ -703,6 +750,7 @@ static void update_insn_state_arm64(struct type_state *state,
* prevent stale type info from propagating to subsequent instructions.
*/
if (has_reg_type(state, dst->reg1) &&
+ strcmp(dl->ins.name, "mov") &&
strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {
pr_debug_dtp("%s [%x] invalidate reg%d",
dl->ins.name, insn_offset, dst->reg1);
@@ -715,8 +763,11 @@ static void update_insn_state_arm64(struct type_state *state,
return;
}
+ /* Register to register or imm value to register transfers */
+ if (!strcmp(dl->ins.name, "mov"))
+ update_mov_insn_state(state, dl, src, dst);
/* Memory to register transfers */
- if (!strncmp(dl->ins.name, "ld", 2))
+ else if (!strncmp(dl->ins.name, "ld", 2))
update_load_insn_state(state, dloc, dl, src, dst);
/* Register to memory transfers */
else if (!strncmp(dl->ins.name, "st", 2))
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking
2026-08-08 12:23 ` [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking Tengda Wu
@ 2026-08-08 13:20 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:20 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Conservative register invalidation incorrectly drops data types for comparison instructions like 'cmp', 'cmn', 'tst', and 'teq'.
- [Medium] Missing DWARF register lifetime propagation and clearing in ARM64 `mov` instruction tracking.
--
--- Patch [19]: [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking ---
commit 16c512fdcea449b7238a3bf5f1a3afbffde8144e
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Support 'mov' instruction tracking
Extend update_insn_state() for arm64 to support register-to-register and
immediate-to-register 'mov' instructions. This propagates data types and
constant values to improve subsequent instruction inference.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index 6e09e97072568..7b780bad8c070 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> +static void update_mov_insn_state(struct type_state *state,
> + struct disasm_line *dl,
> + struct annotated_op_loc *src,
> + struct annotated_op_loc *dst)
> +{
> + struct type_state_reg *tsr;
> + u32 insn_offset = dl->al.offset;
> + int sreg = src->reg1;
> + int dreg = dst->reg1;
> +
> + if (!has_reg_type(state, dreg))
> + return;
> +
> + tsr = &state->regs[dreg];
> + tsr->copied_from = -1;
> +
> + if (src->imm) {
> + tsr->kind = TSR_KIND_CONST;
> + tsr->imm_value = src->offset;
> + tsr->offset = 0;
> + tsr->ok = true;
[Severity: Medium]
Does this code need to clear lifetime_active for the destination register?
If an immediate value overwrites a register, failing to clear lifetime_active
might incorrectly protect the new constant from being invalidated across
function calls.
> +
> + pr_debug_dtp("mov [%x] imm=%#"PRIx64" -> reg%d\n",
> + insn_offset, tsr->imm_value, dreg);
> + return;
> + }
> +
> + if (!has_reg_type(state, sreg) || !state->regs[sreg].ok) {
> + invalidate_reg_state(tsr);
> + return;
> + }
> +
> + tsr->type = state->regs[sreg].type;
> + tsr->kind = state->regs[sreg].kind;
> + tsr->imm_value = state->regs[sreg].imm_value;
> + tsr->offset = state->regs[sreg].offset;
> + tsr->ok = state->regs[sreg].ok;
[Severity: Medium]
Does this code miss propagating the DWARF register lifetime?
If lifetime_active is not propagated from state->regs[sreg] to tsr during a
register-to-register move, a valid type state might be prematurely invalidated
across function calls.
> +
> + if (tsr->kind == TSR_KIND_TYPE || tsr->kind == TSR_KIND_POINTER)
> + tsr->copied_from = sreg;
> +
> + pr_debug_dtp("mov [%x] reg%d -> reg%d", insn_offset, sreg, dreg);
> + pr_debug_type_name(&tsr->type, tsr->kind);
> +}
[ ... ]
> @@ -703,6 +750,7 @@ static void update_insn_state_arm64(struct type_state *state,
> * prevent stale type info from propagating to subsequent instructions.
> */
> if (has_reg_type(state, dst->reg1) &&
> + strcmp(dl->ins.name, "mov") &&
> strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {
> pr_debug_dtp("%s [%x] invalidate reg%d",
> dl->ins.name, insn_offset, dst->reg1);
> invalidate_reg_state(&state->regs[dst->reg1]);
[Severity: Medium]
Does this conservative register invalidation incorrectly drop data types for
comparison instructions like cmp, cmn, tst, and teq?
Because comparison instructions are not explicitly exempted alongside mov,
ld, and st, their first operand (which can be parsed as a target register)
will be unconditionally invalidated. This causes data type profiling to
silently fail for subsequent uses of a register after it's checked with a
comparison instruction.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=19
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 20/23] perf annotate-arm64: Support 'add' instruction tracking
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (18 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 19/23] perf annotate-arm64: Support 'mov' instruction tracking Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:14 ` sashiko-bot
2026-08-08 12:23 ` [PATCH v4 21/23] perf annotate-arm64: Support 'adrp' instruction to track global variables Tengda Wu
` (2 subsequent siblings)
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to track 'add' instructions for
structure member address calculation, which commonly appear as:
add dreg, base, #offset
add dreg, base, reg2 (reg2 holds a constant)
Unlike x86, the arm64 'add' instruction has an extra base register among
its source operands. Therefore, in terms of propagating the data type,
it is essentially performing a 'mov', except that before the 'mov', it
first needs to be updated by adding the offset or reg2.
A real-world example is shown below:
ffff80008001c9a8 <flush_ptrace_hw_breakpoint>:
ffff80008001c9c4: add x19, x0, #0xeb8 // x0 (task_struct*) + 0xeb8 -> x19
* ffff80008001c9d0: ldr x0, [x19]
Before this commit, the type flow broke at the 'add' instruction,
leaving the subsequent load with no type information:
chk [28] reg19 offset=0 ok=0 kind=0 cfa : no type information
final result: no type information
After this commit, the tracker correctly follows the member address
calculation:
var [0] reg0 offset 0 type='struct task_struct*'
add [1c] address of 0xeb8(reg0) -> reg19 type='struct task_struct*'
chk [28] reg19 offset=0 ok=1 kind=1 (struct task_struct*) : Good!
found by insn track: 0(reg19) type-offset=0xeb8
final result: type='struct task_struct'
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 87 ++++++++++++++++++-
1 file changed, 85 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 7b780bad8c07..eaeb4433fc3a 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -688,6 +688,87 @@ static void update_mov_insn_state(struct type_state *state,
pr_debug_type_name(&tsr->type, tsr->kind);
}
+static void update_add_insn_state(struct type_state *state,
+ struct disasm_line *dl,
+ struct annotated_op_loc *src,
+ struct annotated_op_loc *dst)
+{
+ struct type_state_reg *tsr;
+ struct type_state_reg src_tsr;
+ u32 insn_offset = dl->al.offset;
+ int sreg = src->reg1;
+ int dreg = dst->reg1;
+ u64 imm_value;
+
+ if (!has_reg_type(state, dreg))
+ return;
+
+ tsr = &state->regs[dreg];
+ tsr->copied_from = -1;
+
+retry:
+ if (!has_reg_type(state, sreg) || !state->regs[sreg].ok) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ src_tsr = state->regs[sreg];
+
+ /*
+ * Handle 'add' instructions of the form:
+ * add dreg, base, #offset (immediate offset)
+ * add dreg, base, reg2 (reg2 holds a constant)
+ *
+ * For case 2, retrieve the constant value from reg2
+ * and use it as the offset.
+ */
+ imm_value = src->offset;
+ if (src->multi_regs) {
+ int reg2 = (sreg == src->reg1) ? src->reg2 : src->reg1;
+
+ if (!has_reg_type(state, reg2) || !state->regs[reg2].ok) {
+ /* Unable to resolve type for dst, bail out */
+ invalidate_reg_state(tsr);
+ return;
+ }
+ if (state->regs[reg2].kind == TSR_KIND_CONST)
+ imm_value = state->regs[reg2].imm_value;
+ }
+
+ if (src_tsr.kind == TSR_KIND_CONST) {
+ tsr->kind = src_tsr.kind;
+ tsr->imm_value = src_tsr.imm_value + imm_value;
+ tsr->offset = 0;
+ tsr->ok = src_tsr.ok;
+
+ pr_debug_dtp("add [%x] imm %#"PRIx64"(reg%d) -> reg%d\n",
+ insn_offset, imm_value, sreg, dreg);
+ return;
+ }
+
+ if (src_tsr.kind == TSR_KIND_POINTER ||
+ (src_tsr.kind == TSR_KIND_TYPE &&
+ dwarf_tag(&src_tsr.type) == DW_TAG_pointer_type)) {
+ tsr->type = src_tsr.type;
+ tsr->kind = src_tsr.kind;
+ tsr->imm_value = src_tsr.imm_value;
+ tsr->offset = src_tsr.offset + imm_value;
+ tsr->ok = src_tsr.ok;
+
+ pr_debug_dtp("add [%x] address of %#"PRIx64"(reg%d) -> reg%d",
+ insn_offset, imm_value, sreg, dreg);
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ return;
+ }
+
+ /* Try another register if any */
+ if (src->multi_regs && src->reg1 != src->reg2 && sreg != src->reg2) {
+ sreg = src->reg2;
+ goto retry;
+ }
+ invalidate_reg_state(tsr);
+}
+
static void update_insn_state_arm64(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
@@ -750,7 +831,7 @@ static void update_insn_state_arm64(struct type_state *state,
* prevent stale type info from propagating to subsequent instructions.
*/
if (has_reg_type(state, dst->reg1) &&
- strcmp(dl->ins.name, "mov") &&
+ strcmp(dl->ins.name, "add") && strcmp(dl->ins.name, "mov") &&
strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {
pr_debug_dtp("%s [%x] invalidate reg%d",
dl->ins.name, insn_offset, dst->reg1);
@@ -763,8 +844,10 @@ static void update_insn_state_arm64(struct type_state *state,
return;
}
+ if (!strcmp(dl->ins.name, "add"))
+ update_add_insn_state(state, dl, src, dst);
/* Register to register or imm value to register transfers */
- if (!strcmp(dl->ins.name, "mov"))
+ else if (!strcmp(dl->ins.name, "mov"))
update_mov_insn_state(state, dl, src, dst);
/* Memory to register transfers */
else if (!strncmp(dl->ins.name, "ld", 2))
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 20/23] perf annotate-arm64: Support 'add' instruction tracking
2026-08-08 12:23 ` [PATCH v4 20/23] perf annotate-arm64: Support 'add' " Tengda Wu
@ 2026-08-08 13:14 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:14 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing state invalidation when adding a non-constant dynamic variable to a pointer causes the type tracker to silently assume an offset of 0.
- [High] Incorrect offset calculation for scaled ADD instructions due to ignored shift and extend modifiers.
- [Medium] Pointer type tracking is dropped for commutative ADD instructions when the first operand is a constant, due to a premature early return.
--
--- Patch [20]: [PATCH v4 20/23] perf annotate-arm64: Support 'add' instruction tracking ---
commit 8c78c99c0185c9b7a385f10b087f345530162939
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Support 'add' instruction tracking
Extend update_insn_state() for arm64 to track 'add' instructions for
structure member address calculation.
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> +static void update_add_insn_state(struct type_state *state,
> + struct disasm_line *dl,
> + struct annotated_op_loc *src,
> + struct annotated_op_loc *dst)
> +{
[ ... ]
> + /*
> + * Handle 'add' instructions of the form:
> + * add dreg, base, #offset (immediate offset)
> + * add dreg, base, reg2 (reg2 holds a constant)
> + *
> + * For case 2, retrieve the constant value from reg2
> + * and use it as the offset.
> + */
> + imm_value = src->offset;
> + if (src->multi_regs) {
> + int reg2 = (sreg == src->reg1) ? src->reg2 : src->reg1;
> +
> + if (!has_reg_type(state, reg2) || !state->regs[reg2].ok) {
> + /* Unable to resolve type for dst, bail out */
> + invalidate_reg_state(tsr);
> + return;
> + }
> + if (state->regs[reg2].kind == TSR_KIND_CONST)
> + imm_value = state->regs[reg2].imm_value;
> + }
[Severity: High]
If reg2 is dynamic (not TSR_KIND_CONST), does this code fail to invalidate
the destination tracking state?
It looks like imm_value will silently remain set to its previous value
(src->offset), which could cause the type tracker to assume an incorrect
offset of 0 instead of properly invalidating the tracking state.
[Severity: High]
Does this offset calculation ignore shift and extend modifiers for scaled
ADD instructions?
For example, if the instruction is "add x0, x1, x2, LSL #3", it appears we
will add the unscaled raw imm_value of reg2 to the struct offset, mapping
subsequent memory accesses to the wrong struct field or array index.
> +
> + if (src_tsr.kind == TSR_KIND_CONST) {
> + tsr->kind = src_tsr.kind;
> + tsr->imm_value = src_tsr.imm_value + imm_value;
> + tsr->offset = 0;
> + tsr->ok = src_tsr.ok;
> +
> + pr_debug_dtp("add [%x] imm %#"PRIx64"(reg%d) -> reg%d\n",
> + insn_offset, imm_value, sreg, dreg);
> + return;
> + }
[Severity: Medium]
For commutative ADD instructions where the first operand is a constant,
does this early return drop the pointer type tracking?
If the instruction is "add rd, const_reg, ptr_reg", it looks like sreg will
evaluate to TSR_KIND_CONST and trigger this return before the pointer
register (ptr_reg) can be evaluated via the "goto retry" logic below.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=20
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 21/23] perf annotate-arm64: Support 'adrp' instruction to track global variables
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (19 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 20/23] perf annotate-arm64: Support 'add' " Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 12:23 ` [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking Tengda Wu
2026-08-08 12:24 ` [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer Tengda Wu
22 siblings, 0 replies; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to track global variable types
calculated via page-relative addressing.
On arm64, global variables are typically accessed by first calculating
the page address using 'adrp', followed by an 'add' or 'ldr' to get the
specific symbol address. Without tracking 'adrp', the instruction
tracker loses the base address, making it impossible to resolve
global symbols and their associated DWARF types.
Introduce TSR_KIND_GLOBAL_ADDR to represent a partial global address
state. When encountering 'adrp', store the page-aligned target address
in the register's type state. Upon a subsequent 'add' or 'ldr'
instruction that references a TSR_KIND_GLOBAL_ADDR register, combine
the page address with the immediate offset.
A real-world example is shown below:
ffff80008032e008 <folios_put_refs>:
ffff80008032e048: adrp x24, ffff80008202f000 <nr_cpu_ids>
ffff80008032e050: add x24, x24, #0xd40
* ffff80008032e078: ldr x0, [x24]
Before this commit, x24 was unknown, leading to no type information:
chk [70] reg24 offset=0 ok=0 kind=0 cfa : no type information
final result: no type information
After this commit, the tracker correctly follows the adrp/add flow:
adrp [40] global addr=0xffff80008202f000 -> reg24
add [48] global 0xd40(reg24) -> reg24
chk [70] reg24 offset=0 ok=1 kind=7 global addr : Good!
final result: type='struct folio*'
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 82 +++++++++++++++++--
tools/perf/util/annotate-data.c | 25 +++++-
tools/perf/util/annotate-data.h | 1 +
3 files changed, 98 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index eaeb4433fc3a..7eb3bef26a64 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -408,7 +408,7 @@ static void adjust_reg_index_state(struct type_state *state,
* to the source struct's field offset.
*/
static int propagate_load_reg_state(struct type_state *state,
- struct data_loc_info *dloc,
+ struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl, int dreg,
struct annotated_op_loc *src,
int reg_offset, const char *insn_name)
@@ -500,6 +500,32 @@ static int propagate_load_reg_state(struct type_state *state,
pr_debug_type_name(&tsr->type, tsr->kind);
return 0;
}
+ /* Or check if it's a global variable */
+ else if (src_tsr.kind == TSR_KIND_GLOBAL_ADDR) {
+ u64 ip = dloc->ms->sym->start + dl->al.offset;
+ u64 addr = src_tsr.imm_value + reg_offset;
+ int offset;
+
+ if (!get_global_var_type(cu_die, dloc, ip, addr, &offset, &type_die) ||
+ !die_get_member_type(&type_die, offset, &type_die))
+ return -1;
+
+ tsr->type = type_die;
+ tsr->kind = TSR_KIND_TYPE;
+ tsr->offset = 0;
+ tsr->imm_value = 0;
+ tsr->ok = true;
+
+ if (src->multi_regs) {
+ pr_debug_dtp("%s [%x] global (reg%d, reg%d) -> reg%d",
+ insn_name, insn_offset, src->reg1, src->reg2, dreg);
+ } else {
+ pr_debug_dtp("%s [%x] global (reg%d) -> reg%d",
+ insn_name, insn_offset, sreg, dreg);
+ }
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ return 0;
+ }
/* Or try another register if any */
else if (src->multi_regs && src->reg1 != src->reg2 && sreg != src->reg2) {
sreg = src->reg2;
@@ -510,7 +536,7 @@ static int propagate_load_reg_state(struct type_state *state,
}
static void update_load_insn_state(struct type_state *state,
- struct data_loc_info *dloc,
+ struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl,
struct annotated_op_loc *src,
struct annotated_op_loc *dst)
@@ -523,7 +549,7 @@ static void update_load_insn_state(struct type_state *state,
goto out_err_adjust;
/* Handle the first destination register */
- if (propagate_load_reg_state(state, dloc, dl, dst->reg1, src,
+ if (propagate_load_reg_state(state, dloc, cu_die, dl, dst->reg1, src,
reg_offset, insn_name))
goto out_err_adjust;
@@ -532,7 +558,7 @@ static void update_load_insn_state(struct type_state *state,
int reg_size = arm64__reg_size(dl->ops.target.raw);
if (reg_size < 0 ||
- propagate_load_reg_state(state, dloc, dl, dst->reg2, src,
+ propagate_load_reg_state(state, dloc, cu_die, dl, dst->reg2, src,
reg_offset + reg_size, insn_name))
goto out_err_adjust;
}
@@ -735,14 +761,16 @@ static void update_add_insn_state(struct type_state *state,
imm_value = state->regs[reg2].imm_value;
}
- if (src_tsr.kind == TSR_KIND_CONST) {
+ if (src_tsr.kind == TSR_KIND_CONST || src_tsr.kind == TSR_KIND_GLOBAL_ADDR) {
tsr->kind = src_tsr.kind;
+ /* For 'adrp + add' pair: resolve the full global variable address. */
tsr->imm_value = src_tsr.imm_value + imm_value;
tsr->offset = 0;
tsr->ok = src_tsr.ok;
- pr_debug_dtp("add [%x] imm %#"PRIx64"(reg%d) -> reg%d\n",
- insn_offset, imm_value, sreg, dreg);
+ pr_debug_dtp("add [%x] %s %#"PRIx64"(reg%d) -> reg%d\n",
+ insn_offset, src_tsr.kind == TSR_KIND_CONST ?
+ "imm" : "global", imm_value, sreg, dreg);
return;
}
@@ -769,6 +797,39 @@ static void update_add_insn_state(struct type_state *state,
invalidate_reg_state(tsr);
}
+static void update_adrp_insn_state(struct type_state *state,
+ struct disasm_line *dl,
+ struct annotated_op_loc *dst)
+{
+ struct type_state_reg *tsr;
+ u32 insn_offset = dl->al.offset;
+ int dreg = dst->reg1;
+
+ if (!has_reg_type(state, dreg))
+ return;
+
+ tsr = &state->regs[dreg];
+ tsr->copied_from = -1;
+
+ if (!dl->ops.source.addr) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ tsr->kind = TSR_KIND_GLOBAL_ADDR;
+ /*
+ * Stores a partial page-relative address. The full absolute address
+ * of the global variable will be resolved when a subsequent 'add' or
+ * 'ldr' instruction consumes this register.
+ */
+ tsr->imm_value = dl->ops.source.addr;
+ tsr->offset = 0;
+ tsr->ok = true;
+
+ pr_debug_dtp("adrp [%x] global addr=%#"PRIx64" -> reg%d\n",
+ insn_offset, tsr->imm_value, dreg);
+}
+
static void update_insn_state_arm64(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
@@ -831,6 +892,7 @@ static void update_insn_state_arm64(struct type_state *state,
* prevent stale type info from propagating to subsequent instructions.
*/
if (has_reg_type(state, dst->reg1) &&
+ strcmp(dl->ins.name, "adrp") &&
strcmp(dl->ins.name, "add") && strcmp(dl->ins.name, "mov") &&
strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {
pr_debug_dtp("%s [%x] invalidate reg%d",
@@ -844,14 +906,16 @@ static void update_insn_state_arm64(struct type_state *state,
return;
}
- if (!strcmp(dl->ins.name, "add"))
+ if (!strcmp(dl->ins.name, "adrp"))
+ update_adrp_insn_state(state, dl, dst);
+ else if (!strcmp(dl->ins.name, "add"))
update_add_insn_state(state, dl, src, dst);
/* Register to register or imm value to register transfers */
else if (!strcmp(dl->ins.name, "mov"))
update_mov_insn_state(state, dl, src, dst);
/* Memory to register transfers */
else if (!strncmp(dl->ins.name, "ld", 2))
- update_load_insn_state(state, dloc, dl, src, dst);
+ update_load_insn_state(state, dloc, cu_die, dl, src, dst);
/* Register to memory transfers */
else if (!strncmp(dl->ins.name, "st", 2))
update_store_insn_state(state, dloc, dl, src, dst);
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 9dcdc3c7e688..7fe5b979563f 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -70,6 +70,9 @@ void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind)
case TSR_KIND_CANARY:
pr_info(" stack canary\n");
return;
+ case TSR_KIND_GLOBAL_ADDR:
+ pr_info(" global address\n");
+ return;
case TSR_KIND_TYPE:
default:
break;
@@ -609,7 +612,8 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
stack->kind = kind;
stack->imm_value = imm_value;
- if (kind == TSR_KIND_POINTER || kind == TSR_KIND_CONST) {
+ if (kind == TSR_KIND_POINTER || kind == TSR_KIND_CONST ||
+ kind == TSR_KIND_GLOBAL_ADDR) {
stack->compound = false;
return;
}
@@ -1279,6 +1283,25 @@ static enum type_match_result check_matching_type(struct type_state *state,
if (dloc->op->offset < 0 && reg != state->stack_reg && reg != dloc->fbreg)
goto check_kernel;
}
+
+ if (state->regs[reg].kind == TSR_KIND_GLOBAL_ADDR) {
+ u64 var_addr = state->regs[reg].imm_value + dloc->op->offset;
+ int var_offset;
+
+ pr_debug_dtp("global addr");
+
+ /*
+ * The register holds the address of a global variable. Try to
+ * find the variable by the address and get its type.
+ */
+ if (get_global_var_type(cu_die, dloc, dloc->ip, var_addr,
+ &var_offset, type_die)) {
+ dloc->type_offset = var_offset;
+ return PERF_TMR_OK;
+ }
+ /* No need to retry global variables */
+ return PERF_TMR_BAIL_OUT;
+ }
check_non_register:
if (reg == dloc->fbreg || reg == state->stack_reg) {
struct type_state_stack *stack;
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 06fc4dbfb35c..8158e43b30df 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -37,6 +37,7 @@ enum type_state_kind {
TSR_KIND_PERCPU_POINTER,
TSR_KIND_POINTER,
TSR_KIND_CANARY,
+ TSR_KIND_GLOBAL_ADDR,
};
/**
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (20 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 21/23] perf annotate-arm64: Support 'adrp' instruction to track global variables Tengda Wu
@ 2026-08-08 12:23 ` Tengda Wu
2026-08-08 13:18 ` sashiko-bot
2026-08-08 12:24 ` [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer Tengda Wu
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:23 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to handle per-cpu variable
addressing.
On arm64, per-cpu variables are accessed by adding a per-cpu offset
(typically from the '__per_cpu_offset' array) to the address of a global
variable or a local variable with '__percpu' modifier (e.g.,
's32 __percpu *counters' in struct percpu_counter). This results in
instruction patterns like:
ldr x0, [x6, x5] // Pattern A: direct load per-cpu instance
add x0, x6, x5 // Pattern B: compute per-cpu address
where x6 holds the per-cpu offset retrieved from:
adrp x4, <page>
add x4, x4, #offset // x4 = &__per_cpu_offset
ldr x6, [x4, w0, sxtw #3] // x6 = __per_cpu_offset[cpu]
and x5 is one of the following:
case 1: global variable
adrp x5, <page>
add x5, x5, #offset // x5 = &global_var
case 2: local variable with '__percpu' modifier
ldr x1, [x25, #32] // x1 = local_percpu_ptr
To handle such cases:
1. Identify per-cpu base initialization: Detect 'adrp + ldr' pairs that
resolve to the '__per_cpu_offset' symbol and mark the destination
register as TSR_KIND_PERCPU_BASE.
2. Propagate type information: During subsequent 'ldr' or 'add'
instructions, if one register is TSR_KIND_PERCPU_BASE, attempt to
resolve the type from the other register.
A real-world example is shown below:
ffff8000808f2d28 <cppc_set_perf>:
ffff8000808f2d38: adrp x2, ffff800082033000
ffff8000808f2d3c: add x5, x2, #0x3f8 // x5 = &__per_cpu_offset
ffff8000808f2d44: adrp x2, ffff800081f73000
ffff8000808f2d48: add x2, x2, #0x6b8 // x2 = &cpu_pcc_subspace_idx
ffff8000808f2d6c: ldr x5, [x5, w0, sxtw #3] // x5 = __per_cpu_offset[cpu]
* ffff8000808f2d80: ldr w23, [x5, x2] // per_cpu(cpu_pcc_subspace_idx, cpu)
Before this commit, the tracker could not link x5 back to a per-cpu
context, resulting in an incorrect data type resolution:
adrp [10] global addr=0xffff800082033000 -> reg2
add [14] global 0x3f8(reg2) -> reg5
adrp [1c] global addr=0xffff800081f73000 -> reg2
add [20] global 0x6b8(reg2) -> reg2
ldr [44] global (reg5, reg0) -> reg5 type='long unsigned int[]' size=0x1000
chk [58] reg5 offset=0 ok=1 kind=1 (long unsigned int[]) : Good!
found by insn track: 0(reg5, reg2) type-offset=0
final result: type='long unsigned int' size=0x8
After this commit, the tracker correctly identifies the per-cpu flow and
resolves the actual variable type:
ldr [44] global (reg5, reg0) -> reg5 percpu base
chk [58] reg5 offset=0 ok=1 kind=2 percpu var : retry
chk [58] reg2 offset=0 ok=1 kind=7 global addr : Good!
found by insn track: 0(reg5, reg2) type-offset=0
final result: type='int' size=0x4
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 61 ++++++++++++++++++-
tools/perf/util/annotate-data.c | 24 +++++++-
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index 7eb3bef26a64..b01fa6cf865f 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -12,6 +12,7 @@
#include "../disasm.h"
#include "../annotate-data.h"
#include "../debug.h"
+#include "../dso.h"
#include "../map.h"
#include "../symbol.h"
@@ -505,13 +506,23 @@ static int propagate_load_reg_state(struct type_state *state,
u64 ip = dloc->ms->sym->start + dl->al.offset;
u64 addr = src_tsr.imm_value + reg_offset;
int offset;
+ u8 kind;
+ const char *var_name = NULL;
+
+ /* it might be per-cpu offset */
+ if (dso__kernel(map__dso(dloc->ms->map)) &&
+ get_global_var_info(dloc, addr, &var_name, &offset) &&
+ !strcmp(var_name, "__per_cpu_offset"))
+ kind = TSR_KIND_PERCPU_BASE;
+ else
+ kind = TSR_KIND_TYPE;
if (!get_global_var_type(cu_die, dloc, ip, addr, &offset, &type_die) ||
!die_get_member_type(&type_die, offset, &type_die))
return -1;
tsr->type = type_die;
- tsr->kind = TSR_KIND_TYPE;
+ tsr->kind = kind;
tsr->offset = 0;
tsr->imm_value = 0;
tsr->ok = true;
@@ -526,6 +537,24 @@ static int propagate_load_reg_state(struct type_state *state,
pr_debug_type_name(&tsr->type, tsr->kind);
return 0;
}
+ /* Or check if it's a per-cpu access */
+ else if (src_tsr.kind == TSR_KIND_PERCPU_BASE) {
+ int reg2;
+
+ if (!src->multi_regs || src->reg1 == src->reg2 ||
+ sreg == src->reg2 /* retried */)
+ return -1;
+
+ reg2 = src->reg2;
+ if (!has_reg_type(state, reg2) || !state->regs[reg2].ok ||
+ (state->regs[reg2].kind != TSR_KIND_GLOBAL_ADDR &&
+ state->regs[reg2].kind != TSR_KIND_TYPE))
+ return -1;
+
+ /* Treat percpu as array: resolve type from reg2 */
+ sreg = src->reg2;
+ goto retry;
+ }
/* Or try another register if any */
else if (src->multi_regs && src->reg1 != src->reg2 && sreg != src->reg2) {
sreg = src->reg2;
@@ -789,6 +818,36 @@ static void update_add_insn_state(struct type_state *state,
return;
}
+ if (src_tsr.kind == TSR_KIND_PERCPU_BASE) {
+ int reg2;
+
+ if (!src->multi_regs) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ reg2 = (sreg == src->reg1) ? src->reg2 : src->reg1;
+
+ if (!has_reg_type(state, reg2) || !state->regs[reg2].ok ||
+ (state->regs[reg2].kind != TSR_KIND_GLOBAL_ADDR &&
+ state->regs[reg2].kind != TSR_KIND_TYPE)) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ /* Treat percpu as array: inherit type from reg2 */
+ tsr->type = state->regs[reg2].type;
+ tsr->kind = state->regs[reg2].kind;
+ tsr->offset = state->regs[reg2].offset;
+ tsr->imm_value = state->regs[reg2].imm_value;
+ tsr->ok = state->regs[reg2].ok;
+
+ pr_debug_dtp("add [%x] percpu (reg%d, reg%d) -> reg%d",
+ insn_offset, src->reg1, src->reg2, dreg);
+ pr_debug_type_name(&tsr->type, tsr->kind);
+ return;
+ }
+
/* Try another register if any */
if (src->multi_regs && src->reg1 != src->reg2 && sreg != src->reg2) {
sreg = src->reg2;
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 7fe5b979563f..8b0584486cce 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1253,11 +1253,33 @@ static enum type_match_result check_matching_type(struct type_state *state,
}
if (state->regs[reg].kind == TSR_KIND_PERCPU_BASE) {
- u64 var_addr = dloc->op->offset;
+ u64 var_addr;
int var_offset;
pr_debug_dtp("percpu var");
+ if (arch__is_arm64(dloc->arch)) {
+ int reg2;
+
+ if (!dloc->op->multi_regs ||
+ dloc->op->reg1 == dloc->op->reg2 || !retry)
+ return PERF_TMR_BAIL_OUT;
+
+ reg2 = dloc->op->reg2;
+ if (!has_reg_type(state, reg2) ||
+ !state->regs[reg2].ok ||
+ (state->regs[reg2].kind != TSR_KIND_GLOBAL_ADDR &&
+ state->regs[reg2].kind != TSR_KIND_TYPE))
+ return PERF_TMR_NO_TYPE;
+
+ pr_debug_dtp(" : retry\n");
+ retry = false;
+ reg = reg2;
+ goto again;
+ }
+
+ var_addr = dloc->op->offset;
+
if (dloc->op->multi_regs) {
int reg2 = dloc->op->reg2;
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking
2026-08-08 12:23 ` [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking Tengda Wu
@ 2026-08-08 13:18 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:18 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete offset calculation for multi-register memory operands causes inaccurate type resolution.
--
--- Patch [22]: [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking ---
commit d7e10ade80ba3983e6d311e1ab11f19b4af2d53e
Author: Tengda Wu <wutengda@huaweicloud.com>
Subject: perf annotate-arm64: Support per-cpu variable access tracking
This commit extends update_insn_state() for arm64 to handle per-cpu variable
addressing. It identifies per-cpu base initialization and propagates type
information to subsequent instructions. This allows the tracker to correctly
resolve the actual variable type in per-cpu flows.
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 7fe5b979563f7..8b0584486cce9 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -1253,11 +1253,33 @@ static enum type_match_result check_matching_type(struct type_state *state,
> }
>
> if (state->regs[reg].kind == TSR_KIND_PERCPU_BASE) {
> - u64 var_addr = dloc->op->offset;
> + u64 var_addr;
> int var_offset;
[ ... ]
> + var_addr = dloc->op->offset;
> +
> if (dloc->op->multi_regs) {
> int reg2 = dloc->op->reg2;
[Severity: Medium]
Does the TSR_KIND_GLOBAL_ADDR handling later in check_matching_type() need
similar logic for multi-register operands to ensure accurate type resolution?
Currently, for TSR_KIND_GLOBAL_ADDR, the logic does not consider multi-register
operands:
if (state->regs[reg].kind == TSR_KIND_GLOBAL_ADDR) {
u64 var_addr = state->regs[reg].imm_value + dloc->op->offset;
int var_offset;
This calculation omits adding state->regs[reg2].imm_value for multi-register
operands, unlike the TSR_KIND_PERCPU_BASE logic in the same function. The
omission also appears to apply to TSR_KIND_TYPE and TSR_KIND_POINTER.
Can this incomplete offset calculation cause incorrect type resolution in
tools/perf/util/annotate-data.c:check_matching_type() when perf processes a
memory instruction where op->multi_regs is true, the primary register is
TSR_KIND_GLOBAL_ADDR (or TYPE/POINTER), and the secondary register is
TSR_KIND_CONST?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=22
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer
2026-08-08 12:23 [PATCH v4 00/23] perf arm64: Support data type profiling Tengda Wu
` (21 preceding siblings ...)
2026-08-08 12:23 ` [PATCH v4 22/23] perf annotate-arm64: Support per-cpu variable access tracking Tengda Wu
@ 2026-08-08 12:24 ` Tengda Wu
2026-08-08 13:20 ` sashiko-bot
22 siblings, 1 reply; 42+ messages in thread
From: Tengda Wu @ 2026-08-08 12:24 UTC (permalink / raw)
To: Namhyung Kim, james.clark, xueshuai, Li Huafei
Cc: Peter Zijlstra, leo.yan, Ian Rogers, Kim Phillips, Mark Rutland,
Arnaldo Carvalho de Melo, Ingo Molnar, Bill Wendling,
Nick Desaulniers, Alexander Shishkin, Adrian Hunter, Zecheng Li,
linux-perf-users, linux-kernel, llvm, Tengda Wu
Extend update_insn_state() for arm64 to handle the 'mrs' instruction,
enabling the tracking of the 'current' task pointer in the kernel.
On arm64, the kernel uses the 'sp_el0' system register to store the
address of the currently executing 'struct task_struct'. This is
typically accessed via the 'get_current()' inline function, resulting
in the instruction 'mrs xN, sp_el0'.
To resolve the data type of the target register, first verify the
access is to 'sp_el0' within a kernel DSO. Then, locate the
'get_current()' inline function's DWARF Die at the current PC and
extract its return type (which is 'struct task_struct *').
Introduce 'task_struct_cache' to cache the DWARF offset of task_struct
alongside its corresponding dbg handle. This is particularly important
because the compiler-generated stack canary check code (which loads from
'current->stack_canary') often exists in code sections or leaf functions
where the local Compilation Unit (CU) lacks a full 'struct task_struct'
definition. Caching the offset allows 'perf annotate' to consistently
resolve task-related fields across the entire kernel binary.
A real-world example is shown below:
ffff8000800deee8 <kthread_blkcg>:
ffff8000800deef0: mrs x0, sp_el0 // x0 = current
* ffff8000800deef4: ldr w1, [x0, #44]
Before this commit, the type flow starts with no information:
chk [c] reg0 offset=0x2c ok=0 kind=0 cfa : no type information
final result: no type information
After this commit, the tracker identifies the 'current' pointer
from the system register:
mrs [8] sp_el0 -> reg0 type='struct task_struct*'
chk [c] reg0 offset=0x2c ok=1 kind=1 (struct task_struct*) : Good!
found by insn track: 0x2c(reg0) type-offset=0x2c
final result: type='struct task_struct'
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
.../perf/util/annotate-arch/annotate-arm64.c | 97 ++++++++++++++++++-
1 file changed, 95 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
index b01fa6cf865f..9d1f315dcb28 100644
--- a/tools/perf/util/annotate-arch/annotate-arm64.c
+++ b/tools/perf/util/annotate-arch/annotate-arm64.c
@@ -15,6 +15,7 @@
#include "../dso.h"
#include "../map.h"
#include "../symbol.h"
+#include "../strbuf.h"
struct arch_arm64 {
struct arch arch;
@@ -889,6 +890,96 @@ static void update_adrp_insn_state(struct type_state *state,
insn_offset, tsr->imm_value, dreg);
}
+static struct {
+ Dwarf_Off dieoff;
+ Dwarf *dbg;
+} task_struct_cache;
+
+static void update_mrs_insn_state(struct type_state *state,
+ struct data_loc_info *dloc, Dwarf_Die *cu_die,
+ struct disasm_line *dl,
+ struct annotated_op_loc *dst)
+{
+ struct type_state_reg *tsr;
+ Dwarf_Die type_die;
+ u32 insn_offset = dl->al.offset;
+ int dreg = dst->reg1;
+ Dwarf_Die func_die;
+ Dwarf_Attribute attr;
+ u64 ip, pc;
+
+ if (!has_reg_type(state, dreg))
+ return;
+
+ tsr = &state->regs[dreg];
+ tsr->copied_from = -1;
+
+ /* Handle case difference: LLVM (SP_EL0) vs objdump (sp_el0) */
+ if (!dso__kernel(map__dso(dloc->ms->map)) ||
+ strcasecmp(dl->ops.source.raw, "sp_el0")) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ ip = dloc->ms->sym->start + dl->al.offset;
+ pc = map__rip_2objdump(dloc->ms->map, ip);
+
+ /*
+ * Invalidate cached DIE offset if the debug info handle changed
+ * (e.g. switching between vmlinux and a kernel module).
+ */
+ if (task_struct_cache.dbg != dloc->di->dbg) {
+ task_struct_cache.dieoff = 0;
+ task_struct_cache.dbg = dloc->di->dbg;
+ }
+
+ if (!task_struct_cache.dieoff ||
+ !dwarf_offdie(dloc->di->dbg, task_struct_cache.dieoff, &type_die)) {
+ struct strbuf sb;
+ char *type_name;
+ /*
+ * Find the inline function 'get_current()' Dwarf_Die
+ * and obtain its return value data type, which should
+ * be 'struct task_struct*'.
+ */
+ if (!die_find_inlinefunc(cu_die, pc, &func_die) ||
+ !die_compare_name(&func_die, "get_current") ||
+ !dwarf_attr_integrate(&func_die, DW_AT_type, &attr) ||
+ !dwarf_formref_die(&attr, &type_die)) {
+ invalidate_reg_state(tsr);
+ return;
+ }
+
+ strbuf_init(&sb, 32);
+ die_get_typename_from_type(&type_die, &sb);
+ type_name = strbuf_detach(&sb, NULL);
+
+ if (!type_name || strcmp(type_name, "struct task_struct*")) {
+ invalidate_reg_state(tsr);
+ free(type_name);
+ return;
+ }
+
+ /*
+ * Cache the 'struct task_struct*' die offset globally.
+ * This allows us to resolve stack canary accesses even
+ * in CUs that lack a full task_struct definition (e.g.,
+ * compiler-generated entry/exit code).
+ */
+ task_struct_cache.dieoff = dwarf_dieoffset(&type_die);
+ free(type_name);
+ }
+
+ tsr->type = type_die;
+ tsr->kind = TSR_KIND_TYPE;
+ tsr->offset = 0;
+ tsr->imm_value = 0;
+ tsr->ok = true;
+
+ pr_debug_dtp("mrs [%x] sp_el0 -> reg%d", insn_offset, dreg);
+ pr_debug_type_name(&type_die, tsr->kind);
+}
+
static void update_insn_state_arm64(struct type_state *state,
struct data_loc_info *dloc, Dwarf_Die *cu_die,
struct disasm_line *dl)
@@ -951,7 +1042,7 @@ static void update_insn_state_arm64(struct type_state *state,
* prevent stale type info from propagating to subsequent instructions.
*/
if (has_reg_type(state, dst->reg1) &&
- strcmp(dl->ins.name, "adrp") &&
+ strcmp(dl->ins.name, "mrs") && strcmp(dl->ins.name, "adrp") &&
strcmp(dl->ins.name, "add") && strcmp(dl->ins.name, "mov") &&
strncmp(dl->ins.name, "ld", 2) && strncmp(dl->ins.name, "st", 2)) {
pr_debug_dtp("%s [%x] invalidate reg%d",
@@ -965,7 +1056,9 @@ static void update_insn_state_arm64(struct type_state *state,
return;
}
- if (!strcmp(dl->ins.name, "adrp"))
+ if (!strcmp(dl->ins.name, "mrs"))
+ update_mrs_insn_state(state, dloc, cu_die, dl, dst);
+ else if (!strcmp(dl->ins.name, "adrp"))
update_adrp_insn_state(state, dl, dst);
else if (!strcmp(dl->ins.name, "add"))
update_add_insn_state(state, dl, src, dst);
--
2.34.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer
2026-08-08 12:24 ` [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer Tengda Wu
@ 2026-08-08 13:20 ` sashiko-bot
0 siblings, 0 replies; 42+ messages in thread
From: sashiko-bot @ 2026-08-08 13:20 UTC (permalink / raw)
To: Tengda Wu; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The global `task_struct_cache` may permanently cache a DWARF offset for an incomplete forward declaration of `struct task_struct`, breaking task tracking globally.
- [High] The globally static `task_struct_cache` caches a raw `Dwarf *` pointer as a validation key, which is susceptible to an ABA problem when DWARF handles are recycled, leading to corrupt DWARF parsing.
- [Medium] Calling `die_find_inlinefunc()` with a Compilation Unit DIE (`cu_die`) instead of a Subprogram DIE (`sp_die`) causes a severe O(N) recursive DWARF tree traversal.
- [Medium] Caching the DWARF offset of `struct task_struct *` relies on a brittle non-deterministic order dependency, silently failing for compiler-generated code like stack canaries.
- [Low] Missing explicit inclusion of `<strings.h>` for `strcasecmp()`, which violates subsystem guidelines and causes build failures on musl libc.
--
--- Patch [23]: [PATCH v4 23/23] perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer ---
commit 9ea98019ec4e55f722077c51c12557f82e4d5f60
Author: Tengda Wu <wutengda@huaweicloud.com>
perf annotate-arm64: Support 'mrs' instruction to track 'current' pointer
This commit extends the arm64 instruction state tracker to handle mrs
instructions reading from sp_el0. It attempts to track the current
task pointer by locating the get_current() inline function and caching
the struct task_struct DWARF offset for subsequent resolution.
> diff --git a/tools/perf/util/annotate-arch/annotate-arm64.c b/tools/perf/util/annotate-arch/annotate-arm64.c
> index b01fa6cf865ff..9d1f315dcb280 100644
> --- a/tools/perf/util/annotate-arch/annotate-arm64.c
> +++ b/tools/perf/util/annotate-arch/annotate-arm64.c
[ ... ]
> @@ -889,6 +890,96 @@ static void update_adrp_insn_state(struct type_state *state,
[ ... ]
> + /* Handle case difference: LLVM (SP_EL0) vs objdump (sp_el0) */
> + if (!dso__kernel(map__dso(dloc->ms->map)) ||
> + strcasecmp(dl->ops.source.raw, "sp_el0")) {
[Severity: Low]
Is the <strings.h> header missing for strcasecmp()?
Building perf with musl libc requires explicit inclusion of POSIX headers,
and omitting <strings.h> can cause musl build failures.
> + invalidate_reg_state(tsr);
> + return;
> + }
[ ... ]
> + /*
> + * Invalidate cached DIE offset if the debug info handle changed
> + * (e.g. switching between vmlinux and a kernel module).
> + */
> + if (task_struct_cache.dbg != dloc->di->dbg) {
> + task_struct_cache.dieoff = 0;
> + task_struct_cache.dbg = dloc->di->dbg;
> + }
[Severity: High]
Does checking the raw Dwarf pointer expose an ABA problem?
If debug info handles are freed and reallocated for different DSOs during
analysis, the memory address could be recycled. This would bypass the cache
invalidation and cause perf to use a stale DWARF offset from the old DSO
on the new DSO, potentially leading to out-of-bounds reads in libdw.
> +
> + if (!task_struct_cache.dieoff ||
> + !dwarf_offdie(dloc->di->dbg, task_struct_cache.dieoff, &type_die)) {
> + struct strbuf sb;
> + char *type_name;
> + /*
> + * Find the inline function 'get_current()' Dwarf_Die
[Severity: Medium]
Can this cache mechanism create a non-deterministic order dependency?
If the first mrs instruction processed is a stack canary, which does not
belong to get_current(), the search will fail and the state is invalidated.
However, if a true get_current() is processed first, the cache is populated
and the function check is bypassed for subsequent stack canaries, allowing
them to succeed.
> + * and obtain its return value data type, which should
> + * be 'struct task_struct*'.
> + */
> + if (!die_find_inlinefunc(cu_die, pc, &func_die) ||
[Severity: Medium]
Will passing the Compilation Unit DIE (cu_die) instead of a Subprogram
DIE cause a severe performance regression?
Because the underlying callback returns DIE_FIND_CB_CONTINUE, passing
the entire compilation unit forces die_find_inlinefunc() to perform a
full, inefficient O(N) recursive DWARF AST walk across every node in the
file when the cache is empty.
> + !die_compare_name(&func_die, "get_current") ||
> + !dwarf_attr_integrate(&func_die, DW_AT_type, &attr) ||
> + !dwarf_formref_die(&attr, &type_die)) {
> + invalidate_reg_state(tsr);
> + return;
> + }
> +
> + strbuf_init(&sb, 32);
> + die_get_typename_from_type(&type_die, &sb);
> + type_name = strbuf_detach(&sb, NULL);
> +
> + if (!type_name || strcmp(type_name, "struct task_struct*")) {
> + invalidate_reg_state(tsr);
> + free(type_name);
> + return;
> + }
> +
> + /*
> + * Cache the 'struct task_struct*' die offset globally.
> + * This allows us to resolve stack canary accesses even
> + * in CUs that lack a full task_struct definition (e.g.,
> + * compiler-generated entry/exit code).
> + */
> + task_struct_cache.dieoff = dwarf_dieoffset(&type_die);
[Severity: High]
Could this code permanently cache incomplete forward declarations of
struct task_struct?
The string comparison matches "struct task_struct*", but there is no check
to verify if the DIE is a complete definition (e.g., by checking
DW_AT_declaration or dwarf_aggregate_size()).
If the first processed CU only has a forward declaration, its incomplete
offset will be cached globally, causing all subsequent member offset
lookups to fail since an incomplete type has no children.
> + free(type_name);
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260808122400.2961238-1-wutengda@huaweicloud.com?part=23
^ permalink raw reply [flat|nested] 42+ messages in thread