All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace
@ 2024-01-22 11:20 Changbin Du
  2024-01-22 11:20 ` [PATCH v5 1/5] perf: build: introduce the libcapstone Changbin Du
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Changbin Du @ 2024-01-22 11:20 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
	Andi Kleen, Thomas Richter, changbin.du, Changbin Du

This series introduces capstone disassembler engine to print instructions of
Intel PT trace, which was printed via the XED tool.

The advantages compared to XED tool:
    * Support arm, arm64, x86-32, x86_64, s390 (more could be supported),
      xed only for x86_64.
    * More friendly to read. Immediate address operands are shown as symbol+offs.

Display raw instructions:
    $ sudo perf record --event intel_pt//u -- ls
    $ sudo perf script --insn-trace
                perf 17423 [000] 423271.557970005:      7f2d95f16217 __GI___ioctl+0x7 (/lib/x86_64-linux-gnu/libc-2.27.so) insn: 48 3d 01 f0 ff ff
                perf 17423 [000] 423271.557970005:      7f2d95f1621d __GI___ioctl+0xd (/lib/x86_64-linux-gnu/libc-2.27.so) insn: 73 01
                perf 17423 [000] 423271.557970338:      7f2d95f1621f __GI___ioctl+0xf (/lib/x86_64-linux-gnu/libc-2.27.so) insn: c3
                perf 17423 [000] 423271.557970338:      5593ad3346d7 perf_evsel__enable_cpu+0x97 (/work/linux/tools/perf/perf) insn: 85 c0
                perf 17423 [000] 423271.557970338:      5593ad3346d9 perf_evsel__enable_cpu+0x99 (/work/linux/tools/perf/perf) insn: 75 12
                perf 17423 [000] 423271.557970338:      5593ad3346db perf_evsel__enable_cpu+0x9b (/work/linux/tools/perf/perf) insn: 49 8b 84 24 a8 00 00 00
                perf 17423 [000] 423271.557970338:      5593ad3346e3 perf_evsel__enable_cpu+0xa3 (/work/linux/tools/perf/perf) insn: 48 8b 50 20

Display mnemonic instructions:
    $ sudo perf script --insn-trace=disasm
                perf 17423 [000] 423271.557970005:      7f2d95f16217 __GI___ioctl+0x7 (/lib/x86_64-linux-gnu/libc-2.27.so)		cmpq $-0xfff, %rax
                perf 17423 [000] 423271.557970005:      7f2d95f1621d __GI___ioctl+0xd (/lib/x86_64-linux-gnu/libc-2.27.so)		jae __GI___ioctl+0x10
                perf 17423 [000] 423271.557970338:      7f2d95f1621f __GI___ioctl+0xf (/lib/x86_64-linux-gnu/libc-2.27.so)		retq
                perf 17423 [000] 423271.557970338:      5593ad3346d7 perf_evsel__enable_cpu+0x97 (/work/linux/tools/perf/perf)		testl %eax, %eax
                perf 17423 [000] 423271.557970338:      5593ad3346d9 perf_evsel__enable_cpu+0x99 (/work/linux/tools/perf/perf)		jne perf_evsel__enable_cpu+0xad
                perf 17423 [000] 423271.557970338:      5593ad3346db perf_evsel__enable_cpu+0x9b (/work/linux/tools/perf/perf)		movq 0xa8(%r12), %rax
                perf 17423 [000] 423271.557970338:      5593ad3346e3 perf_evsel__enable_cpu+0xa3 (/work/linux/tools/perf/perf)		movq 0x20(%rax), %rdx
                perf 17423 [000] 423271.557970338:      5593ad3346e7 perf_evsel__enable_cpu+0xa7 (/work/linux/tools/perf/perf)		cmpl %edx, %ebx
                perf 17423 [000] 423271.557970338:      5593ad3346e9 perf_evsel__enable_cpu+0xa9 (/work/linux/tools/perf/perf)		jl perf_evsel__enable_cpu+0x60
                perf 17423 [000] 423271.557970338:      5593ad3346eb perf_evsel__enable_cpu+0xab (/work/linux/tools/perf/perf)		xorl %eax, %eax

v5:
  - fixes and improments suggested by Adrian Hunter
v4:
  - rename 'insn_disam' to 'disasm' (Adrian Hunter)
v3:
  - fix s390 detection. (Thomas Richter)
v2:
  - add a new field 'insn_disam' instead of changing the default output.
  - preserve the old --xed option.

Changbin Du (5):
  perf: build: introduce the libcapstone
  perf: util: use capstone disasm engine to show assembly instructions
  perf: script: add field 'disasm' to display mnemonic instructions
  perf: script: add raw|disasm arguments to --insn-trace option
  perf: script: prefer capstone to XED

 tools/build/Makefile.feature               |   2 +
 tools/build/feature/Makefile               |   4 +
 tools/build/feature/test-all.c             |   4 +
 tools/build/feature/test-libcapstone.c     |  11 ++
 tools/perf/Documentation/perf-intel-pt.txt |  14 ++-
 tools/perf/Documentation/perf-script.txt   |  20 ++--
 tools/perf/Makefile.config                 |  21 ++++
 tools/perf/Makefile.perf                   |   3 +
 tools/perf/builtin-script.c                |  33 +++--
 tools/perf/tests/make                      |   2 +
 tools/perf/ui/browsers/res_sample.c        |   2 +-
 tools/perf/ui/browsers/scripts.c           |   2 +-
 tools/perf/util/Build                      |   1 +
 tools/perf/util/print_insn.c               | 133 +++++++++++++++++++++
 tools/perf/util/print_insn.h               |  16 +++
 15 files changed, 242 insertions(+), 26 deletions(-)
 create mode 100644 tools/build/feature/test-libcapstone.c
 create mode 100644 tools/perf/util/print_insn.c
 create mode 100644 tools/perf/util/print_insn.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v5 1/5] perf: build: introduce the libcapstone
  2024-01-22 11:20 [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
@ 2024-01-22 11:20 ` Changbin Du
  2024-02-05  9:21   ` Adrian Hunter
  2024-01-22 11:20 ` [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Changbin Du @ 2024-01-22 11:20 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
	Andi Kleen, Thomas Richter, changbin.du, Changbin Du

Later we will use libcapstone to disassemble instructions of samples.

Signed-off-by: Changbin Du <changbin.du@huawei.com>

---
v2:
  - change tools/perf/tests/make also.
---
 tools/build/Makefile.feature           |  2 ++
 tools/build/feature/Makefile           |  4 ++++
 tools/build/feature/test-all.c         |  4 ++++
 tools/build/feature/test-libcapstone.c | 11 +++++++++++
 tools/perf/Makefile.config             | 21 +++++++++++++++++++++
 tools/perf/Makefile.perf               |  3 +++
 tools/perf/tests/make                  |  2 ++
 7 files changed, 47 insertions(+)
 create mode 100644 tools/build/feature/test-libcapstone.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 64df118376df..1e2ab148d5db 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -87,6 +87,7 @@ FEATURE_TESTS_EXTRA :=                  \
          gtk2-infobar                   \
          hello                          \
          libbabeltrace                  \
+         libcapstone                    \
          libbfd-liberty                 \
          libbfd-liberty-z               \
          libopencsd                     \
@@ -134,6 +135,7 @@ FEATURE_DISPLAY ?=              \
          libcrypto              \
          libunwind              \
          libdw-dwarf-unwind     \
+         libcapstone            \
          zlib                   \
          lzma                   \
          get_cpuid              \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 37722e509eb9..ed54cef450f5 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -54,6 +54,7 @@ FILES=                                          \
          test-timerfd.bin                       \
          test-libdw-dwarf-unwind.bin            \
          test-libbabeltrace.bin                 \
+         test-libcapstone.bin			\
          test-compile-32.bin                    \
          test-compile-x32.bin                   \
          test-zlib.bin                          \
@@ -286,6 +287,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin:
 $(OUTPUT)test-libbabeltrace.bin:
 	$(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
 
+$(OUTPUT)test-libcapstone.bin:
+	$(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone)
+
 $(OUTPUT)test-compile-32.bin:
 	$(CC) -m32 -o $@ test-compile.c
 
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 6f4bf386a3b5..dd0a18c2ef8f 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -134,6 +134,10 @@
 #undef main
 #endif
 
+#define main main_test_libcapstone
+# include "test-libcapstone.c"
+#undef main
+
 #define main main_test_lzma
 # include "test-lzma.c"
 #undef main
diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c
new file mode 100644
index 000000000000..fbe8dba189e9
--- /dev/null
+++ b/tools/build/feature/test-libcapstone.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <capstone/capstone.h>
+
+int main(void)
+{
+	csh handle;
+
+	cs_open(CS_ARCH_X86, CS_MODE_64, &handle);
+	return 0;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index aa55850fbc21..3e1072c59757 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -191,6 +191,15 @@ endif
 FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS)
 FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf
 
+# for linking with debug library, run like:
+# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/
+ifdef LIBCAPSTONE_DIR
+  LIBCAPSTONE_CFLAGS  := -I$(LIBCAPSTONE_DIR)/include
+  LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/
+endif
+FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS)
+FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone
+
 ifdef LIBZSTD_DIR
   LIBZSTD_CFLAGS  := -I$(LIBZSTD_DIR)/lib
   LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib
@@ -1094,6 +1103,18 @@ ifndef NO_LIBBABELTRACE
   endif
 endif
 
+ifndef NO_CAPSTONE
+  $(call feature_check,libcapstone)
+  ifeq ($(feature-libcapstone), 1)
+    CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS)
+    LDFLAGS += $(LICAPSTONE_LDFLAGS)
+    EXTLIBS += -lcapstone
+    $(call detected,CONFIG_LIBCAPSTONE)
+  else
+    msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel);
+  endif
+endif
+
 ifndef NO_AUXTRACE
   ifeq ($(SRCARCH),x86)
     ifeq ($(feature-get_cpuid), 0)
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 27e7c478880f..56c2720c1d0f 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -84,6 +84,9 @@ include ../scripts/utilities.mak
 # Define NO_LIBBABELTRACE if you do not want libbabeltrace support
 # for CTF data format.
 #
+# Define NO_CAPSTONE if you do not want libcapstone support
+# for disasm engine.
+#
 # Define NO_LZMA if you do not want to support compressed (xz) kernel modules
 #
 # Define NO_AUXTRACE if you do not want AUX area tracing support
diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 8a4da7eb637a..b08026f5d4e7 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -83,6 +83,7 @@ make_no_libelf      := NO_LIBELF=1
 make_no_libunwind   := NO_LIBUNWIND=1
 make_no_libdw_dwarf_unwind := NO_LIBDW_DWARF_UNWIND=1
 make_no_backtrace   := NO_BACKTRACE=1
+make_no_libcapstone := NO_CAPSTONE=1
 make_no_libnuma     := NO_LIBNUMA=1
 make_no_libaudit    := NO_LIBAUDIT=1
 make_no_libbionic   := NO_LIBBIONIC=1
@@ -152,6 +153,7 @@ run += make_no_libelf
 run += make_no_libunwind
 run += make_no_libdw_dwarf_unwind
 run += make_no_backtrace
+run += make_no_libcapstone
 run += make_no_libnuma
 run += make_no_libaudit
 run += make_no_libbionic
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions
  2024-01-22 11:20 [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
  2024-01-22 11:20 ` [PATCH v5 1/5] perf: build: introduce the libcapstone Changbin Du
@ 2024-01-22 11:20 ` Changbin Du
  2024-02-05  9:22   ` Adrian Hunter
  2024-01-22 11:20 ` [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Changbin Du @ 2024-01-22 11:20 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
	Andi Kleen, Thomas Richter, changbin.du, Changbin Du

Currently, the instructions of samples are shown as raw hex strings
which are hard to read. x86 has a special option '--xed' to disassemble
the hex string via intel XED tool.

Here we use capstone as our disassembler engine to give more friendly
instructions. We select libcapstone because capstone can provide more
insn details. Perf will fallback to raw instructions if libcapstone is
not available.

The advantages compared to XED tool:
 * Support arm, arm64, x86-32, x86_64 (more could be supported),
   xed only for x86_64.
 * Immediate address operands are shown as symbol+offs.

Signed-off-by: Changbin Du <changbin.du@huawei.com>

---
v2:
  - line up the output by preceding two tabs. (Adrian Hunter)
  - removed the tailing space. (Adrian Hunter)
  - forward declaration for perf_sample, thread, machine. (Adrian Hunter)
  - other trivial fixes (Adrian Hunter)
---
 tools/perf/builtin-script.c  |   8 +--
 tools/perf/util/Build        |   1 +
 tools/perf/util/print_insn.c | 133 +++++++++++++++++++++++++++++++++++
 tools/perf/util/print_insn.h |  16 +++++
 4 files changed, 153 insertions(+), 5 deletions(-)
 create mode 100644 tools/perf/util/print_insn.c
 create mode 100644 tools/perf/util/print_insn.h

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b1f57401ff23..4817a37f16e2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -34,6 +34,7 @@
 #include "util/event.h"
 #include "ui/ui.h"
 #include "print_binary.h"
+#include "print_insn.h"
 #include "archinsn.h"
 #include <linux/bitmap.h>
 #include <linux/kernel.h>
@@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
 	if (PRINT_FIELD(INSNLEN))
 		printed += fprintf(fp, " ilen: %d", sample->insn_len);
 	if (PRINT_FIELD(INSN) && sample->insn_len) {
-		int i;
-
-		printed += fprintf(fp, " insn:");
-		for (i = 0; i < sample->insn_len; i++)
-			printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
+		printed += fprintf(fp, " insn: ");
+		printed += sample__fprintf_insn_raw(sample, fp);
 	}
 	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
 		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 8027f450fa3e..2cbeeb79b6ef 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -32,6 +32,7 @@ perf-y += perf_regs.o
 perf-y += perf-regs-arch/
 perf-y += path.o
 perf-y += print_binary.o
+perf-y += print_insn.o
 perf-y += rlimit.o
 perf-y += argv_split.o
 perf-y += rbtree.o
diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
new file mode 100644
index 000000000000..36b403d4a4df
--- /dev/null
+++ b/tools/perf/util/print_insn.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Instruction binary disassembler based on capstone.
+ *
+ * Author(s): Changbin Du <changbin.du@huawei.com>
+ */
+#include <string.h>
+#include <stdbool.h>
+#include "debug.h"
+#include "event.h"
+#include "symbol.h"
+#include "machine.h"
+#include "thread.h"
+#include "print_insn.h"
+
+size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
+{
+	int printed = 0;
+
+	for (int i = 0; i < sample->insn_len; i++) {
+		printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
+		if (sample->insn_len - i > 1)
+			printed += fprintf(fp, " ");
+	}
+	return printed;
+}
+
+#ifdef HAVE_LIBCAPSTONE_SUPPORT
+#include <capstone/capstone.h>
+
+static int capstone_init(struct machine *machine, csh *cs_handle)
+{
+	cs_arch arch;
+	cs_mode mode;
+
+	if (machine__is(machine, "x86_64")) {
+		arch = CS_ARCH_X86;
+		mode = CS_MODE_64;
+	} else if (machine__normalized_is(machine, "x86")) {
+		arch = CS_ARCH_X86;
+		mode = CS_MODE_32;
+	} else if (machine__normalized_is(machine, "arm64")) {
+		arch = CS_ARCH_ARM64;
+		mode = CS_MODE_ARM;
+	} else if (machine__normalized_is(machine, "arm")) {
+		arch = CS_ARCH_ARM;
+		mode = CS_MODE_ARM + CS_MODE_V8;
+	} else if (machine__normalized_is(machine, "s390")) {
+		arch = CS_ARCH_SYSZ;
+		mode = CS_MODE_BIG_ENDIAN;
+	} else {
+		return -1;
+	}
+
+	if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
+		pr_warning_once("cs_open failed\n");
+		return -1;
+	}
+
+	if (machine__normalized_is(machine, "x86")) {
+		cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
+		/*
+		 * Resolving address operands to symbols is implemented
+		 * on x86 by investigating instruction details.
+		 */
+		cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
+	}
+
+	return 0;
+}
+
+static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
+			     cs_insn *insn, FILE *fp)
+{
+	struct addr_location al;
+	size_t printed = 0;
+
+	if (insn->detail && insn->detail->x86.op_count == 1) {
+		cs_x86_op *op = &insn->detail->x86.operands[0];
+
+		addr_location__init(&al);
+		if (op->type == X86_OP_IMM &&
+		    thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
+			printed += fprintf(fp, "%s ", insn[0].mnemonic);
+			printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
+			addr_location__exit(&al);
+			return printed;
+		}
+		addr_location__exit(&al);
+	}
+
+	printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
+	return printed;
+}
+
+size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
+			    struct machine *machine, FILE *fp)
+{
+	csh cs_handle;
+	cs_insn *insn;
+	size_t count;
+	size_t printed = 0;
+	int ret;
+
+	/* TODO: Try to initiate capstone only once but need a proper place. */
+	ret = capstone_init(machine, &cs_handle);
+	if (ret < 0) {
+		/* fallback */
+		return sample__fprintf_insn_raw(sample, fp);
+	}
+
+	count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
+			  sample->ip, 1, &insn);
+	if (count > 0) {
+		if (machine__normalized_is(machine, "x86"))
+			printed += print_insn_x86(sample, thread, &insn[0], fp);
+		else
+			printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
+		cs_free(insn, count);
+	} else {
+		printed += fprintf(fp, "illegal instruction");
+	}
+
+	cs_close(&cs_handle);
+	return printed;
+}
+#else
+size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
+			    struct machine *machine __maybe_unused, FILE *fp)
+{
+	return sample__fprintf_insn_raw(sample, fp);
+}
+#endif
diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
new file mode 100644
index 000000000000..80dda6da7c60
--- /dev/null
+++ b/tools/perf/util/print_insn.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef PERF_PRINT_INSN_H
+#define PERF_PRINT_INSN_H
+
+#include <stddef.h>
+#include <stdio.h>
+
+struct perf_sample;
+struct thread;
+struct machine;
+
+size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
+			    struct machine *machine, FILE *fp);
+size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
+
+#endif /* PERF_PRINT_INSN_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions
  2024-01-22 11:20 [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
  2024-01-22 11:20 ` [PATCH v5 1/5] perf: build: introduce the libcapstone Changbin Du
  2024-01-22 11:20 ` [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
@ 2024-01-22 11:20 ` Changbin Du
  2024-02-05  9:23   ` Adrian Hunter
  2024-01-22 11:20 ` [PATCH v5 4/5] perf: script: add raw|disasm arguments to --insn-trace option Changbin Du
  2024-01-22 11:20 ` [PATCH v5 5/5] perf: script: prefer capstone to XED Changbin Du
  4 siblings, 1 reply; 15+ messages in thread
From: Changbin Du @ 2024-01-22 11:20 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
	Andi Kleen, Thomas Richter, changbin.du, Changbin Du

In addition to the 'insn' field, this adds a new field 'disasm' to
display mnemonic instructions instead of the raw code.

$ sudo perf script -F +disasm
       perf-exec 1443864 [006] 2275506.209848:          psb:  psb offs: 0                                      0 [unknown] ([unknown])
       perf-exec 1443864 [006] 2275506.209848:          cbr:  cbr: 41 freq: 4100 MHz (114%)                    0 [unknown] ([unknown])
              ls 1443864 [006] 2275506.209905:          1  branches:uH:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	movq %rsp, %rdi
              ls 1443864 [006] 2275506.209908:          1  branches:uH:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	callq _dl_start+0x0

Signed-off-by: Changbin Du <changbin.du@huawei.com>

---
v2:
  - update Documentation.
---
 tools/perf/Documentation/perf-script.txt | 13 +++++++------
 tools/perf/builtin-script.c              |  8 +++++++-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index ff9a52e44688..578fa59f51a5 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -132,9 +132,10 @@ OPTIONS
         Comma separated list of fields to print. Options are:
         comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
         srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
-        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
-        phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
-        machine_pid, vcpu, cgroup, retire_lat.
+        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
+        insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
+        code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
+
         Field list can be prepended with the type, trace, sw or hw,
         to indicate to which event type the field list applies.
         e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
@@ -217,9 +218,9 @@ OPTIONS
 	Instruction Trace decoding. For calls and returns, it will display the
 	name of the symbol indented with spaces to reflect the stack depth.
 
-	When doing instruction trace decoding insn and insnlen give the
-	instruction bytes and the instruction length of the current
-	instruction.
+	When doing instruction trace decoding, insn, disasm and insnlen give the
+	instruction bytes, disassembled instructions (requires libcapstone support)
+	and the instruction length of the current instruction respectively.
 
 	The synth field is used by synthesized events which may be created when
 	Instruction Trace decoding.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4817a37f16e2..4ac9670704ff 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -135,6 +135,7 @@ enum perf_output_field {
 	PERF_OUTPUT_CGROUP          = 1ULL << 39,
 	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
 	PERF_OUTPUT_DSOFF           = 1ULL << 41,
+	PERF_OUTPUT_DISASM          = 1ULL << 42,
 };
 
 struct perf_script {
@@ -190,6 +191,7 @@ struct output_option {
 	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
 	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
 	{.str = "insn", .field = PERF_OUTPUT_INSN},
+	{.str = "disasm", .field = PERF_OUTPUT_DISASM},
 	{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
 	{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
 	{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
@@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
 		printed += fprintf(fp, " insn: ");
 		printed += sample__fprintf_insn_raw(sample, fp);
 	}
+	if (PRINT_FIELD(DISASM) && sample->insn_len) {
+		printed += fprintf(fp, "\t\t");
+		printed += sample__fprintf_insn(sample, thread, machine, fp);
+	}
 	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
 		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
 
@@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
 		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
 		     "addr,symoff,srcline,period,iregs,uregs,brstack,"
 		     "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
-		     "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
+		     "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
 		     "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
 		     "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
 		     parse_output_fields),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 4/5] perf: script: add raw|disasm arguments to --insn-trace option
  2024-01-22 11:20 [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
                   ` (2 preceding siblings ...)
  2024-01-22 11:20 ` [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
@ 2024-01-22 11:20 ` Changbin Du
  2024-01-22 11:20 ` [PATCH v5 5/5] perf: script: prefer capstone to XED Changbin Du
  4 siblings, 0 replies; 15+ messages in thread
From: Changbin Du @ 2024-01-22 11:20 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
	Andi Kleen, Thomas Richter, changbin.du, Changbin Du

Now '--insn-trace' accept a argument to specify the output format:
  - raw: display raw instructions.
  - disasm: display mnemonic instructions (if capstone is installed).

$ sudo perf script --insn-trace=raw
              ls 1443864 [006] 2275506.209908875:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: 48 89 e7
              ls 1443864 [006] 2275506.209908875:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: e8 e8 0c 00 00
              ls 1443864 [006] 2275506.209908875:      7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so) insn: f3 0f 1e fa

$ sudo perf script --insn-trace=disasm
              ls 1443864 [006] 2275506.209908875:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)		movq %rsp, %rdi
              ls 1443864 [006] 2275506.209908875:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)		callq _dl_start+0x0
              ls 1443864 [006] 2275506.209908875:      7f216b426df0 _dl_start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	illegal instruction
              ls 1443864 [006] 2275506.209908875:      7f216b426df4 _dl_start+0x4 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	pushq %rbp
              ls 1443864 [006] 2275506.209908875:      7f216b426df5 _dl_start+0x5 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	movq %rsp, %rbp
              ls 1443864 [006] 2275506.209908875:      7f216b426df8 _dl_start+0x8 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	pushq %r15

Signed-off-by: Changbin Du <changbin.du@huawei.com>

---
v2:
  - update Documentation (Adrian Hunter)
---
 tools/perf/Documentation/perf-script.txt |  7 ++++---
 tools/perf/builtin-script.c              | 17 +++++++++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 578fa59f51a5..005e51df855e 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -442,9 +442,10 @@ include::itrace.txt[]
 	will be printed. Each entry has function name and file/line. Enabled by
 	default, disable with --no-inline.
 
---insn-trace::
-	Show instruction stream for intel_pt traces. Combine with --xed to
-	show disassembly.
+--insn-trace[=<raw|disasm>]::
+	Show instruction stream in bytes (raw) or disassembled (disasm)
+	for intel_pt traces. The default is 'raw'. To use xed, combine
+	'raw' with --xed to show disassembly done by xed.
 
 --xed::
 	Run xed disassembler on output. Requires installing the xed disassembler.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4ac9670704ff..fd684293d4d9 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3769,10 +3769,19 @@ static int perf_script__process_auxtrace_info(struct perf_session *session,
 #endif
 
 static int parse_insn_trace(const struct option *opt __maybe_unused,
-			    const char *str __maybe_unused,
-			    int unset __maybe_unused)
+			    const char *str, int unset __maybe_unused)
 {
-	parse_output_fields(NULL, "+insn,-event,-period", 0);
+	const char *fields = "+insn,-event,-period";
+
+	if (str) {
+		if (strcmp(str, "disasm") == 0)
+			fields = "+disasm,-event,-period";
+		else if (strlen(str) != 0 && strcmp(str, "raw") != 0) {
+			fprintf(stderr, "Only accept raw|disasm\n");
+			return -EINVAL;
+		}
+	}
+	parse_output_fields(NULL, fields, 0);
 	itrace_parse_synth_opts(opt, "i0ns", 0);
 	symbol_conf.nanosecs = true;
 	return 0;
@@ -3918,7 +3927,7 @@ int cmd_script(int argc, const char **argv)
 		   "only consider these symbols"),
 	OPT_INTEGER(0, "addr-range", &symbol_conf.addr_range,
 		    "Use with -S to list traced records within address range"),
-	OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, NULL,
+	OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, "raw|disasm",
 			"Decode instructions from itrace", parse_insn_trace),
 	OPT_CALLBACK_OPTARG(0, "xed", NULL, NULL, NULL,
 			"Run xed disassembler on output", parse_xed),
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 5/5] perf: script: prefer capstone to XED
  2024-01-22 11:20 [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
                   ` (3 preceding siblings ...)
  2024-01-22 11:20 ` [PATCH v5 4/5] perf: script: add raw|disasm arguments to --insn-trace option Changbin Du
@ 2024-01-22 11:20 ` Changbin Du
  4 siblings, 0 replies; 15+ messages in thread
From: Changbin Du @ 2024-01-22 11:20 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, Adrian Hunter, linux-kernel, linux-perf-users,
	Andi Kleen, Thomas Richter, changbin.du, Changbin Du

Now perf can show assembly instructions with libcapstone for x86, and the
capstone is better in general.

Signed-off-by: Changbin Du <changbin.du@huawei.com>

---
v2:
  - update Documentation.  (Adrian Hunter)
  - fix typo.
---
 tools/perf/Documentation/perf-intel-pt.txt | 14 +++++++++-----
 tools/perf/ui/browsers/res_sample.c        |  2 +-
 tools/perf/ui/browsers/scripts.c           |  2 +-
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/perf/Documentation/perf-intel-pt.txt b/tools/perf/Documentation/perf-intel-pt.txt
index 2109690b0d5f..59ab1ff9d75f 100644
--- a/tools/perf/Documentation/perf-intel-pt.txt
+++ b/tools/perf/Documentation/perf-intel-pt.txt
@@ -115,9 +115,13 @@ toggle respectively.
 
 perf script also supports higher level ways to dump instruction traces:
 
+	perf script --insn-trace=disasm
+
+or to use the xed disassembler, which requires installing the xed tool
+(see XED below):
+
 	perf script --insn-trace --xed
 
-Dump all instructions. This requires installing the xed tool (see XED below)
 Dumping all instructions in a long trace can be fairly slow. It is usually better
 to start with higher level decoding, like
 
@@ -130,12 +134,12 @@ or
 and then select a time range of interest. The time range can then be examined
 in detail with
 
-	perf script --time starttime,stoptime --insn-trace --xed
+	perf script --time starttime,stoptime --insn-trace=disasm
 
 While examining the trace it's also useful to filter on specific CPUs using
 the -C option
 
-	perf script --time starttime,stoptime --insn-trace --xed -C 1
+	perf script --time starttime,stoptime --insn-trace=disasm -C 1
 
 Dump all instructions in time range on CPU 1.
 
@@ -1306,7 +1310,7 @@ Without timestamps, --per-thread must be specified to distinguish threads.
 
 perf script can be used to provide an instruction trace
 
- $ perf script --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
+ $ perf script --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
        CPU 0/KVM  1440  ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms])                movq  0x48(%rax), %r9
        CPU 0/KVM  1440  ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms])                movq  0x50(%rax), %r10
        CPU 0/KVM  1440  ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms])                movq  0x58(%rax), %r11
@@ -1407,7 +1411,7 @@ There were none.
 
 'perf script' can be used to provide an instruction trace showing timestamps
 
- $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace --xed -F+ipc | grep -C10 vmresume | head -21
+ $ perf script -i perf.data.kvm --guestkallsyms $KALLSYMS --insn-trace=disasm -F+ipc | grep -C10 vmresume | head -21
        CPU 1/KVM 17006 [001] 11500.262865593:  ffffffff82133cdd __vmx_vcpu_run+0x3d ([kernel.kallsyms])                 movq  0x48(%rax), %r9
        CPU 1/KVM 17006 [001] 11500.262865593:  ffffffff82133ce1 __vmx_vcpu_run+0x41 ([kernel.kallsyms])                 movq  0x50(%rax), %r10
        CPU 1/KVM 17006 [001] 11500.262865593:  ffffffff82133ce5 __vmx_vcpu_run+0x45 ([kernel.kallsyms])                 movq  0x58(%rax), %r11
diff --git a/tools/perf/ui/browsers/res_sample.c b/tools/perf/ui/browsers/res_sample.c
index 7cb2d6678039..5f60e515b12e 100644
--- a/tools/perf/ui/browsers/res_sample.c
+++ b/tools/perf/ui/browsers/res_sample.c
@@ -83,7 +83,7 @@ int res_sample_browse(struct res_sample *res_samples, int num_res,
 		     r->tid ? "--tid " : "",
 		     r->tid ? (sprintf(tidbuf, "%d", r->tid), tidbuf) : "",
 		     extra_format,
-		     rstype == A_ASM ? "-F +insn --xed" :
+		     rstype == A_ASM ? "-F +disasm" :
 		     rstype == A_SOURCE ? "-F +srcline,+srccode" : "",
 		     symbol_conf.inline_name ? "--inline" : "",
 		     "--show-lost-events ",
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 50d45054ed6c..e437d7889de6 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -107,7 +107,7 @@ static int list_scripts(char *script_name, bool *custom,
 	if (evsel)
 		attr_to_script(scriptc.extra_format, &evsel->core.attr);
 	add_script_option("Show individual samples", "", &scriptc);
-	add_script_option("Show individual samples with assembler", "-F +insn --xed",
+	add_script_option("Show individual samples with assembler", "-F +disasm",
 			  &scriptc);
 	add_script_option("Show individual samples with source", "-F +srcline,+srccode",
 			  &scriptc);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 1/5] perf: build: introduce the libcapstone
  2024-01-22 11:20 ` [PATCH v5 1/5] perf: build: introduce the libcapstone Changbin Du
@ 2024-02-05  9:21   ` Adrian Hunter
  2024-02-05 11:51     ` Changbin Du
  0 siblings, 1 reply; 15+ messages in thread
From: Adrian Hunter @ 2024-02-05  9:21 UTC (permalink / raw)
  To: Changbin Du, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
	Thomas Richter, changbin.du

On 22/01/24 13:20, Changbin Du wrote:
> Later we will use libcapstone to disassemble instructions of samples.
> 
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> 
> ---
> v2:
>   - change tools/perf/tests/make also.
> ---
>  tools/build/Makefile.feature           |  2 ++
>  tools/build/feature/Makefile           |  4 ++++
>  tools/build/feature/test-all.c         |  4 ++++
>  tools/build/feature/test-libcapstone.c | 11 +++++++++++
>  tools/perf/Makefile.config             | 21 +++++++++++++++++++++
>  tools/perf/Makefile.perf               |  3 +++
>  tools/perf/tests/make                  |  2 ++
>  7 files changed, 47 insertions(+)
>  create mode 100644 tools/build/feature/test-libcapstone.c

Perhaps add it also to display with perf version --build-options

> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 64df118376df..1e2ab148d5db 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -87,6 +87,7 @@ FEATURE_TESTS_EXTRA :=                  \
>           gtk2-infobar                   \
>           hello                          \
>           libbabeltrace                  \
> +         libcapstone                    \
>           libbfd-liberty                 \
>           libbfd-liberty-z               \
>           libopencsd                     \
> @@ -134,6 +135,7 @@ FEATURE_DISPLAY ?=              \
>           libcrypto              \
>           libunwind              \
>           libdw-dwarf-unwind     \
> +         libcapstone            \
>           zlib                   \
>           lzma                   \
>           get_cpuid              \
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 37722e509eb9..ed54cef450f5 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -54,6 +54,7 @@ FILES=                                          \
>           test-timerfd.bin                       \
>           test-libdw-dwarf-unwind.bin            \
>           test-libbabeltrace.bin                 \
> +         test-libcapstone.bin			\
>           test-compile-32.bin                    \
>           test-compile-x32.bin                   \
>           test-zlib.bin                          \
> @@ -286,6 +287,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin:
>  $(OUTPUT)test-libbabeltrace.bin:
>  	$(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
>  
> +$(OUTPUT)test-libcapstone.bin:
> +	$(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone)
> +
>  $(OUTPUT)test-compile-32.bin:
>  	$(CC) -m32 -o $@ test-compile.c
>  
> diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> index 6f4bf386a3b5..dd0a18c2ef8f 100644
> --- a/tools/build/feature/test-all.c
> +++ b/tools/build/feature/test-all.c
> @@ -134,6 +134,10 @@
>  #undef main
>  #endif
>  
> +#define main main_test_libcapstone
> +# include "test-libcapstone.c"
> +#undef main
> +
>  #define main main_test_lzma
>  # include "test-lzma.c"
>  #undef main
> diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c
> new file mode 100644
> index 000000000000..fbe8dba189e9
> --- /dev/null
> +++ b/tools/build/feature/test-libcapstone.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <capstone/capstone.h>
> +
> +int main(void)
> +{
> +	csh handle;
> +
> +	cs_open(CS_ARCH_X86, CS_MODE_64, &handle);
> +	return 0;
> +}
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index aa55850fbc21..3e1072c59757 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -191,6 +191,15 @@ endif
>  FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS)
>  FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf
>  
> +# for linking with debug library, run like:
> +# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/
> +ifdef LIBCAPSTONE_DIR
> +  LIBCAPSTONE_CFLAGS  := -I$(LIBCAPSTONE_DIR)/include
> +  LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/
> +endif
> +FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS)
> +FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone
> +
>  ifdef LIBZSTD_DIR
>    LIBZSTD_CFLAGS  := -I$(LIBZSTD_DIR)/lib
>    LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib
> @@ -1094,6 +1103,18 @@ ifndef NO_LIBBABELTRACE
>    endif
>  endif
>  
> +ifndef NO_CAPSTONE
> +  $(call feature_check,libcapstone)
> +  ifeq ($(feature-libcapstone), 1)
> +    CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS)
> +    LDFLAGS += $(LICAPSTONE_LDFLAGS)
> +    EXTLIBS += -lcapstone
> +    $(call detected,CONFIG_LIBCAPSTONE)
> +  else
> +    msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel);
> +  endif
> +endif
> +
>  ifndef NO_AUXTRACE
>    ifeq ($(SRCARCH),x86)
>      ifeq ($(feature-get_cpuid), 0)
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 27e7c478880f..56c2720c1d0f 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -84,6 +84,9 @@ include ../scripts/utilities.mak
>  # Define NO_LIBBABELTRACE if you do not want libbabeltrace support
>  # for CTF data format.
>  #
> +# Define NO_CAPSTONE if you do not want libcapstone support
> +# for disasm engine.
> +#
>  # Define NO_LZMA if you do not want to support compressed (xz) kernel modules
>  #
>  # Define NO_AUXTRACE if you do not want AUX area tracing support
> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index 8a4da7eb637a..b08026f5d4e7 100644
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make
> @@ -83,6 +83,7 @@ make_no_libelf      := NO_LIBELF=1
>  make_no_libunwind   := NO_LIBUNWIND=1
>  make_no_libdw_dwarf_unwind := NO_LIBDW_DWARF_UNWIND=1
>  make_no_backtrace   := NO_BACKTRACE=1
> +make_no_libcapstone := NO_CAPSTONE=1
>  make_no_libnuma     := NO_LIBNUMA=1
>  make_no_libaudit    := NO_LIBAUDIT=1
>  make_no_libbionic   := NO_LIBBIONIC=1

Also needs to be added to make_minimal

> @@ -152,6 +153,7 @@ run += make_no_libelf
>  run += make_no_libunwind
>  run += make_no_libdw_dwarf_unwind
>  run += make_no_backtrace
> +run += make_no_libcapstone
>  run += make_no_libnuma
>  run += make_no_libaudit
>  run += make_no_libbionic


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions
  2024-01-22 11:20 ` [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
@ 2024-02-05  9:22   ` Adrian Hunter
  2024-02-05 11:36     ` Changbin Du
  0 siblings, 1 reply; 15+ messages in thread
From: Adrian Hunter @ 2024-02-05  9:22 UTC (permalink / raw)
  To: Changbin Du, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
	Thomas Richter, changbin.du

On 22/01/24 13:20, Changbin Du wrote:
> Currently, the instructions of samples are shown as raw hex strings
> which are hard to read. x86 has a special option '--xed' to disassemble
> the hex string via intel XED tool.
> 
> Here we use capstone as our disassembler engine to give more friendly
> instructions. We select libcapstone because capstone can provide more
> insn details. Perf will fallback to raw instructions if libcapstone is
> not available.
> 
> The advantages compared to XED tool:
>  * Support arm, arm64, x86-32, x86_64 (more could be supported),
>    xed only for x86_64.
>  * Immediate address operands are shown as symbol+offs.
> 
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> 
> ---
> v2:
>   - line up the output by preceding two tabs. (Adrian Hunter)
>   - removed the tailing space. (Adrian Hunter)
>   - forward declaration for perf_sample, thread, machine. (Adrian Hunter)
>   - other trivial fixes (Adrian Hunter)
> ---
>  tools/perf/builtin-script.c  |   8 +--
>  tools/perf/util/Build        |   1 +
>  tools/perf/util/print_insn.c | 133 +++++++++++++++++++++++++++++++++++
>  tools/perf/util/print_insn.h |  16 +++++
>  4 files changed, 153 insertions(+), 5 deletions(-)
>  create mode 100644 tools/perf/util/print_insn.c
>  create mode 100644 tools/perf/util/print_insn.h
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index b1f57401ff23..4817a37f16e2 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -34,6 +34,7 @@
>  #include "util/event.h"
>  #include "ui/ui.h"
>  #include "print_binary.h"
> +#include "print_insn.h"
>  #include "archinsn.h"
>  #include <linux/bitmap.h>
>  #include <linux/kernel.h>
> @@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  	if (PRINT_FIELD(INSNLEN))
>  		printed += fprintf(fp, " ilen: %d", sample->insn_len);
>  	if (PRINT_FIELD(INSN) && sample->insn_len) {
> -		int i;
> -
> -		printed += fprintf(fp, " insn:");
> -		for (i = 0; i < sample->insn_len; i++)
> -			printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
> +		printed += fprintf(fp, " insn: ");

Better without the space after 'insn:', then
sample__fprintf_insn_raw() can add 1 space before each
byte.
		printed += fprintf(fp, " insn:");

See also further below.

> +		printed += sample__fprintf_insn_raw(sample, fp);
>  	}
>  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 8027f450fa3e..2cbeeb79b6ef 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -32,6 +32,7 @@ perf-y += perf_regs.o
>  perf-y += perf-regs-arch/
>  perf-y += path.o
>  perf-y += print_binary.o
> +perf-y += print_insn.o
>  perf-y += rlimit.o
>  perf-y += argv_split.o
>  perf-y += rbtree.o
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> new file mode 100644
> index 000000000000..36b403d4a4df
> --- /dev/null
> +++ b/tools/perf/util/print_insn.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Instruction binary disassembler based on capstone.
> + *
> + * Author(s): Changbin Du <changbin.du@huawei.com>
> + */
> +#include <string.h>
> +#include <stdbool.h>
> +#include "debug.h"
> +#include "event.h"
> +#include "symbol.h"
> +#include "machine.h"
> +#include "thread.h"
> +#include "print_insn.h"
> +
> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
> +{
> +	int printed = 0;
> +
> +	for (int i = 0; i < sample->insn_len; i++) {
> +		printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
> +		if (sample->insn_len - i > 1)
> +			printed += fprintf(fp, " ");

Now there are 2 spaces between.  Better like this:

	for (int i = 0; i < sample->insn_len; i++)
		printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);


> +	}
> +	return printed;
> +}
> +
> +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> +#include <capstone/capstone.h>
> +
> +static int capstone_init(struct machine *machine, csh *cs_handle)
> +{
> +	cs_arch arch;
> +	cs_mode mode;
> +
> +	if (machine__is(machine, "x86_64")) {
> +		arch = CS_ARCH_X86;
> +		mode = CS_MODE_64;
> +	} else if (machine__normalized_is(machine, "x86")) {
> +		arch = CS_ARCH_X86;
> +		mode = CS_MODE_32;
> +	} else if (machine__normalized_is(machine, "arm64")) {
> +		arch = CS_ARCH_ARM64;
> +		mode = CS_MODE_ARM;
> +	} else if (machine__normalized_is(machine, "arm")) {
> +		arch = CS_ARCH_ARM;
> +		mode = CS_MODE_ARM + CS_MODE_V8;
> +	} else if (machine__normalized_is(machine, "s390")) {
> +		arch = CS_ARCH_SYSZ;
> +		mode = CS_MODE_BIG_ENDIAN;
> +	} else {
> +		return -1;
> +	}
> +
> +	if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
> +		pr_warning_once("cs_open failed\n");
> +		return -1;
> +	}
> +
> +	if (machine__normalized_is(machine, "x86")) {
> +		cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
> +		/*
> +		 * Resolving address operands to symbols is implemented
> +		 * on x86 by investigating instruction details.
> +		 */
> +		cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
> +	}
> +
> +	return 0;
> +}
> +
> +static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
> +			     cs_insn *insn, FILE *fp)
> +{
> +	struct addr_location al;
> +	size_t printed = 0;
> +
> +	if (insn->detail && insn->detail->x86.op_count == 1) {
> +		cs_x86_op *op = &insn->detail->x86.operands[0];
> +
> +		addr_location__init(&al);
> +		if (op->type == X86_OP_IMM &&
> +		    thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
> +			printed += fprintf(fp, "%s ", insn[0].mnemonic);
> +			printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
> +			addr_location__exit(&al);
> +			return printed;
> +		}
> +		addr_location__exit(&al);
> +	}
> +
> +	printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> +	return printed;
> +}
> +
> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> +			    struct machine *machine, FILE *fp)
> +{
> +	csh cs_handle;
> +	cs_insn *insn;
> +	size_t count;
> +	size_t printed = 0;
> +	int ret;
> +
> +	/* TODO: Try to initiate capstone only once but need a proper place. */
> +	ret = capstone_init(machine, &cs_handle);
> +	if (ret < 0) {
> +		/* fallback */
> +		return sample__fprintf_insn_raw(sample, fp);
> +	}
> +
> +	count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
> +			  sample->ip, 1, &insn);
> +	if (count > 0) {
> +		if (machine__normalized_is(machine, "x86"))
> +			printed += print_insn_x86(sample, thread, &insn[0], fp);
> +		else
> +			printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> +		cs_free(insn, count);
> +	} else {
> +		printed += fprintf(fp, "illegal instruction");
> +	}
> +
> +	cs_close(&cs_handle);
> +	return printed;
> +}
> +#else
> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
> +			    struct machine *machine __maybe_unused, FILE *fp)
> +{
> +	return sample__fprintf_insn_raw(sample, fp);
> +}
> +#endif
> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> new file mode 100644
> index 000000000000..80dda6da7c60
> --- /dev/null
> +++ b/tools/perf/util/print_insn.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef PERF_PRINT_INSN_H
> +#define PERF_PRINT_INSN_H
> +
> +#include <stddef.h>
> +#include <stdio.h>
> +
> +struct perf_sample;
> +struct thread;
> +struct machine;
> +
> +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> +			    struct machine *machine, FILE *fp);
> +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> +
> +#endif /* PERF_PRINT_INSN_H */


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions
  2024-01-22 11:20 ` [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
@ 2024-02-05  9:23   ` Adrian Hunter
  2024-02-05 11:41     ` Changbin Du
  2024-02-05 12:19     ` Changbin Du
  0 siblings, 2 replies; 15+ messages in thread
From: Adrian Hunter @ 2024-02-05  9:23 UTC (permalink / raw)
  To: Changbin Du, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
	Thomas Richter, changbin.du

On 22/01/24 13:20, Changbin Du wrote:
> In addition to the 'insn' field, this adds a new field 'disasm' to
> display mnemonic instructions instead of the raw code.
> 
> $ sudo perf script -F +disasm
>        perf-exec 1443864 [006] 2275506.209848:          psb:  psb offs: 0                                      0 [unknown] ([unknown])
>        perf-exec 1443864 [006] 2275506.209848:          cbr:  cbr: 41 freq: 4100 MHz (114%)                    0 [unknown] ([unknown])
>               ls 1443864 [006] 2275506.209905:          1  branches:uH:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	movq %rsp, %rdi
>               ls 1443864 [006] 2275506.209908:          1  branches:uH:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	callq _dl_start+0x0
> 
> Signed-off-by: Changbin Du <changbin.du@huawei.com>
> 
> ---
> v2:
>   - update Documentation.
> ---
>  tools/perf/Documentation/perf-script.txt | 13 +++++++------
>  tools/perf/builtin-script.c              |  8 +++++++-
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index ff9a52e44688..578fa59f51a5 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -132,9 +132,10 @@ OPTIONS
>          Comma separated list of fields to print. Options are:
>          comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
>          srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
> -        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
> -        phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
> -        machine_pid, vcpu, cgroup, retire_lat.
> +        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
> +        insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
> +        code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
> +
>          Field list can be prepended with the type, trace, sw or hw,
>          to indicate to which event type the field list applies.
>          e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
> @@ -217,9 +218,9 @@ OPTIONS
>  	Instruction Trace decoding. For calls and returns, it will display the
>  	name of the symbol indented with spaces to reflect the stack depth.
>  
> -	When doing instruction trace decoding insn and insnlen give the
> -	instruction bytes and the instruction length of the current
> -	instruction.
> +	When doing instruction trace decoding, insn, disasm and insnlen give the
> +	instruction bytes, disassembled instructions (requires libcapstone support)
> +	and the instruction length of the current instruction respectively.
>  
>  	The synth field is used by synthesized events which may be created when
>  	Instruction Trace decoding.
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 4817a37f16e2..4ac9670704ff 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -135,6 +135,7 @@ enum perf_output_field {
>  	PERF_OUTPUT_CGROUP          = 1ULL << 39,
>  	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
>  	PERF_OUTPUT_DSOFF           = 1ULL << 41,
> +	PERF_OUTPUT_DISASM          = 1ULL << 42,
>  };
>  
>  struct perf_script {
> @@ -190,6 +191,7 @@ struct output_option {
>  	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
>  	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
>  	{.str = "insn", .field = PERF_OUTPUT_INSN},
> +	{.str = "disasm", .field = PERF_OUTPUT_DISASM},
>  	{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
>  	{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
>  	{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
> @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  		printed += fprintf(fp, " insn: ");
>  		printed += sample__fprintf_insn_raw(sample, fp);
>  	}
> +	if (PRINT_FIELD(DISASM) && sample->insn_len) {
> +		printed += fprintf(fp, "\t\t");

This is good, except if both 'insn' and 'disasm' are used together.
It either:
 a) without libcapstone, prints insn bytes twice

	Probably simpler to make 'disasm' without libcapstone
	a fatal error explaining that perf needs to be built
	with libcapstone support for 'disasm' to work.

 b) with libcapstone, disassembly does not line up nicely

> +		printed += sample__fprintf_insn(sample, thread, machine, fp);
> +	}
>  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>  
> @@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
>  		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
>  		     "addr,symoff,srcline,period,iregs,uregs,brstack,"
>  		     "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
> -		     "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
> +		     "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
>  		     "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
>  		     "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
>  		     parse_output_fields),


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions
  2024-02-05  9:22   ` Adrian Hunter
@ 2024-02-05 11:36     ` Changbin Du
  0 siblings, 0 replies; 15+ messages in thread
From: Changbin Du @ 2024-02-05 11:36 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Changbin Du, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users, Andi Kleen, Thomas Richter, changbin.du

On Mon, Feb 05, 2024 at 11:22:51AM +0200, Adrian Hunter wrote:
> On 22/01/24 13:20, Changbin Du wrote:
> > Currently, the instructions of samples are shown as raw hex strings
> > which are hard to read. x86 has a special option '--xed' to disassemble
> > the hex string via intel XED tool.
> > 
> > Here we use capstone as our disassembler engine to give more friendly
> > instructions. We select libcapstone because capstone can provide more
> > insn details. Perf will fallback to raw instructions if libcapstone is
> > not available.
> > 
> > The advantages compared to XED tool:
> >  * Support arm, arm64, x86-32, x86_64 (more could be supported),
> >    xed only for x86_64.
> >  * Immediate address operands are shown as symbol+offs.
> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > 
> > ---
> > v2:
> >   - line up the output by preceding two tabs. (Adrian Hunter)
> >   - removed the tailing space. (Adrian Hunter)
> >   - forward declaration for perf_sample, thread, machine. (Adrian Hunter)
> >   - other trivial fixes (Adrian Hunter)
> > ---
> >  tools/perf/builtin-script.c  |   8 +--
> >  tools/perf/util/Build        |   1 +
> >  tools/perf/util/print_insn.c | 133 +++++++++++++++++++++++++++++++++++
> >  tools/perf/util/print_insn.h |  16 +++++
> >  4 files changed, 153 insertions(+), 5 deletions(-)
> >  create mode 100644 tools/perf/util/print_insn.c
> >  create mode 100644 tools/perf/util/print_insn.h
> > 
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index b1f57401ff23..4817a37f16e2 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -34,6 +34,7 @@
> >  #include "util/event.h"
> >  #include "ui/ui.h"
> >  #include "print_binary.h"
> > +#include "print_insn.h"
> >  #include "archinsn.h"
> >  #include <linux/bitmap.h>
> >  #include <linux/kernel.h>
> > @@ -1511,11 +1512,8 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> >  	if (PRINT_FIELD(INSNLEN))
> >  		printed += fprintf(fp, " ilen: %d", sample->insn_len);
> >  	if (PRINT_FIELD(INSN) && sample->insn_len) {
> > -		int i;
> > -
> > -		printed += fprintf(fp, " insn:");
> > -		for (i = 0; i < sample->insn_len; i++)
> > -			printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
> > +		printed += fprintf(fp, " insn: ");
> 
> Better without the space after 'insn:', then
> sample__fprintf_insn_raw() can add 1 space before each
> byte.
> 		printed += fprintf(fp, " insn:");
>
This is designed to. Because I want sample__fprintf_insn_raw() to be independent
from the callsite.

> See also further below.
> 
> > +		printed += sample__fprintf_insn_raw(sample, fp);
> >  	}
> >  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> >  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index 8027f450fa3e..2cbeeb79b6ef 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -32,6 +32,7 @@ perf-y += perf_regs.o
> >  perf-y += perf-regs-arch/
> >  perf-y += path.o
> >  perf-y += print_binary.o
> > +perf-y += print_insn.o
> >  perf-y += rlimit.o
> >  perf-y += argv_split.o
> >  perf-y += rbtree.o
> > diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> > new file mode 100644
> > index 000000000000..36b403d4a4df
> > --- /dev/null
> > +++ b/tools/perf/util/print_insn.c
> > @@ -0,0 +1,133 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Instruction binary disassembler based on capstone.
> > + *
> > + * Author(s): Changbin Du <changbin.du@huawei.com>
> > + */
> > +#include <string.h>
> > +#include <stdbool.h>
> > +#include "debug.h"
> > +#include "event.h"
> > +#include "symbol.h"
> > +#include "machine.h"
> > +#include "thread.h"
> > +#include "print_insn.h"
> > +
> > +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
> > +{
> > +	int printed = 0;
> > +
> > +	for (int i = 0; i < sample->insn_len; i++) {
> > +		printed += fprintf(fp, "%02x ", (unsigned char)sample->insn[i]);
> > +		if (sample->insn_len - i > 1)
> > +			printed += fprintf(fp, " ");
> 
> Now there are 2 spaces between.  Better like this:
> 
> 	for (int i = 0; i < sample->insn_len; i++)
> 		printed += fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
> 
> 
> > +	}
> > +	return printed;
> > +}
> > +
> > +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> > +#include <capstone/capstone.h>
> > +
> > +static int capstone_init(struct machine *machine, csh *cs_handle)
> > +{
> > +	cs_arch arch;
> > +	cs_mode mode;
> > +
> > +	if (machine__is(machine, "x86_64")) {
> > +		arch = CS_ARCH_X86;
> > +		mode = CS_MODE_64;
> > +	} else if (machine__normalized_is(machine, "x86")) {
> > +		arch = CS_ARCH_X86;
> > +		mode = CS_MODE_32;
> > +	} else if (machine__normalized_is(machine, "arm64")) {
> > +		arch = CS_ARCH_ARM64;
> > +		mode = CS_MODE_ARM;
> > +	} else if (machine__normalized_is(machine, "arm")) {
> > +		arch = CS_ARCH_ARM;
> > +		mode = CS_MODE_ARM + CS_MODE_V8;
> > +	} else if (machine__normalized_is(machine, "s390")) {
> > +		arch = CS_ARCH_SYSZ;
> > +		mode = CS_MODE_BIG_ENDIAN;
> > +	} else {
> > +		return -1;
> > +	}
> > +
> > +	if (cs_open(arch, mode, cs_handle) != CS_ERR_OK) {
> > +		pr_warning_once("cs_open failed\n");
> > +		return -1;
> > +	}
> > +
> > +	if (machine__normalized_is(machine, "x86")) {
> > +		cs_option(*cs_handle, CS_OPT_SYNTAX, CS_OPT_SYNTAX_ATT);
> > +		/*
> > +		 * Resolving address operands to symbols is implemented
> > +		 * on x86 by investigating instruction details.
> > +		 */
> > +		cs_option(*cs_handle, CS_OPT_DETAIL, CS_OPT_ON);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
> > +			     cs_insn *insn, FILE *fp)
> > +{
> > +	struct addr_location al;
> > +	size_t printed = 0;
> > +
> > +	if (insn->detail && insn->detail->x86.op_count == 1) {
> > +		cs_x86_op *op = &insn->detail->x86.operands[0];
> > +
> > +		addr_location__init(&al);
> > +		if (op->type == X86_OP_IMM &&
> > +		    thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
> > +			printed += fprintf(fp, "%s ", insn[0].mnemonic);
> > +			printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
> > +			addr_location__exit(&al);
> > +			return printed;
> > +		}
> > +		addr_location__exit(&al);
> > +	}
> > +
> > +	printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> > +	return printed;
> > +}
> > +
> > +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> > +			    struct machine *machine, FILE *fp)
> > +{
> > +	csh cs_handle;
> > +	cs_insn *insn;
> > +	size_t count;
> > +	size_t printed = 0;
> > +	int ret;
> > +
> > +	/* TODO: Try to initiate capstone only once but need a proper place. */
> > +	ret = capstone_init(machine, &cs_handle);
> > +	if (ret < 0) {
> > +		/* fallback */
> > +		return sample__fprintf_insn_raw(sample, fp);
> > +	}
> > +
> > +	count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
> > +			  sample->ip, 1, &insn);
> > +	if (count > 0) {
> > +		if (machine__normalized_is(machine, "x86"))
> > +			printed += print_insn_x86(sample, thread, &insn[0], fp);
> > +		else
> > +			printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> > +		cs_free(insn, count);
> > +	} else {
> > +		printed += fprintf(fp, "illegal instruction");
> > +	}
> > +
> > +	cs_close(&cs_handle);
> > +	return printed;
> > +}
> > +#else
> > +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread __maybe_unused,
> > +			    struct machine *machine __maybe_unused, FILE *fp)
> > +{
> > +	return sample__fprintf_insn_raw(sample, fp);
> > +}
> > +#endif
> > diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> > new file mode 100644
> > index 000000000000..80dda6da7c60
> > --- /dev/null
> > +++ b/tools/perf/util/print_insn.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef PERF_PRINT_INSN_H
> > +#define PERF_PRINT_INSN_H
> > +
> > +#include <stddef.h>
> > +#include <stdio.h>
> > +
> > +struct perf_sample;
> > +struct thread;
> > +struct machine;
> > +
> > +size_t sample__fprintf_insn(struct perf_sample *sample, struct thread *thread,
> > +			    struct machine *machine, FILE *fp);
> > +size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> > +
> > +#endif /* PERF_PRINT_INSN_H */
> 

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions
  2024-02-05  9:23   ` Adrian Hunter
@ 2024-02-05 11:41     ` Changbin Du
  2024-02-05 12:19     ` Changbin Du
  1 sibling, 0 replies; 15+ messages in thread
From: Changbin Du @ 2024-02-05 11:41 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Changbin Du, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users, Andi Kleen, Thomas Richter, changbin.du

On Mon, Feb 05, 2024 at 11:23:21AM +0200, Adrian Hunter wrote:
> On 22/01/24 13:20, Changbin Du wrote:
> > In addition to the 'insn' field, this adds a new field 'disasm' to
> > display mnemonic instructions instead of the raw code.
> > 
> > $ sudo perf script -F +disasm
> >        perf-exec 1443864 [006] 2275506.209848:          psb:  psb offs: 0                                      0 [unknown] ([unknown])
> >        perf-exec 1443864 [006] 2275506.209848:          cbr:  cbr: 41 freq: 4100 MHz (114%)                    0 [unknown] ([unknown])
> >               ls 1443864 [006] 2275506.209905:          1  branches:uH:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	movq %rsp, %rdi
> >               ls 1443864 [006] 2275506.209908:          1  branches:uH:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	callq _dl_start+0x0
> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > 
> > ---
> > v2:
> >   - update Documentation.
> > ---
> >  tools/perf/Documentation/perf-script.txt | 13 +++++++------
> >  tools/perf/builtin-script.c              |  8 +++++++-
> >  2 files changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index ff9a52e44688..578fa59f51a5 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -132,9 +132,10 @@ OPTIONS
> >          Comma separated list of fields to print. Options are:
> >          comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
> >          srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
> > -        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
> > -        phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
> > -        machine_pid, vcpu, cgroup, retire_lat.
> > +        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
> > +        insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
> > +        code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
> > +
> >          Field list can be prepended with the type, trace, sw or hw,
> >          to indicate to which event type the field list applies.
> >          e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
> > @@ -217,9 +218,9 @@ OPTIONS
> >  	Instruction Trace decoding. For calls and returns, it will display the
> >  	name of the symbol indented with spaces to reflect the stack depth.
> >  
> > -	When doing instruction trace decoding insn and insnlen give the
> > -	instruction bytes and the instruction length of the current
> > -	instruction.
> > +	When doing instruction trace decoding, insn, disasm and insnlen give the
> > +	instruction bytes, disassembled instructions (requires libcapstone support)
> > +	and the instruction length of the current instruction respectively.
> >  
> >  	The synth field is used by synthesized events which may be created when
> >  	Instruction Trace decoding.
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 4817a37f16e2..4ac9670704ff 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -135,6 +135,7 @@ enum perf_output_field {
> >  	PERF_OUTPUT_CGROUP          = 1ULL << 39,
> >  	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
> >  	PERF_OUTPUT_DSOFF           = 1ULL << 41,
> > +	PERF_OUTPUT_DISASM          = 1ULL << 42,
> >  };
> >  
> >  struct perf_script {
> > @@ -190,6 +191,7 @@ struct output_option {
> >  	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
> >  	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
> >  	{.str = "insn", .field = PERF_OUTPUT_INSN},
> > +	{.str = "disasm", .field = PERF_OUTPUT_DISASM},
> >  	{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
> >  	{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
> >  	{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
> > @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> >  		printed += fprintf(fp, " insn: ");
> >  		printed += sample__fprintf_insn_raw(sample, fp);
> >  	}
> > +	if (PRINT_FIELD(DISASM) && sample->insn_len) {
> > +		printed += fprintf(fp, "\t\t");
> 
> This is good, except if both 'insn' and 'disasm' are used together.
> It either:
>  a) without libcapstone, prints insn bytes twice
>
> 	Probably simpler to make 'disasm' without libcapstone
> 	a fatal error explaining that perf needs to be built
> 	with libcapstone support for 'disasm' to work.
> 
>  b) with libcapstone, disassembly does not line up nicely
>
I prefer to #a which is simpler. I'll rename sample__fprintf_insn() to
sample__fprintf_insn_asm() wihtout fallback to raw version.

> > +		printed += sample__fprintf_insn(sample, thread, machine, fp);
> > +	}
> >  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> >  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> >  
> > @@ -3900,7 +3906,7 @@ int cmd_script(int argc, const char **argv)
> >  		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,dsoff,"
> >  		     "addr,symoff,srcline,period,iregs,uregs,brstack,"
> >  		     "brstacksym,flags,data_src,weight,bpf-output,brstackinsn,"
> > -		     "brstackinsnlen,brstackoff,callindent,insn,insnlen,synth,"
> > +		     "brstackinsnlen,brstackoff,callindent,insn,disasm,insnlen,synth,"
> >  		     "phys_addr,metric,misc,srccode,ipc,tod,data_page_size,"
> >  		     "code_page_size,ins_lat,machine_pid,vcpu,cgroup,retire_lat",
> >  		     parse_output_fields),
> 

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 1/5] perf: build: introduce the libcapstone
  2024-02-05  9:21   ` Adrian Hunter
@ 2024-02-05 11:51     ` Changbin Du
  0 siblings, 0 replies; 15+ messages in thread
From: Changbin Du @ 2024-02-05 11:51 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Changbin Du, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users, Andi Kleen, Thomas Richter, changbin.du

On Mon, Feb 05, 2024 at 11:21:23AM +0200, Adrian Hunter wrote:
> On 22/01/24 13:20, Changbin Du wrote:
> > Later we will use libcapstone to disassemble instructions of samples.
> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > 
> > ---
> > v2:
> >   - change tools/perf/tests/make also.
> > ---
> >  tools/build/Makefile.feature           |  2 ++
> >  tools/build/feature/Makefile           |  4 ++++
> >  tools/build/feature/test-all.c         |  4 ++++
> >  tools/build/feature/test-libcapstone.c | 11 +++++++++++
> >  tools/perf/Makefile.config             | 21 +++++++++++++++++++++
> >  tools/perf/Makefile.perf               |  3 +++
> >  tools/perf/tests/make                  |  2 ++
> >  7 files changed, 47 insertions(+)
> >  create mode 100644 tools/build/feature/test-libcapstone.c
> 
> Perhaps add it also to display with perf version --build-options
>
no problem.

> > 
> > diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> > index 64df118376df..1e2ab148d5db 100644
> > --- a/tools/build/Makefile.feature
> > +++ b/tools/build/Makefile.feature
> > @@ -87,6 +87,7 @@ FEATURE_TESTS_EXTRA :=                  \
> >           gtk2-infobar                   \
> >           hello                          \
> >           libbabeltrace                  \
> > +         libcapstone                    \
> >           libbfd-liberty                 \
> >           libbfd-liberty-z               \
> >           libopencsd                     \
> > @@ -134,6 +135,7 @@ FEATURE_DISPLAY ?=              \
> >           libcrypto              \
> >           libunwind              \
> >           libdw-dwarf-unwind     \
> > +         libcapstone            \
> >           zlib                   \
> >           lzma                   \
> >           get_cpuid              \
> > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> > index 37722e509eb9..ed54cef450f5 100644
> > --- a/tools/build/feature/Makefile
> > +++ b/tools/build/feature/Makefile
> > @@ -54,6 +54,7 @@ FILES=                                          \
> >           test-timerfd.bin                       \
> >           test-libdw-dwarf-unwind.bin            \
> >           test-libbabeltrace.bin                 \
> > +         test-libcapstone.bin			\
> >           test-compile-32.bin                    \
> >           test-compile-x32.bin                   \
> >           test-zlib.bin                          \
> > @@ -286,6 +287,9 @@ $(OUTPUT)test-libdw-dwarf-unwind.bin:
> >  $(OUTPUT)test-libbabeltrace.bin:
> >  	$(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
> >  
> > +$(OUTPUT)test-libcapstone.bin:
> > +	$(BUILD) # -lcapstone provided by $(FEATURE_CHECK_LDFLAGS-libcapstone)
> > +
> >  $(OUTPUT)test-compile-32.bin:
> >  	$(CC) -m32 -o $@ test-compile.c
> >  
> > diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
> > index 6f4bf386a3b5..dd0a18c2ef8f 100644
> > --- a/tools/build/feature/test-all.c
> > +++ b/tools/build/feature/test-all.c
> > @@ -134,6 +134,10 @@
> >  #undef main
> >  #endif
> >  
> > +#define main main_test_libcapstone
> > +# include "test-libcapstone.c"
> > +#undef main
> > +
> >  #define main main_test_lzma
> >  # include "test-lzma.c"
> >  #undef main
> > diff --git a/tools/build/feature/test-libcapstone.c b/tools/build/feature/test-libcapstone.c
> > new file mode 100644
> > index 000000000000..fbe8dba189e9
> > --- /dev/null
> > +++ b/tools/build/feature/test-libcapstone.c
> > @@ -0,0 +1,11 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <capstone/capstone.h>
> > +
> > +int main(void)
> > +{
> > +	csh handle;
> > +
> > +	cs_open(CS_ARCH_X86, CS_MODE_64, &handle);
> > +	return 0;
> > +}
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index aa55850fbc21..3e1072c59757 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -191,6 +191,15 @@ endif
> >  FEATURE_CHECK_CFLAGS-libbabeltrace := $(LIBBABELTRACE_CFLAGS)
> >  FEATURE_CHECK_LDFLAGS-libbabeltrace := $(LIBBABELTRACE_LDFLAGS) -lbabeltrace-ctf
> >  
> > +# for linking with debug library, run like:
> > +# make DEBUG=1 LIBCAPSTONE_DIR=/opt/capstone/
> > +ifdef LIBCAPSTONE_DIR
> > +  LIBCAPSTONE_CFLAGS  := -I$(LIBCAPSTONE_DIR)/include
> > +  LIBCAPSTONE_LDFLAGS := -L$(LIBCAPSTONE_DIR)/
> > +endif
> > +FEATURE_CHECK_CFLAGS-libcapstone := $(LIBCAPSTONE_CFLAGS)
> > +FEATURE_CHECK_LDFLAGS-libcapstone := $(LIBCAPSTONE_LDFLAGS) -lcapstone
> > +
> >  ifdef LIBZSTD_DIR
> >    LIBZSTD_CFLAGS  := -I$(LIBZSTD_DIR)/lib
> >    LIBZSTD_LDFLAGS := -L$(LIBZSTD_DIR)/lib
> > @@ -1094,6 +1103,18 @@ ifndef NO_LIBBABELTRACE
> >    endif
> >  endif
> >  
> > +ifndef NO_CAPSTONE
> > +  $(call feature_check,libcapstone)
> > +  ifeq ($(feature-libcapstone), 1)
> > +    CFLAGS += -DHAVE_LIBCAPSTONE_SUPPORT $(LIBCAPSTONE_CFLAGS)
> > +    LDFLAGS += $(LICAPSTONE_LDFLAGS)
> > +    EXTLIBS += -lcapstone
> > +    $(call detected,CONFIG_LIBCAPSTONE)
> > +  else
> > +    msg := $(warning No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel);
> > +  endif
> > +endif
> > +
> >  ifndef NO_AUXTRACE
> >    ifeq ($(SRCARCH),x86)
> >      ifeq ($(feature-get_cpuid), 0)
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 27e7c478880f..56c2720c1d0f 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -84,6 +84,9 @@ include ../scripts/utilities.mak
> >  # Define NO_LIBBABELTRACE if you do not want libbabeltrace support
> >  # for CTF data format.
> >  #
> > +# Define NO_CAPSTONE if you do not want libcapstone support
> > +# for disasm engine.
> > +#
> >  # Define NO_LZMA if you do not want to support compressed (xz) kernel modules
> >  #
> >  # Define NO_AUXTRACE if you do not want AUX area tracing support
> > diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> > index 8a4da7eb637a..b08026f5d4e7 100644
> > --- a/tools/perf/tests/make
> > +++ b/tools/perf/tests/make
> > @@ -83,6 +83,7 @@ make_no_libelf      := NO_LIBELF=1
> >  make_no_libunwind   := NO_LIBUNWIND=1
> >  make_no_libdw_dwarf_unwind := NO_LIBDW_DWARF_UNWIND=1
> >  make_no_backtrace   := NO_BACKTRACE=1
> > +make_no_libcapstone := NO_CAPSTONE=1
> >  make_no_libnuma     := NO_LIBNUMA=1
> >  make_no_libaudit    := NO_LIBAUDIT=1
> >  make_no_libbionic   := NO_LIBBIONIC=1
> 
> Also needs to be added to make_minimal
> 
okay. thanks!

> > @@ -152,6 +153,7 @@ run += make_no_libelf
> >  run += make_no_libunwind
> >  run += make_no_libdw_dwarf_unwind
> >  run += make_no_backtrace
> > +run += make_no_libcapstone
> >  run += make_no_libnuma
> >  run += make_no_libaudit
> >  run += make_no_libbionic
> 

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions
  2024-02-05  9:23   ` Adrian Hunter
  2024-02-05 11:41     ` Changbin Du
@ 2024-02-05 12:19     ` Changbin Du
  2024-02-05 13:10       ` Adrian Hunter
  1 sibling, 1 reply; 15+ messages in thread
From: Changbin Du @ 2024-02-05 12:19 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Changbin Du, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users, Andi Kleen, Thomas Richter, changbin.du

On Mon, Feb 05, 2024 at 11:23:21AM +0200, Adrian Hunter wrote:
> On 22/01/24 13:20, Changbin Du wrote:
> > In addition to the 'insn' field, this adds a new field 'disasm' to
> > display mnemonic instructions instead of the raw code.
> > 
> > $ sudo perf script -F +disasm
> >        perf-exec 1443864 [006] 2275506.209848:          psb:  psb offs: 0                                      0 [unknown] ([unknown])
> >        perf-exec 1443864 [006] 2275506.209848:          cbr:  cbr: 41 freq: 4100 MHz (114%)                    0 [unknown] ([unknown])
> >               ls 1443864 [006] 2275506.209905:          1  branches:uH:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	movq %rsp, %rdi
> >               ls 1443864 [006] 2275506.209908:          1  branches:uH:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	callq _dl_start+0x0
> > 
> > Signed-off-by: Changbin Du <changbin.du@huawei.com>
> > 
> > ---
> > v2:
> >   - update Documentation.
> > ---
> >  tools/perf/Documentation/perf-script.txt | 13 +++++++------
> >  tools/perf/builtin-script.c              |  8 +++++++-
> >  2 files changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> > index ff9a52e44688..578fa59f51a5 100644
> > --- a/tools/perf/Documentation/perf-script.txt
> > +++ b/tools/perf/Documentation/perf-script.txt
> > @@ -132,9 +132,10 @@ OPTIONS
> >          Comma separated list of fields to print. Options are:
> >          comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
> >          srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
> > -        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
> > -        phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
> > -        machine_pid, vcpu, cgroup, retire_lat.
> > +        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
> > +        insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
> > +        code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
> > +
> >          Field list can be prepended with the type, trace, sw or hw,
> >          to indicate to which event type the field list applies.
> >          e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
> > @@ -217,9 +218,9 @@ OPTIONS
> >  	Instruction Trace decoding. For calls and returns, it will display the
> >  	name of the symbol indented with spaces to reflect the stack depth.
> >  
> > -	When doing instruction trace decoding insn and insnlen give the
> > -	instruction bytes and the instruction length of the current
> > -	instruction.
> > +	When doing instruction trace decoding, insn, disasm and insnlen give the
> > +	instruction bytes, disassembled instructions (requires libcapstone support)
> > +	and the instruction length of the current instruction respectively.
> >  
> >  	The synth field is used by synthesized events which may be created when
> >  	Instruction Trace decoding.
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 4817a37f16e2..4ac9670704ff 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -135,6 +135,7 @@ enum perf_output_field {
> >  	PERF_OUTPUT_CGROUP          = 1ULL << 39,
> >  	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
> >  	PERF_OUTPUT_DSOFF           = 1ULL << 41,
> > +	PERF_OUTPUT_DISASM          = 1ULL << 42,
> >  };
> >  
> >  struct perf_script {
> > @@ -190,6 +191,7 @@ struct output_option {
> >  	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
> >  	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
> >  	{.str = "insn", .field = PERF_OUTPUT_INSN},
> > +	{.str = "disasm", .field = PERF_OUTPUT_DISASM},
> >  	{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
> >  	{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
> >  	{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
> > @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> >  		printed += fprintf(fp, " insn: ");
> >  		printed += sample__fprintf_insn_raw(sample, fp);
> >  	}
> > +	if (PRINT_FIELD(DISASM) && sample->insn_len) {
> > +		printed += fprintf(fp, "\t\t");
> 
> This is good, except if both 'insn' and 'disasm' are used together.
> It either:
>  a) without libcapstone, prints insn bytes twice
> 
> 	Probably simpler to make 'disasm' without libcapstone
> 	a fatal error explaining that perf needs to be built
> 	with libcapstone support for 'disasm' to work.
>
Instead of fatal error, I print a warning message for this. Because
perf_sample__fprintf_insn() cannot return negtive error number.

>  b) with libcapstone, disassembly does not line up nicely
> 
 

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions
  2024-02-05 12:19     ` Changbin Du
@ 2024-02-05 13:10       ` Adrian Hunter
  2024-02-06  2:00         ` Changbin Du
  0 siblings, 1 reply; 15+ messages in thread
From: Adrian Hunter @ 2024-02-05 13:10 UTC (permalink / raw)
  To: Changbin Du
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Ian Rogers, linux-kernel, linux-perf-users, Andi Kleen,
	Thomas Richter, changbin.du

On 5/02/24 14:19, Changbin Du wrote:
> On Mon, Feb 05, 2024 at 11:23:21AM +0200, Adrian Hunter wrote:
>> On 22/01/24 13:20, Changbin Du wrote:
>>> In addition to the 'insn' field, this adds a new field 'disasm' to
>>> display mnemonic instructions instead of the raw code.
>>>
>>> $ sudo perf script -F +disasm
>>>        perf-exec 1443864 [006] 2275506.209848:          psb:  psb offs: 0                                      0 [unknown] ([unknown])
>>>        perf-exec 1443864 [006] 2275506.209848:          cbr:  cbr: 41 freq: 4100 MHz (114%)                    0 [unknown] ([unknown])
>>>               ls 1443864 [006] 2275506.209905:          1  branches:uH:      7f216b426100 _start+0x0 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	movq %rsp, %rdi
>>>               ls 1443864 [006] 2275506.209908:          1  branches:uH:      7f216b426103 _start+0x3 (/usr/lib/x86_64-linux-gnu/ld-2.31.so)	callq _dl_start+0x0
>>>
>>> Signed-off-by: Changbin Du <changbin.du@huawei.com>
>>>
>>> ---
>>> v2:
>>>   - update Documentation.
>>> ---
>>>  tools/perf/Documentation/perf-script.txt | 13 +++++++------
>>>  tools/perf/builtin-script.c              |  8 +++++++-
>>>  2 files changed, 14 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
>>> index ff9a52e44688..578fa59f51a5 100644
>>> --- a/tools/perf/Documentation/perf-script.txt
>>> +++ b/tools/perf/Documentation/perf-script.txt
>>> @@ -132,9 +132,10 @@ OPTIONS
>>>          Comma separated list of fields to print. Options are:
>>>          comm, tid, pid, time, cpu, event, trace, ip, sym, dso, dsoff, addr, symoff,
>>>          srcline, period, iregs, uregs, brstack, brstacksym, flags, bpf-output,
>>> -        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, insnlen, synth,
>>> -        phys_addr, metric, misc, srccode, ipc, data_page_size, code_page_size, ins_lat,
>>> -        machine_pid, vcpu, cgroup, retire_lat.
>>> +        brstackinsn, brstackinsnlen, brstackoff, callindent, insn, disasm,
>>> +        insnlen, synth, phys_addr, metric, misc, srccode, ipc, data_page_size,
>>> +        code_page_size, ins_lat, machine_pid, vcpu, cgroup, retire_lat.
>>> +
>>>          Field list can be prepended with the type, trace, sw or hw,
>>>          to indicate to which event type the field list applies.
>>>          e.g., -F sw:comm,tid,time,ip,sym  and -F trace:time,cpu,trace
>>> @@ -217,9 +218,9 @@ OPTIONS
>>>  	Instruction Trace decoding. For calls and returns, it will display the
>>>  	name of the symbol indented with spaces to reflect the stack depth.
>>>  
>>> -	When doing instruction trace decoding insn and insnlen give the
>>> -	instruction bytes and the instruction length of the current
>>> -	instruction.
>>> +	When doing instruction trace decoding, insn, disasm and insnlen give the
>>> +	instruction bytes, disassembled instructions (requires libcapstone support)
>>> +	and the instruction length of the current instruction respectively.
>>>  
>>>  	The synth field is used by synthesized events which may be created when
>>>  	Instruction Trace decoding.
>>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>>> index 4817a37f16e2..4ac9670704ff 100644
>>> --- a/tools/perf/builtin-script.c
>>> +++ b/tools/perf/builtin-script.c
>>> @@ -135,6 +135,7 @@ enum perf_output_field {
>>>  	PERF_OUTPUT_CGROUP          = 1ULL << 39,
>>>  	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
>>>  	PERF_OUTPUT_DSOFF           = 1ULL << 41,
>>> +	PERF_OUTPUT_DISASM          = 1ULL << 42,
>>>  };
>>>  
>>>  struct perf_script {
>>> @@ -190,6 +191,7 @@ struct output_option {
>>>  	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
>>>  	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
>>>  	{.str = "insn", .field = PERF_OUTPUT_INSN},
>>> +	{.str = "disasm", .field = PERF_OUTPUT_DISASM},
>>>  	{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
>>>  	{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
>>>  	{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
>>> @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>>>  		printed += fprintf(fp, " insn: ");
>>>  		printed += sample__fprintf_insn_raw(sample, fp);
>>>  	}
>>> +	if (PRINT_FIELD(DISASM) && sample->insn_len) {
>>> +		printed += fprintf(fp, "\t\t");
>>
>> This is good, except if both 'insn' and 'disasm' are used together.
>> It either:
>>  a) without libcapstone, prints insn bytes twice
>>
>> 	Probably simpler to make 'disasm' without libcapstone
>> 	a fatal error explaining that perf needs to be built
>> 	with libcapstone support for 'disasm' to work.
>>
> Instead of fatal error, I print a warning message for this. Because
> perf_sample__fprintf_insn() cannot return negtive error number.

It could be validated in advance, perhaps in parse_output_fields().

> 
>>  b) with libcapstone, disassembly does not line up nicely
>>
>  
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions
  2024-02-05 13:10       ` Adrian Hunter
@ 2024-02-06  2:00         ` Changbin Du
  0 siblings, 0 replies; 15+ messages in thread
From: Changbin Du @ 2024-02-06  2:00 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Changbin Du, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users, Andi Kleen, Thomas Richter, changbin.du

On Mon, Feb 05, 2024 at 03:10:58PM +0200, Adrian Hunter wrote:
> On 5/02/24 14:19, Changbin Du wrote:
> > On Mon, Feb 05, 2024 at 11:23:21AM +0200, Adrian Hunter wrote:
> >>>  struct perf_script {
> >>> @@ -190,6 +191,7 @@ struct output_option {
> >>>  	{.str = "bpf-output",   .field = PERF_OUTPUT_BPF_OUTPUT},
> >>>  	{.str = "callindent", .field = PERF_OUTPUT_CALLINDENT},
> >>>  	{.str = "insn", .field = PERF_OUTPUT_INSN},
> >>> +	{.str = "disasm", .field = PERF_OUTPUT_DISASM},
> >>>  	{.str = "insnlen", .field = PERF_OUTPUT_INSNLEN},
> >>>  	{.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN},
> >>>  	{.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF},
> >>> @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
> >>>  		printed += fprintf(fp, " insn: ");
> >>>  		printed += sample__fprintf_insn_raw(sample, fp);
> >>>  	}
> >>> +	if (PRINT_FIELD(DISASM) && sample->insn_len) {
> >>> +		printed += fprintf(fp, "\t\t");
> >>
> >> This is good, except if both 'insn' and 'disasm' are used together.
> >> It either:
> >>  a) without libcapstone, prints insn bytes twice
> >>
> >> 	Probably simpler to make 'disasm' without libcapstone
> >> 	a fatal error explaining that perf needs to be built
> >> 	with libcapstone support for 'disasm' to work.
> >>
> > Instead of fatal error, I print a warning message for this. Because
> > perf_sample__fprintf_insn() cannot return negtive error number.
> 
> It could be validated in advance, perhaps in parse_output_fields().
>
I did as below:

--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3112,6 +3112,13 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
                        rc = -EINVAL;
                        goto out;
                }
+#ifndef HAVE_LIBCAPSTONE_SUPPORT
+               if (change != REMOVE && strcmp(tok, "disasm") == 0) {
+                       fprintf(stderr, "Field \"disasm\" requires perf to be built with libcapstone support.\n");
+                       rc = -EINVAL;
+                       goto out;
+               }
+#endif

Then,
$ sudo perf script -F +disasm
Field "disasm" requires perf to be built with libcapstone support.

 Usage: perf script [<options>]
 ...

> > 
> >>  b) with libcapstone, disassembly does not line up nicely
> >>
> >  
> > 
> 

-- 
Cheers,
Changbin Du

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-02-06  2:01 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 11:20 [PATCH v5 0/5] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
2024-01-22 11:20 ` [PATCH v5 1/5] perf: build: introduce the libcapstone Changbin Du
2024-02-05  9:21   ` Adrian Hunter
2024-02-05 11:51     ` Changbin Du
2024-01-22 11:20 ` [PATCH v5 2/5] perf: util: use capstone disasm engine to show assembly instructions Changbin Du
2024-02-05  9:22   ` Adrian Hunter
2024-02-05 11:36     ` Changbin Du
2024-01-22 11:20 ` [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions Changbin Du
2024-02-05  9:23   ` Adrian Hunter
2024-02-05 11:41     ` Changbin Du
2024-02-05 12:19     ` Changbin Du
2024-02-05 13:10       ` Adrian Hunter
2024-02-06  2:00         ` Changbin Du
2024-01-22 11:20 ` [PATCH v5 4/5] perf: script: add raw|disasm arguments to --insn-trace option Changbin Du
2024-01-22 11:20 ` [PATCH v5 5/5] perf: script: prefer capstone to XED Changbin Du

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.