[PATCH v4 1/2] perf, capstone: Support 32bit code under 64bit OS

[PATCH v4 1/2] perf, capstone: Support 32bit code under 64bit OS

[Andi Kleen]

Use the DSO to resolve whether an IP is 32bit or 64bit and use that to
configure capstone to the correct mode. This allows to correctly
disassemble 32bit code under a 64bit OS.

% cat > loop.c
volatile int var;
int main(void)
{
	int i;
	for (i = 0; i < 100000; i++)
		var++;
}
% gcc -m32 -o loop loop.c
% perf record -e cycles:u ./loop
% perf script -F +disasm
            loop   82665 1833176.618023:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
            loop   82665 1833176.618029:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
            loop   82665 1833176.618031:          7 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
            loop   82665 1833176.618034:         91 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
            loop   82665 1833176.618036:       1242 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---

v2: Factor out DSO lookup into separate function
v3: Pass down al
v4: Simplify is64bitip
---
 tools/perf/builtin-script.c  |  9 +++++----
 tools/perf/util/print_insn.c | 24 ++++++++++++++++++++----
 tools/perf/util/print_insn.h |  2 +-
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 37088cc0ff1b..0299b1ed8744 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1517,7 +1517,8 @@ void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
 static int perf_sample__fprintf_insn(struct perf_sample *sample,
 				     struct perf_event_attr *attr,
 				     struct thread *thread,
-				     struct machine *machine, FILE *fp)
+				     struct machine *machine, FILE *fp,
+				     struct addr_location *al)
 {
 	int printed = 0;
 
@@ -1531,7 +1532,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
 	}
 	if (PRINT_FIELD(DISASM) && sample->insn_len) {
 		printed += fprintf(fp, "\t\t");
-		printed += sample__fprintf_insn_asm(sample, thread, machine, fp);
+		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
 	}
 	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
 		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
@@ -1606,7 +1607,7 @@ static int perf_sample__fprintf_bts(struct perf_sample *sample,
 	if (print_srcline_last)
 		printed += map__fprintf_srcline(al->map, al->addr, "\n  ", fp);
 
-	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
+	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
 	printed += fprintf(fp, "\n");
 	if (PRINT_FIELD(SRCCODE)) {
 		int ret = map__fprintf_srccode(al->map, al->addr, stdout,
@@ -2259,7 +2260,7 @@ static void process_event(struct perf_script *script,
 
 	if (evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
 		perf_sample__fprintf_bpf_output(sample, fp);
-	perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
+	perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
 
 	if (PRINT_FIELD(PHYS_ADDR))
 		fprintf(fp, "%16" PRIx64, sample->phys_addr);
diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
index 459e0e93d7b1..8e4e3cffd677 100644
--- a/tools/perf/util/print_insn.c
+++ b/tools/perf/util/print_insn.c
@@ -12,6 +12,8 @@
 #include "machine.h"
 #include "thread.h"
 #include "print_insn.h"
+#include "map.h"
+#include "dso.h"
 
 size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
 {
@@ -28,12 +30,12 @@ size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
 #ifdef HAVE_LIBCAPSTONE_SUPPORT
 #include <capstone/capstone.h>
 
-static int capstone_init(struct machine *machine, csh *cs_handle)
+static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
 {
 	cs_arch arch;
 	cs_mode mode;
 
-	if (machine__is(machine, "x86_64")) {
+	if (machine__is(machine, "x86_64") && is64) {
 		arch = CS_ARCH_X86;
 		mode = CS_MODE_64;
 	} else if (machine__normalized_is(machine, "x86")) {
@@ -93,17 +95,31 @@ static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
 	return printed;
 }
 
+static bool is64bitip(struct machine *machine, struct addr_location *al)
+{
+	const struct dso *dso = al->map ? map__dso(al->map) : NULL;
+
+	if (dso)
+		return dso->is_64_bit;
+
+	return machine__is(machine, "x86_64") ||
+		machine__normalized_is(machine, "arm64") ||
+		machine__normalized_is(machine, "s390");
+}
+
 size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
-				struct machine *machine, FILE *fp)
+				struct machine *machine, FILE *fp,
+				struct addr_location *al)
 {
 	csh cs_handle;
 	cs_insn *insn;
 	size_t count;
 	size_t printed = 0;
 	int ret;
+	bool is64bit = is64bitip(machine, al);
 
 	/* TODO: Try to initiate capstone only once but need a proper place. */
-	ret = capstone_init(machine, &cs_handle);
+	ret = capstone_init(machine, &cs_handle, is64bit);
 	if (ret < 0) {
 		/* fallback */
 		return sample__fprintf_insn_raw(sample, fp);
diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
index 465bdcfcc2fd..6447dd41b543 100644
--- a/tools/perf/util/print_insn.h
+++ b/tools/perf/util/print_insn.h
@@ -10,7 +10,7 @@ struct thread;
 struct machine;
 
 size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
-				struct machine *machine, FILE *fp);
+				struct machine *machine, FILE *fp, struct addr_location *al);
 size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
 
 #endif /* PERF_PRINT_INSN_H */
-- 
2.43.0

[PATCH v4 2/2] perf, script, capstone: Add support for -F +brstackdisasm

[Andi Kleen]

Support capstone output for the -F +brstackinsn branch dump.
The new output is enabled with the new field brstackdisasm
This was possible before with --xed, but now also allow
it for users that don't have xed using the builtin capstone support.

Before:

perf record -b emacs -Q --batch '()'
perf script -F +brstackinsn
...
          emacs   55778 1814366.755945:     151564 cycles:P:      7f0ab2d17192 intel_check_word.constprop.0+0x162 (/usr/lib64/ld-linux-x86-64.s>        intel_check_word.constprop.0+237:
        00007f0ab2d1711d        insn: 75 e6                     # PRED 3 cycles [3]
        00007f0ab2d17105        insn: 73 51
        00007f0ab2d17107        insn: 48 89 c1
        00007f0ab2d1710a        insn: 48 39 ca
        00007f0ab2d1710d        insn: 73 96
        00007f0ab2d1710f        insn: 48 8d 04 11
        00007f0ab2d17113        insn: 48 d1 e8
        00007f0ab2d17116        insn: 49 8d 34 c1
        00007f0ab2d1711a        insn: 44 3a 06
        00007f0ab2d1711d        insn: 75 e6                     # PRED 3 cycles [6] 3.00 IPC
        00007f0ab2d17105        insn: 73 51                     # PRED 1 cycles [7] 1.00 IPC
        00007f0ab2d17158        insn: 48 8d 50 01
        00007f0ab2d1715c        insn: eb 92                     # PRED 1 cycles [8] 2.00 IPC
        00007f0ab2d170f0        insn: 48 39 ca
        00007f0ab2d170f3        insn: 73 b0                     # PRED 1 cycles [9] 2.00 IPC

After (perf must be compiled with capstone):

perf script -F +brstackdisasm

...
           emacs   55778 1814366.755945:     151564 cycles:P:      7f0ab2d17192 intel_check_word.constprop.0+0x162 (/usr/lib64/ld-linux-x86-64.s>        intel_check_word.constprop.0+237:
        00007f0ab2d1711d        jne intel_check_word.constprop.0+0xd5   # PRED 3 cycles [3]
        00007f0ab2d17105        jae intel_check_word.constprop.0+0x128
        00007f0ab2d17107        movq %rax, %rcx
        00007f0ab2d1710a        cmpq %rcx, %rdx
        00007f0ab2d1710d        jae intel_check_word.constprop.0+0x75
        00007f0ab2d1710f        leaq (%rcx, %rdx), %rax
        00007f0ab2d17113        shrq $1, %rax
        00007f0ab2d17116        leaq (%r9, %rax, 8), %rsi
        00007f0ab2d1711a        cmpb (%rsi), %r8b
        00007f0ab2d1711d        jne intel_check_word.constprop.0+0xd5   # PRED 3 cycles [6] 3.00 IPC
        00007f0ab2d17105        jae intel_check_word.constprop.0+0x128  # PRED 1 cycles [7] 1.00 IPC
        00007f0ab2d17158        leaq 1(%rax), %rdx
        00007f0ab2d1715c        jmp intel_check_word.constprop.0+0xc0   # PRED 1 cycles [8] 2.00 IPC
        00007f0ab2d170f0        cmpq %rcx, %rdx
        00007f0ab2d170f3        jae intel_check_word.constprop.0+0x75   # PRED 1 cycles [9] 2.00 IPC

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---

v2: Use brstackdisasm instead of keying of disasm
---
 tools/perf/Documentation/perf-script.txt |  7 +++-
 tools/perf/builtin-script.c              | 32 +++++++++++----
 tools/perf/util/dump-insn.h              |  1 +
 tools/perf/util/print_insn.c             | 52 ++++++++++++++++++++++++
 tools/perf/util/print_insn.h             |  3 ++
 5 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 005e51df855e..ff086ef05a0c 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -132,9 +132,9 @@ 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, disasm,
+        brstackinsn, brstackinsnlen, brstackdisasm, 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.
+        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.
@@ -257,6 +257,9 @@ OPTIONS
 	can’t know the next sequential instruction after an unconditional branch unless
 	you calculate that based on its length.
 
+	brstackdisasm acts like brstackinsn, but will print disassembled instructions if
+	perf is built with the capstone library.
+
 	The brstackoff field will print an offset into a specific dso/binary.
 
 	With the metric option perf script can compute metrics for
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 0299b1ed8744..db18d2c54c59 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -136,6 +136,7 @@ enum perf_output_field {
 	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
 	PERF_OUTPUT_DSOFF           = 1ULL << 41,
 	PERF_OUTPUT_DISASM          = 1ULL << 42,
+	PERF_OUTPUT_BRSTACKDISASM   = 1ULL << 43,
 };
 
 struct perf_script {
@@ -210,6 +211,7 @@ struct output_option {
 	{.str = "vcpu", .field = PERF_OUTPUT_VCPU},
 	{.str = "cgroup", .field = PERF_OUTPUT_CGROUP},
 	{.str = "retire_lat", .field = PERF_OUTPUT_RETIRE_LAT},
+	{.str = "brstackdisasm", .field = PERF_OUTPUT_BRSTACKDISASM},
 };
 
 enum {
@@ -510,7 +512,8 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
 		       "selected. Hence, no address to lookup the source line number.\n");
 		return -EINVAL;
 	}
-	if ((PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN)) && !allow_user_set &&
+	if ((PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN) || PRINT_FIELD(BRSTACKDISASM))
+	    && !allow_user_set &&
 	    !(evlist__combined_branch_type(session->evlist) & PERF_SAMPLE_BRANCH_ANY)) {
 		pr_err("Display of branch stack assembler requested, but non all-branch filter set\n"
 		       "Hint: run 'perf record -b ...'\n");
@@ -1162,6 +1165,20 @@ static int print_srccode(struct thread *thread, u8 cpumode, uint64_t addr)
 	return ret;
 }
 
+static const char *any_dump_insn(struct perf_event_attr *attr,
+			struct perf_insn *x, uint64_t ip,
+			u8 *inbuf, int inlen, int *lenp)
+{
+#ifdef HAVE_LIBCAPSTONE_SUPPORT
+	if (PRINT_FIELD(BRSTACKDISASM)) {
+		const char *p = cs_dump_insn(x, ip, inbuf, inlen, lenp);
+		if (p)
+			return p;
+	}
+#endif
+	return dump_insn(x, ip, inbuf, inlen, lenp);
+}
+
 static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
 			    struct perf_insn *x, u8 *inbuf, int len,
 			    int insn, FILE *fp, int *total_cycles,
@@ -1170,7 +1187,7 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
 {
 	int ilen = 0;
 	int printed = fprintf(fp, "\t%016" PRIx64 "\t%-30s\t", ip,
-			      dump_insn(x, ip, inbuf, len, &ilen));
+			      any_dump_insn(attr, x, ip, inbuf, len, &ilen));
 
 	if (PRINT_FIELD(BRSTACKINSNLEN))
 		printed += fprintf(fp, "ilen: %d\t", ilen);
@@ -1262,6 +1279,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 		nr = max_blocks + 1;
 
 	x.thread = thread;
+	x.machine = machine;
 	x.cpu = sample->cpu;
 
 	printed += fprintf(fp, "%c", '\n');
@@ -1313,7 +1331,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 			} else {
 				ilen = 0;
 				printed += fprintf(fp, "\t%016" PRIx64 "\t%s", ip,
-						   dump_insn(&x, ip, buffer + off, len - off, &ilen));
+						   any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen));
 				if (PRINT_FIELD(BRSTACKINSNLEN))
 					printed += fprintf(fp, "\tilen: %d", ilen);
 				printed += fprintf(fp, "\n");
@@ -1361,7 +1379,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 			goto out;
 		ilen = 0;
 		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", sample->ip,
-			dump_insn(&x, sample->ip, buffer, len, &ilen));
+			any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen));
 		if (PRINT_FIELD(BRSTACKINSNLEN))
 			printed += fprintf(fp, "\tilen: %d", ilen);
 		printed += fprintf(fp, "\n");
@@ -1372,7 +1390,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 	for (off = 0; off <= end - start; off += ilen) {
 		ilen = 0;
 		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", start + off,
-				   dump_insn(&x, start + off, buffer + off, len - off, &ilen));
+				   any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen));
 		if (PRINT_FIELD(BRSTACKINSNLEN))
 			printed += fprintf(fp, "\tilen: %d", ilen);
 		printed += fprintf(fp, "\n");
@@ -1534,7 +1552,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
 		printed += fprintf(fp, "\t\t");
 		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
 	}
-	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
+	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN) || PRINT_FIELD(BRSTACKDISASM))
 		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
 
 	return printed;
@@ -3940,7 +3958,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,disasm,insnlen,synth,"
+		     "brstackinsnlen,brstackdisasm,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),
diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h
index 650125061530..4a7797dd6d09 100644
--- a/tools/perf/util/dump-insn.h
+++ b/tools/perf/util/dump-insn.h
@@ -11,6 +11,7 @@ struct thread;
 struct perf_insn {
 	/* Initialized by callers: */
 	struct thread *thread;
+	struct machine *machine;
 	u8	      cpumode;
 	bool	      is64bit;
 	int	      cpu;
diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
index 8e4e3cffd677..bca4449f0fa8 100644
--- a/tools/perf/util/print_insn.c
+++ b/tools/perf/util/print_insn.c
@@ -12,6 +12,7 @@
 #include "machine.h"
 #include "thread.h"
 #include "print_insn.h"
+#include "dump-insn.h"
 #include "map.h"
 #include "dso.h"
 
@@ -71,6 +72,57 @@ static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
 	return 0;
 }
 
+static void dump_insn_x86(struct thread *thread, cs_insn *insn, struct perf_insn *x)
+{
+	struct addr_location al;
+	bool printed = false;
+
+	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, x->cpumode, op->imm, &al) &&
+		    al.sym &&
+		    al.addr < al.sym->end) {
+			snprintf(x->out, sizeof(x->out), "%s %s+%#" PRIx64 " [%#" PRIx64 "]", insn[0].mnemonic,
+					al.sym->name, al.addr - al.sym->start, op->imm);
+			printed = true;
+		}
+		addr_location__exit(&al);
+	}
+
+	if (!printed)
+		snprintf(x->out, sizeof(x->out), "%s %s", insn[0].mnemonic, insn[0].op_str);
+}
+
+const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
+			 u8 *inbuf, int inlen, int *lenp)
+{
+	int ret;
+	int count;
+	cs_insn *insn;
+	csh cs_handle;
+
+	ret = capstone_init(x->machine, &cs_handle, x->is64bit);
+	if (ret < 0)
+		return NULL;
+
+	count = cs_disasm(cs_handle, (uint8_t *)inbuf, inlen, ip, 1, &insn);
+	if (count > 0) {
+		if (machine__normalized_is(x->machine, "x86"))
+			dump_insn_x86(x->thread, &insn[0], x);
+		else
+			snprintf(x->out, sizeof(x->out), "%s %s",
+					insn[0].mnemonic, insn[0].op_str);
+		*lenp = insn->size;
+		cs_free(insn, count);
+	} else {
+		return NULL;
+	}
+	return x->out;
+}
+
 static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
 			     cs_insn *insn, FILE *fp)
 {
diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
index 6447dd41b543..c2a6391a45ce 100644
--- a/tools/perf/util/print_insn.h
+++ b/tools/perf/util/print_insn.h
@@ -8,9 +8,12 @@
 struct perf_sample;
 struct thread;
 struct machine;
+struct perf_insn;
 
 size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
 				struct machine *machine, FILE *fp, struct addr_location *al);
 size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
+const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
+                         u8 *inbuf, int inlen, int *lenp);
 
 #endif /* PERF_PRINT_INSN_H */
-- 
2.43.0

Re: [PATCH v4 2/2] perf, script, capstone: Add support for -F +brstackdisasm

[Adrian Hunter]

On 8/03/24 19:06, Andi Kleen wrote:
> Support capstone output for the -F +brstackinsn branch dump.
> The new output is enabled with the new field brstackdisasm
> This was possible before with --xed, but now also allow
> it for users that don't have xed using the builtin capstone support.
> 
> Before:
> 
> perf record -b emacs -Q --batch '()'
> perf script -F +brstackinsn
> ...
>           emacs   55778 1814366.755945:     151564 cycles:P:      7f0ab2d17192 intel_check_word.constprop.0+0x162 (/usr/lib64/ld-linux-x86-64.s>        intel_check_word.constprop.0+237:
>         00007f0ab2d1711d        insn: 75 e6                     # PRED 3 cycles [3]
>         00007f0ab2d17105        insn: 73 51
>         00007f0ab2d17107        insn: 48 89 c1
>         00007f0ab2d1710a        insn: 48 39 ca
>         00007f0ab2d1710d        insn: 73 96
>         00007f0ab2d1710f        insn: 48 8d 04 11
>         00007f0ab2d17113        insn: 48 d1 e8
>         00007f0ab2d17116        insn: 49 8d 34 c1
>         00007f0ab2d1711a        insn: 44 3a 06
>         00007f0ab2d1711d        insn: 75 e6                     # PRED 3 cycles [6] 3.00 IPC
>         00007f0ab2d17105        insn: 73 51                     # PRED 1 cycles [7] 1.00 IPC
>         00007f0ab2d17158        insn: 48 8d 50 01
>         00007f0ab2d1715c        insn: eb 92                     # PRED 1 cycles [8] 2.00 IPC
>         00007f0ab2d170f0        insn: 48 39 ca
>         00007f0ab2d170f3        insn: 73 b0                     # PRED 1 cycles [9] 2.00 IPC
> 
> After (perf must be compiled with capstone):
> 
> perf script -F +brstackdisasm
> 
> ...
>            emacs   55778 1814366.755945:     151564 cycles:P:      7f0ab2d17192 intel_check_word.constprop.0+0x162 (/usr/lib64/ld-linux-x86-64.s>        intel_check_word.constprop.0+237:
>         00007f0ab2d1711d        jne intel_check_word.constprop.0+0xd5   # PRED 3 cycles [3]
>         00007f0ab2d17105        jae intel_check_word.constprop.0+0x128
>         00007f0ab2d17107        movq %rax, %rcx
>         00007f0ab2d1710a        cmpq %rcx, %rdx
>         00007f0ab2d1710d        jae intel_check_word.constprop.0+0x75
>         00007f0ab2d1710f        leaq (%rcx, %rdx), %rax
>         00007f0ab2d17113        shrq $1, %rax
>         00007f0ab2d17116        leaq (%r9, %rax, 8), %rsi
>         00007f0ab2d1711a        cmpb (%rsi), %r8b
>         00007f0ab2d1711d        jne intel_check_word.constprop.0+0xd5   # PRED 3 cycles [6] 3.00 IPC
>         00007f0ab2d17105        jae intel_check_word.constprop.0+0x128  # PRED 1 cycles [7] 1.00 IPC
>         00007f0ab2d17158        leaq 1(%rax), %rdx
>         00007f0ab2d1715c        jmp intel_check_word.constprop.0+0xc0   # PRED 1 cycles [8] 2.00 IPC
>         00007f0ab2d170f0        cmpq %rcx, %rdx
>         00007f0ab2d170f3        jae intel_check_word.constprop.0+0x75   # PRED 1 cycles [9] 2.00 IPC
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
> 
> v2: Use brstackdisasm instead of keying of disasm
> ---
>  tools/perf/Documentation/perf-script.txt |  7 +++-
>  tools/perf/builtin-script.c              | 32 +++++++++++----
>  tools/perf/util/dump-insn.h              |  1 +
>  tools/perf/util/print_insn.c             | 52 ++++++++++++++++++++++++
>  tools/perf/util/print_insn.h             |  3 ++
>  5 files changed, 86 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index 005e51df855e..ff086ef05a0c 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -132,9 +132,9 @@ 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, disasm,
> +        brstackinsn, brstackinsnlen, brstackdisasm, 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.
> +        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.
> @@ -257,6 +257,9 @@ OPTIONS
>  	can’t know the next sequential instruction after an unconditional branch unless
>  	you calculate that based on its length.
>  
> +	brstackdisasm acts like brstackinsn, but will print disassembled instructions if
> +	perf is built with the capstone library.
> +
>  	The brstackoff field will print an offset into a specific dso/binary.
>  
>  	With the metric option perf script can compute metrics for
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 0299b1ed8744..db18d2c54c59 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -136,6 +136,7 @@ enum perf_output_field {
>  	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
>  	PERF_OUTPUT_DSOFF           = 1ULL << 41,
>  	PERF_OUTPUT_DISASM          = 1ULL << 42,
> +	PERF_OUTPUT_BRSTACKDISASM   = 1ULL << 43,
>  };
>  
>  struct perf_script {
> @@ -210,6 +211,7 @@ struct output_option {
>  	{.str = "vcpu", .field = PERF_OUTPUT_VCPU},
>  	{.str = "cgroup", .field = PERF_OUTPUT_CGROUP},
>  	{.str = "retire_lat", .field = PERF_OUTPUT_RETIRE_LAT},
> +	{.str = "brstackdisasm", .field = PERF_OUTPUT_BRSTACKDISASM},
>  };
>  
>  enum {
> @@ -510,7 +512,8 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
>  		       "selected. Hence, no address to lookup the source line number.\n");
>  		return -EINVAL;
>  	}
> -	if ((PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN)) && !allow_user_set &&
> +	if ((PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN) || PRINT_FIELD(BRSTACKDISASM))
> +	    && !allow_user_set &&
>  	    !(evlist__combined_branch_type(session->evlist) & PERF_SAMPLE_BRANCH_ANY)) {
>  		pr_err("Display of branch stack assembler requested, but non all-branch filter set\n"
>  		       "Hint: run 'perf record -b ...'\n");
> @@ -1162,6 +1165,20 @@ static int print_srccode(struct thread *thread, u8 cpumode, uint64_t addr)
>  	return ret;
>  }
>  
> +static const char *any_dump_insn(struct perf_event_attr *attr,

attr is __maybe_unused

> +			struct perf_insn *x, uint64_t ip,
> +			u8 *inbuf, int inlen, int *lenp)
> +{
> +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> +	if (PRINT_FIELD(BRSTACKDISASM)) {
> +		const char *p = cs_dump_insn(x, ip, inbuf, inlen, lenp);
> +		if (p)
> +			return p;
> +	}
> +#endif
> +	return dump_insn(x, ip, inbuf, inlen, lenp);
> +}
> +
>  static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
>  			    struct perf_insn *x, u8 *inbuf, int len,
>  			    int insn, FILE *fp, int *total_cycles,
> @@ -1170,7 +1187,7 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
>  {
>  	int ilen = 0;
>  	int printed = fprintf(fp, "\t%016" PRIx64 "\t%-30s\t", ip,
> -			      dump_insn(x, ip, inbuf, len, &ilen));
> +			      any_dump_insn(attr, x, ip, inbuf, len, &ilen));
>  
>  	if (PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += fprintf(fp, "ilen: %d\t", ilen);
> @@ -1262,6 +1279,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  		nr = max_blocks + 1;
>  
>  	x.thread = thread;
> +	x.machine = machine;
>  	x.cpu = sample->cpu;
>  
>  	printed += fprintf(fp, "%c", '\n');
> @@ -1313,7 +1331,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  			} else {
>  				ilen = 0;
>  				printed += fprintf(fp, "\t%016" PRIx64 "\t%s", ip,
> -						   dump_insn(&x, ip, buffer + off, len - off, &ilen));
> +						   any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen));
>  				if (PRINT_FIELD(BRSTACKINSNLEN))
>  					printed += fprintf(fp, "\tilen: %d", ilen);
>  				printed += fprintf(fp, "\n");
> @@ -1361,7 +1379,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  			goto out;
>  		ilen = 0;
>  		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", sample->ip,
> -			dump_insn(&x, sample->ip, buffer, len, &ilen));
> +			any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen));
>  		if (PRINT_FIELD(BRSTACKINSNLEN))
>  			printed += fprintf(fp, "\tilen: %d", ilen);
>  		printed += fprintf(fp, "\n");
> @@ -1372,7 +1390,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  	for (off = 0; off <= end - start; off += ilen) {
>  		ilen = 0;
>  		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", start + off,
> -				   dump_insn(&x, start + off, buffer + off, len - off, &ilen));
> +				   any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen));
>  		if (PRINT_FIELD(BRSTACKINSNLEN))
>  			printed += fprintf(fp, "\tilen: %d", ilen);
>  		printed += fprintf(fp, "\n");
> @@ -1534,7 +1552,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  		printed += fprintf(fp, "\t\t");
>  		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
>  	}
> -	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> +	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN) || PRINT_FIELD(BRSTACKDISASM))
>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>  
>  	return printed;
> @@ -3940,7 +3958,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,disasm,insnlen,synth,"
> +		     "brstackinsnlen,brstackdisasm,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),
> diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h
> index 650125061530..4a7797dd6d09 100644
> --- a/tools/perf/util/dump-insn.h
> +++ b/tools/perf/util/dump-insn.h
> @@ -11,6 +11,7 @@ struct thread;
>  struct perf_insn {
>  	/* Initialized by callers: */
>  	struct thread *thread;
> +	struct machine *machine;
>  	u8	      cpumode;
>  	bool	      is64bit;
>  	int	      cpu;
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> index 8e4e3cffd677..bca4449f0fa8 100644
> --- a/tools/perf/util/print_insn.c
> +++ b/tools/perf/util/print_insn.c
> @@ -12,6 +12,7 @@
>  #include "machine.h"
>  #include "thread.h"
>  #include "print_insn.h"
> +#include "dump-insn.h"
>  #include "map.h"
>  #include "dso.h"
>  
> @@ -71,6 +72,57 @@ static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
>  	return 0;
>  }
>  
> +static void dump_insn_x86(struct thread *thread, cs_insn *insn, struct perf_insn *x)
> +{
> +	struct addr_location al;
> +	bool printed = false;
> +
> +	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, x->cpumode, op->imm, &al) &&
> +		    al.sym &&
> +		    al.addr < al.sym->end) {
> +			snprintf(x->out, sizeof(x->out), "%s %s+%#" PRIx64 " [%#" PRIx64 "]", insn[0].mnemonic,
> +					al.sym->name, al.addr - al.sym->start, op->imm);
> +			printed = true;
> +		}
> +		addr_location__exit(&al);
> +	}
> +
> +	if (!printed)
> +		snprintf(x->out, sizeof(x->out), "%s %s", insn[0].mnemonic, insn[0].op_str);
> +}
> +
> +const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
> +			 u8 *inbuf, int inlen, int *lenp)
> +{
> +	int ret;
> +	int count;
> +	cs_insn *insn;
> +	csh cs_handle;
> +
> +	ret = capstone_init(x->machine, &cs_handle, x->is64bit);
> +	if (ret < 0)
> +		return NULL;
> +
> +	count = cs_disasm(cs_handle, (uint8_t *)inbuf, inlen, ip, 1, &insn);
> +	if (count > 0) {
> +		if (machine__normalized_is(x->machine, "x86"))
> +			dump_insn_x86(x->thread, &insn[0], x);
> +		else
> +			snprintf(x->out, sizeof(x->out), "%s %s",
> +					insn[0].mnemonic, insn[0].op_str);
> +		*lenp = insn->size;
> +		cs_free(insn, count);
> +	} else {
> +		return NULL;
> +	}
> +	return x->out;
> +}
> +

Better to factor out a function that does not depend on "sample"
e.g. see fprintf_insn_asm() below.

static size_t print_insn_x86(struct thread *thread, u8 cpumode, 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, 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;
}

static bool is64bitip(struct machine *machine, struct addr_location *al)
{
	const struct dso *dso = al->map ? map__dso(al->map) : NULL;

	if (dso)
		return dso->is_64_bit;

	return machine__is(machine, "x86_64") ||
		machine__normalized_is(machine, "arm64") ||
		machine__normalized_is(machine, "s390");
}

static ssize_t fprintf_insn_asm(struct machine *machine, struct thread *thread, u8 cpumode,
				bool is64bit, const uint8_t *code, size_t code_size,
				uint64_t ip, int *lenp, FILE *fp)
{
	size_t printed = 0;
	cs_insn *insn;
	csh cs_handle;
	size_t count;
	int ret;

	/* TODO: Try to initiate capstone only once but need a proper place. */
	ret = capstone_init(machine, &cs_handle, is64bit);
	if (ret < 0)
		return ret;

	count = cs_disasm(cs_handle, code, code_size, ip, 1, &insn);
	if (count > 0) {
		if (machine__normalized_is(machine, "x86"))
			printed += print_insn_x86(thread, cpumode, &insn[0], fp);
		else
			printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
		if (lenp)
			*lenp = insn->size;
		cs_free(insn, count);
	} else {
		printed += fprintf(fp, "illegal instruction");
	}

	cs_close(&cs_handle);
	return printed;
}

size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
				struct machine *machine, FILE *fp,
				struct addr_location *al)
{
	bool is64bit = is64bitip(machine, al);
	ssize_t printed;

	printed = fprintf_insn_asm(machine, thread, sample->cpumode, is64bit,
				   (uint8_t *)sample->insn, sample->insn_len,
				   sample->ip, NULL, fp);
	if (printed < 0)
		return sample__fprintf_insn_raw(sample, fp);

	return printed;
}

>  static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>  			     cs_insn *insn, FILE *fp)
>  {
> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> index 6447dd41b543..c2a6391a45ce 100644
> --- a/tools/perf/util/print_insn.h
> +++ b/tools/perf/util/print_insn.h
> @@ -8,9 +8,12 @@
>  struct perf_sample;
>  struct thread;
>  struct machine;
> +struct perf_insn;
>  
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
>  				struct machine *machine, FILE *fp, struct addr_location *al);
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> +const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
> +                         u8 *inbuf, int inlen, int *lenp);
>  
>  #endif /* PERF_PRINT_INSN_H */

Re: [PATCH v4 2/2] perf, script, capstone: Add support for -F +brstackdisasm

[Andi Kleen]

> Better to factor out a function that does not depend on "sample"
> e.g. see fprintf_insn_asm() below.

this doesn't work because it completely ignores the need of the
cs_dump_insn caller for the path that i actually need for my feature,
which requires to return a string.  I didn't apply it.

I did the other fixes. Please no more pointless refactors

-Andi

Re: [PATCH v4 2/2] perf, script, capstone: Add support for -F +brstackdisasm

[Adrian Hunter]

On 19/03/24 00:06, Andi Kleen wrote:
>> Better to factor out a function that does not depend on "sample"
>> e.g. see fprintf_insn_asm() below.
> 
> this doesn't work because it completely ignores the need of the
> cs_dump_insn caller for the path that i actually need for my feature,
> which requires to return a string.  I didn't apply it.

I would probably change the call sites because they already have a
file descriptor, but output to memory is doable:


const char *cs_dump_insn(struct perf_insn *x, uint64_t ip, u8 *inbuf, int inlen, int *lenp)
{
	FILE *fp = fmemopen(x->out, sizeof(x->out), "w+");
	const char *ret = NULL;
	ssize_t printed;

	if (!fp)
		return NULL;

	printed = fprintf_insn_asm(x->machine, x->thread, x->cpumode, x->is64bit,
				   (uint8_t *)inbuf, inlen, ip, lenp, fp);
	if (printed < 0 || fflush(fp))
		goto out_close;

	ret = x->out;
out_close:
	fclose(fp);
	return ret;
}

Re: [PATCH v4 2/2] perf, script, capstone: Add support for -F +brstackdisasm

[Andi Kleen]

On Tue, Mar 19, 2024 at 08:52:33AM +0200, Adrian Hunter wrote:
> On 19/03/24 00:06, Andi Kleen wrote:
> >> Better to factor out a function that does not depend on "sample"
> >> e.g. see fprintf_insn_asm() below.
> > 
> > this doesn't work because it completely ignores the need of the
> > cs_dump_insn caller for the path that i actually need for my feature,
> > which requires to return a string.  I didn't apply it.
> 
> I would probably change the call sites because they already have a
> file descriptor, but output to memory is doable:

> 	FILE *fp = fmemopen(x->out, sizeof(x->out), "w+");

I considered using this at some point, but I'm sure there is some non glibc,
that people build perf with, that doesn't have fmemopen, so I didn't.

Can we just use my version for now and if you prefer more refactor
please submit a follow on cleanup patch?

-andi

[PATCH] perf script: Consolidate capstone print functions

[Adrian Hunter]

Consolidate capstone print functions, to reduce duplication. Amend call
sites to use a file pointer for output, which is consistent with most
perf tools print functions. Add print_opts with an option to print also
the hex value of a resolved symbol+offset.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


On 20/03/24 02:35, Andi Kleen wrote:
> On Tue, Mar 19, 2024 at 08:52:33AM +0200, Adrian Hunter wrote:
>> On 19/03/24 00:06, Andi Kleen wrote:
>>>> Better to factor out a function that does not depend on "sample"
>>>> e.g. see fprintf_insn_asm() below.
>>>
>>> this doesn't work because it completely ignores the need of the
>>> cs_dump_insn caller for the path that i actually need for my feature,
>>> which requires to return a string.  I didn't apply it.
>>
>> I would probably change the call sites because they already have a
>> file descriptor, but output to memory is doable:
> 
>> 	FILE *fp = fmemopen(x->out, sizeof(x->out), "w+");
> 
> I considered using this at some point, but I'm sure there is some non glibc,
> that people build perf with, that doesn't have fmemopen, so I didn't.

fmemopen() is POSIX since 2008

> 
> Can we just use my version for now and if you prefer more refactor
> please submit a follow on cleanup patch?

Sure, here is a follow on cleanup patch.


 tools/perf/builtin-script.c  |  43 ++++++++++-----
 tools/perf/util/print_insn.c | 103 ++++++++++++-----------------------
 tools/perf/util/print_insn.h |   7 ++-
 3 files changed, 67 insertions(+), 86 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 59933bd52e0f..6384acf8dad7 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1165,18 +1165,29 @@ static int print_srccode(struct thread *thread, u8 cpumode, uint64_t addr)
 	return ret;
 }
 
-static const char *any_dump_insn(struct perf_event_attr *attr __maybe_unused,
-			struct perf_insn *x, uint64_t ip,
-			u8 *inbuf, int inlen, int *lenp)
+static int any_dump_insn(struct perf_event_attr *attr __maybe_unused,
+			 struct perf_insn *x, uint64_t ip,
+			 u8 *inbuf, int inlen, int *lenp,
+			 FILE *fp)
 {
 #ifdef HAVE_LIBCAPSTONE_SUPPORT
 	if (PRINT_FIELD(BRSTACKDISASM)) {
-		const char *p = cs_dump_insn(x, ip, inbuf, inlen, lenp);
-		if (p)
-			return p;
+		int printed = fprintf_insn_asm(x->machine, x->thread, x->cpumode, x->is64bit,
+					       (uint8_t *)inbuf, inlen, ip, lenp,
+					       PRINT_INSN_IMM_HEX, fp);
+
+		if (printed > 0)
+			return printed;
 	}
 #endif
-	return dump_insn(x, ip, inbuf, inlen, lenp);
+	return fprintf(fp, "%s", dump_insn(x, ip, inbuf, inlen, lenp));
+}
+
+static int add_padding(FILE *fp, int printed, int padding)
+{
+	if (printed >= 0 && printed < padding)
+		printed += fprintf(fp, "%*s", padding - printed, "");
+	return printed;
 }
 
 static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
@@ -1186,8 +1197,10 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
 			    struct thread *thread)
 {
 	int ilen = 0;
-	int printed = fprintf(fp, "\t%016" PRIx64 "\t%-30s\t", ip,
-			      any_dump_insn(attr, x, ip, inbuf, len, &ilen));
+	int printed = fprintf(fp, "\t%016" PRIx64 "\t", ip);
+
+	printed += add_padding(fp, any_dump_insn(attr, x, ip, inbuf, len, &ilen, fp), 30);
+	printed += fprintf(fp, "\t");
 
 	if (PRINT_FIELD(BRSTACKINSNLEN))
 		printed += fprintf(fp, "ilen: %d\t", ilen);
@@ -1330,8 +1343,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 				break;
 			} else {
 				ilen = 0;
-				printed += fprintf(fp, "\t%016" PRIx64 "\t%s", ip,
-						   any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen));
+				printed += fprintf(fp, "\t%016" PRIx64 "\t", ip);
+				printed += any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen, fp);
 				if (PRINT_FIELD(BRSTACKINSNLEN))
 					printed += fprintf(fp, "\tilen: %d", ilen);
 				printed += fprintf(fp, "\n");
@@ -1378,8 +1391,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 		if (len <= 0)
 			goto out;
 		ilen = 0;
-		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", sample->ip,
-			any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen));
+		printed += fprintf(fp, "\t%016" PRIx64 "\t", sample->ip);
+		printed += any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen, fp);
 		if (PRINT_FIELD(BRSTACKINSNLEN))
 			printed += fprintf(fp, "\tilen: %d", ilen);
 		printed += fprintf(fp, "\n");
@@ -1389,8 +1402,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
 	}
 	for (off = 0; off <= end - start; off += ilen) {
 		ilen = 0;
-		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", start + off,
-				   any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen));
+		printed += fprintf(fp, "\t%016" PRIx64 "\t", start + off);
+		printed += any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen, fp);
 		if (PRINT_FIELD(BRSTACKINSNLEN))
 			printed += fprintf(fp, "\tilen: %d", ilen);
 		printed += fprintf(fp, "\n");
diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
index 8825330d435f..931a2c5293c9 100644
--- a/tools/perf/util/print_insn.c
+++ b/tools/perf/util/print_insn.c
@@ -72,59 +72,8 @@ static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
 	return 0;
 }
 
-static void dump_insn_x86(struct thread *thread, cs_insn *insn, struct perf_insn *x)
-{
-	struct addr_location al;
-	bool printed = false;
-
-	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, x->cpumode, op->imm, &al) &&
-		    al.sym &&
-		    al.addr < al.sym->end) {
-			snprintf(x->out, sizeof(x->out), "%s %s+%#" PRIx64 " [%#" PRIx64 "]", insn[0].mnemonic,
-					al.sym->name, al.addr - al.sym->start, op->imm);
-			printed = true;
-		}
-		addr_location__exit(&al);
-	}
-
-	if (!printed)
-		snprintf(x->out, sizeof(x->out), "%s %s", insn[0].mnemonic, insn[0].op_str);
-}
-
-const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
-			 u8 *inbuf, int inlen, int *lenp)
-{
-	int ret;
-	int count;
-	cs_insn *insn;
-	csh cs_handle;
-
-	ret = capstone_init(x->machine, &cs_handle, x->is64bit);
-	if (ret < 0)
-		return NULL;
-
-	count = cs_disasm(cs_handle, (uint8_t *)inbuf, inlen, ip, 1, &insn);
-	if (count > 0) {
-		if (machine__normalized_is(x->machine, "x86"))
-			dump_insn_x86(x->thread, &insn[0], x);
-		else
-			snprintf(x->out, sizeof(x->out), "%s %s",
-					insn[0].mnemonic, insn[0].op_str);
-		*lenp = insn->size;
-		cs_free(insn, count);
-	} else {
-		return NULL;
-	}
-	return x->out;
-}
-
-static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
-			     cs_insn *insn, FILE *fp)
+static size_t print_insn_x86(struct thread *thread, u8 cpumode, cs_insn *insn,
+			     int print_opts, FILE *fp)
 {
 	struct addr_location al;
 	size_t printed = 0;
@@ -134,9 +83,11 @@ static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
 
 		addr_location__init(&al);
 		if (op->type == X86_OP_IMM &&
-		    thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
+		    thread__find_symbol(thread, cpumode, op->imm, &al)) {
 			printed += fprintf(fp, "%s ", insn[0].mnemonic);
 			printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
+			if (print_opts & PRINT_INSN_IMM_HEX)
+				printed += fprintf(fp, " [%#" PRIx64 "]", op->imm);
 			addr_location__exit(&al);
 			return printed;
 		}
@@ -159,39 +110,53 @@ static bool is64bitip(struct machine *machine, struct addr_location *al)
 		machine__normalized_is(machine, "s390");
 }
 
-size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
-				struct machine *machine, FILE *fp,
-				struct addr_location *al)
+ssize_t fprintf_insn_asm(struct machine *machine, struct thread *thread, u8 cpumode,
+			 bool is64bit, const uint8_t *code, size_t code_size,
+			 uint64_t ip, int *lenp, int print_opts, FILE *fp)
 {
-	csh cs_handle;
+	size_t printed;
 	cs_insn *insn;
+	csh cs_handle;
 	size_t count;
-	size_t printed = 0;
 	int ret;
-	bool is64bit = is64bitip(machine, al);
 
 	/* TODO: Try to initiate capstone only once but need a proper place. */
 	ret = capstone_init(machine, &cs_handle, is64bit);
-	if (ret < 0) {
-		/* fallback */
-		return sample__fprintf_insn_raw(sample, fp);
-	}
+	if (ret < 0)
+		return ret;
 
-	count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
-			  sample->ip, 1, &insn);
+	count = cs_disasm(cs_handle, code, code_size, ip, 1, &insn);
 	if (count > 0) {
 		if (machine__normalized_is(machine, "x86"))
-			printed += print_insn_x86(sample, thread, &insn[0], fp);
+			printed = print_insn_x86(thread, cpumode, &insn[0], print_opts, fp);
 		else
-			printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
+			printed = fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
+		if (lenp)
+			*lenp = insn->size;
 		cs_free(insn, count);
 	} else {
-		printed += fprintf(fp, "illegal instruction");
+		printed = -1;
 	}
 
 	cs_close(&cs_handle);
 	return printed;
 }
+
+size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
+				struct machine *machine, FILE *fp,
+				struct addr_location *al)
+{
+	bool is64bit = is64bitip(machine, al);
+	ssize_t printed;
+
+	printed = fprintf_insn_asm(machine, thread, sample->cpumode, is64bit,
+				   (uint8_t *)sample->insn, sample->insn_len,
+				   sample->ip, NULL, 0, fp);
+	if (printed < 0)
+		return sample__fprintf_insn_raw(sample, fp);
+
+	return printed;
+}
 #else
 size_t sample__fprintf_insn_asm(struct perf_sample *sample __maybe_unused,
 				struct thread *thread __maybe_unused,
diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
index c2a6391a45ce..07d11af3fc1c 100644
--- a/tools/perf/util/print_insn.h
+++ b/tools/perf/util/print_insn.h
@@ -10,10 +10,13 @@ struct thread;
 struct machine;
 struct perf_insn;
 
+#define PRINT_INSN_IMM_HEX		(1<<0)
+
 size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
 				struct machine *machine, FILE *fp, struct addr_location *al);
 size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
-const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
-                         u8 *inbuf, int inlen, int *lenp);
+ssize_t fprintf_insn_asm(struct machine *machine, struct thread *thread, u8 cpumode,
+			 bool is64bit, const uint8_t *code, size_t code_size,
+			 uint64_t ip, int *lenp, int print_opts, FILE *fp);
 
 #endif /* PERF_PRINT_INSN_H */
-- 
2.34.1

Re: [PATCH] perf script: Consolidate capstone print functions

[Arnaldo Carvalho de Melo]

On Fri, Mar 22, 2024 at 12:21:58PM +0200, Adrian Hunter wrote:
> Consolidate capstone print functions, to reduce duplication. Amend call
> sites to use a file pointer for output, which is consistent with most
> perf tools print functions. Add print_opts with an option to print also
> the hex value of a resolved symbol+offset.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

I'll fix this:

  38    11.53 ubuntu:18.04                  : FAIL gcc version 7.5.0 (Ubuntu 7.5.0-3ubuntu1~18.04) 
    util/print_insn.c: In function 'print_insn_x86':
    util/print_insn.c:90:35: error: expected ')' before 'PRIx64'
         printed += fprintf(fp, " [%#" PRIx64 "]", op->imm);
                                       ^~~~~~
    util/print_insn.c:90:32: error: conversion lacks type at end of format [-Werror=format=]
         printed += fprintf(fp, " [%#" PRIx64 "]", op->imm);
                                    ^
    cc1: all warnings being treated as errors
    /git/perf-6.8.0/tools/build/Makefile.build:158: recipe for target 'util' failed
    make[3]: *** [util] Error 2
 
> 
> On 20/03/24 02:35, Andi Kleen wrote:
> > On Tue, Mar 19, 2024 at 08:52:33AM +0200, Adrian Hunter wrote:
> >> On 19/03/24 00:06, Andi Kleen wrote:
> >>>> Better to factor out a function that does not depend on "sample"
> >>>> e.g. see fprintf_insn_asm() below.
> >>>
> >>> this doesn't work because it completely ignores the need of the
> >>> cs_dump_insn caller for the path that i actually need for my feature,
> >>> which requires to return a string.  I didn't apply it.
> >>
> >> I would probably change the call sites because they already have a
> >> file descriptor, but output to memory is doable:
> > 
> >> 	FILE *fp = fmemopen(x->out, sizeof(x->out), "w+");
> > 
> > I considered using this at some point, but I'm sure there is some non glibc,
> > that people build perf with, that doesn't have fmemopen, so I didn't.
> 
> fmemopen() is POSIX since 2008
> 
> > 
> > Can we just use my version for now and if you prefer more refactor
> > please submit a follow on cleanup patch?
> 
> Sure, here is a follow on cleanup patch.
> 
> 
>  tools/perf/builtin-script.c  |  43 ++++++++++-----
>  tools/perf/util/print_insn.c | 103 ++++++++++++-----------------------
>  tools/perf/util/print_insn.h |   7 ++-
>  3 files changed, 67 insertions(+), 86 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 59933bd52e0f..6384acf8dad7 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1165,18 +1165,29 @@ static int print_srccode(struct thread *thread, u8 cpumode, uint64_t addr)
>  	return ret;
>  }
>  
> -static const char *any_dump_insn(struct perf_event_attr *attr __maybe_unused,
> -			struct perf_insn *x, uint64_t ip,
> -			u8 *inbuf, int inlen, int *lenp)
> +static int any_dump_insn(struct perf_event_attr *attr __maybe_unused,
> +			 struct perf_insn *x, uint64_t ip,
> +			 u8 *inbuf, int inlen, int *lenp,
> +			 FILE *fp)
>  {
>  #ifdef HAVE_LIBCAPSTONE_SUPPORT
>  	if (PRINT_FIELD(BRSTACKDISASM)) {
> -		const char *p = cs_dump_insn(x, ip, inbuf, inlen, lenp);
> -		if (p)
> -			return p;
> +		int printed = fprintf_insn_asm(x->machine, x->thread, x->cpumode, x->is64bit,
> +					       (uint8_t *)inbuf, inlen, ip, lenp,
> +					       PRINT_INSN_IMM_HEX, fp);
> +
> +		if (printed > 0)
> +			return printed;
>  	}
>  #endif
> -	return dump_insn(x, ip, inbuf, inlen, lenp);
> +	return fprintf(fp, "%s", dump_insn(x, ip, inbuf, inlen, lenp));
> +}
> +
> +static int add_padding(FILE *fp, int printed, int padding)
> +{
> +	if (printed >= 0 && printed < padding)
> +		printed += fprintf(fp, "%*s", padding - printed, "");
> +	return printed;
>  }
>  
>  static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
> @@ -1186,8 +1197,10 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
>  			    struct thread *thread)
>  {
>  	int ilen = 0;
> -	int printed = fprintf(fp, "\t%016" PRIx64 "\t%-30s\t", ip,
> -			      any_dump_insn(attr, x, ip, inbuf, len, &ilen));
> +	int printed = fprintf(fp, "\t%016" PRIx64 "\t", ip);
> +
> +	printed += add_padding(fp, any_dump_insn(attr, x, ip, inbuf, len, &ilen, fp), 30);
> +	printed += fprintf(fp, "\t");
>  
>  	if (PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += fprintf(fp, "ilen: %d\t", ilen);
> @@ -1330,8 +1343,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  				break;
>  			} else {
>  				ilen = 0;
> -				printed += fprintf(fp, "\t%016" PRIx64 "\t%s", ip,
> -						   any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen));
> +				printed += fprintf(fp, "\t%016" PRIx64 "\t", ip);
> +				printed += any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen, fp);
>  				if (PRINT_FIELD(BRSTACKINSNLEN))
>  					printed += fprintf(fp, "\tilen: %d", ilen);
>  				printed += fprintf(fp, "\n");
> @@ -1378,8 +1391,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  		if (len <= 0)
>  			goto out;
>  		ilen = 0;
> -		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", sample->ip,
> -			any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen));
> +		printed += fprintf(fp, "\t%016" PRIx64 "\t", sample->ip);
> +		printed += any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen, fp);
>  		if (PRINT_FIELD(BRSTACKINSNLEN))
>  			printed += fprintf(fp, "\tilen: %d", ilen);
>  		printed += fprintf(fp, "\n");
> @@ -1389,8 +1402,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  	}
>  	for (off = 0; off <= end - start; off += ilen) {
>  		ilen = 0;
> -		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", start + off,
> -				   any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen));
> +		printed += fprintf(fp, "\t%016" PRIx64 "\t", start + off);
> +		printed += any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen, fp);
>  		if (PRINT_FIELD(BRSTACKINSNLEN))
>  			printed += fprintf(fp, "\tilen: %d", ilen);
>  		printed += fprintf(fp, "\n");
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> index 8825330d435f..931a2c5293c9 100644
> --- a/tools/perf/util/print_insn.c
> +++ b/tools/perf/util/print_insn.c
> @@ -72,59 +72,8 @@ static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
>  	return 0;
>  }
>  
> -static void dump_insn_x86(struct thread *thread, cs_insn *insn, struct perf_insn *x)
> -{
> -	struct addr_location al;
> -	bool printed = false;
> -
> -	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, x->cpumode, op->imm, &al) &&
> -		    al.sym &&
> -		    al.addr < al.sym->end) {
> -			snprintf(x->out, sizeof(x->out), "%s %s+%#" PRIx64 " [%#" PRIx64 "]", insn[0].mnemonic,
> -					al.sym->name, al.addr - al.sym->start, op->imm);
> -			printed = true;
> -		}
> -		addr_location__exit(&al);
> -	}
> -
> -	if (!printed)
> -		snprintf(x->out, sizeof(x->out), "%s %s", insn[0].mnemonic, insn[0].op_str);
> -}
> -
> -const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
> -			 u8 *inbuf, int inlen, int *lenp)
> -{
> -	int ret;
> -	int count;
> -	cs_insn *insn;
> -	csh cs_handle;
> -
> -	ret = capstone_init(x->machine, &cs_handle, x->is64bit);
> -	if (ret < 0)
> -		return NULL;
> -
> -	count = cs_disasm(cs_handle, (uint8_t *)inbuf, inlen, ip, 1, &insn);
> -	if (count > 0) {
> -		if (machine__normalized_is(x->machine, "x86"))
> -			dump_insn_x86(x->thread, &insn[0], x);
> -		else
> -			snprintf(x->out, sizeof(x->out), "%s %s",
> -					insn[0].mnemonic, insn[0].op_str);
> -		*lenp = insn->size;
> -		cs_free(insn, count);
> -	} else {
> -		return NULL;
> -	}
> -	return x->out;
> -}
> -
> -static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
> -			     cs_insn *insn, FILE *fp)
> +static size_t print_insn_x86(struct thread *thread, u8 cpumode, cs_insn *insn,
> +			     int print_opts, FILE *fp)
>  {
>  	struct addr_location al;
>  	size_t printed = 0;
> @@ -134,9 +83,11 @@ static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>  
>  		addr_location__init(&al);
>  		if (op->type == X86_OP_IMM &&
> -		    thread__find_symbol(thread, sample->cpumode, op->imm, &al)) {
> +		    thread__find_symbol(thread, cpumode, op->imm, &al)) {
>  			printed += fprintf(fp, "%s ", insn[0].mnemonic);
>  			printed += symbol__fprintf_symname_offs(al.sym, &al, fp);
> +			if (print_opts & PRINT_INSN_IMM_HEX)
> +				printed += fprintf(fp, " [%#" PRIx64 "]", op->imm);
>  			addr_location__exit(&al);
>  			return printed;
>  		}
> @@ -159,39 +110,53 @@ static bool is64bitip(struct machine *machine, struct addr_location *al)
>  		machine__normalized_is(machine, "s390");
>  }
>  
> -size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
> -				struct machine *machine, FILE *fp,
> -				struct addr_location *al)
> +ssize_t fprintf_insn_asm(struct machine *machine, struct thread *thread, u8 cpumode,
> +			 bool is64bit, const uint8_t *code, size_t code_size,
> +			 uint64_t ip, int *lenp, int print_opts, FILE *fp)
>  {
> -	csh cs_handle;
> +	size_t printed;
>  	cs_insn *insn;
> +	csh cs_handle;
>  	size_t count;
> -	size_t printed = 0;
>  	int ret;
> -	bool is64bit = is64bitip(machine, al);
>  
>  	/* TODO: Try to initiate capstone only once but need a proper place. */
>  	ret = capstone_init(machine, &cs_handle, is64bit);
> -	if (ret < 0) {
> -		/* fallback */
> -		return sample__fprintf_insn_raw(sample, fp);
> -	}
> +	if (ret < 0)
> +		return ret;
>  
> -	count = cs_disasm(cs_handle, (uint8_t *)sample->insn, sample->insn_len,
> -			  sample->ip, 1, &insn);
> +	count = cs_disasm(cs_handle, code, code_size, ip, 1, &insn);
>  	if (count > 0) {
>  		if (machine__normalized_is(machine, "x86"))
> -			printed += print_insn_x86(sample, thread, &insn[0], fp);
> +			printed = print_insn_x86(thread, cpumode, &insn[0], print_opts, fp);
>  		else
> -			printed += fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> +			printed = fprintf(fp, "%s %s", insn[0].mnemonic, insn[0].op_str);
> +		if (lenp)
> +			*lenp = insn->size;
>  		cs_free(insn, count);
>  	} else {
> -		printed += fprintf(fp, "illegal instruction");
> +		printed = -1;
>  	}
>  
>  	cs_close(&cs_handle);
>  	return printed;
>  }
> +
> +size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
> +				struct machine *machine, FILE *fp,
> +				struct addr_location *al)
> +{
> +	bool is64bit = is64bitip(machine, al);
> +	ssize_t printed;
> +
> +	printed = fprintf_insn_asm(machine, thread, sample->cpumode, is64bit,
> +				   (uint8_t *)sample->insn, sample->insn_len,
> +				   sample->ip, NULL, 0, fp);
> +	if (printed < 0)
> +		return sample__fprintf_insn_raw(sample, fp);
> +
> +	return printed;
> +}
>  #else
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample __maybe_unused,
>  				struct thread *thread __maybe_unused,
> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> index c2a6391a45ce..07d11af3fc1c 100644
> --- a/tools/perf/util/print_insn.h
> +++ b/tools/perf/util/print_insn.h
> @@ -10,10 +10,13 @@ struct thread;
>  struct machine;
>  struct perf_insn;
>  
> +#define PRINT_INSN_IMM_HEX		(1<<0)
> +
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
>  				struct machine *machine, FILE *fp, struct addr_location *al);
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> -const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
> -                         u8 *inbuf, int inlen, int *lenp);
> +ssize_t fprintf_insn_asm(struct machine *machine, struct thread *thread, u8 cpumode,
> +			 bool is64bit, const uint8_t *code, size_t code_size,
> +			 uint64_t ip, int *lenp, int print_opts, FILE *fp);
>  
>  #endif /* PERF_PRINT_INSN_H */
> -- 
> 2.34.1

Re: [PATCH v4 2/2] perf, script, capstone: Add support for -F +brstackdisasm

[Adrian Hunter]

On 8/03/24 19:06, Andi Kleen wrote:
> Support capstone output for the -F +brstackinsn branch dump.
> The new output is enabled with the new field brstackdisasm
> This was possible before with --xed, but now also allow
> it for users that don't have xed using the builtin capstone support.
> 
> Before:
> 
> perf record -b emacs -Q --batch '()'
> perf script -F +brstackinsn
> ...
>           emacs   55778 1814366.755945:     151564 cycles:P:      7f0ab2d17192 intel_check_word.constprop.0+0x162 (/usr/lib64/ld-linux-x86-64.s>        intel_check_word.constprop.0+237:
>         00007f0ab2d1711d        insn: 75 e6                     # PRED 3 cycles [3]
>         00007f0ab2d17105        insn: 73 51
>         00007f0ab2d17107        insn: 48 89 c1
>         00007f0ab2d1710a        insn: 48 39 ca
>         00007f0ab2d1710d        insn: 73 96
>         00007f0ab2d1710f        insn: 48 8d 04 11
>         00007f0ab2d17113        insn: 48 d1 e8
>         00007f0ab2d17116        insn: 49 8d 34 c1
>         00007f0ab2d1711a        insn: 44 3a 06
>         00007f0ab2d1711d        insn: 75 e6                     # PRED 3 cycles [6] 3.00 IPC
>         00007f0ab2d17105        insn: 73 51                     # PRED 1 cycles [7] 1.00 IPC
>         00007f0ab2d17158        insn: 48 8d 50 01
>         00007f0ab2d1715c        insn: eb 92                     # PRED 1 cycles [8] 2.00 IPC
>         00007f0ab2d170f0        insn: 48 39 ca
>         00007f0ab2d170f3        insn: 73 b0                     # PRED 1 cycles [9] 2.00 IPC
> 
> After (perf must be compiled with capstone):
> 
> perf script -F +brstackdisasm
> 
> ...
>            emacs   55778 1814366.755945:     151564 cycles:P:      7f0ab2d17192 intel_check_word.constprop.0+0x162 (/usr/lib64/ld-linux-x86-64.s>        intel_check_word.constprop.0+237:
>         00007f0ab2d1711d        jne intel_check_word.constprop.0+0xd5   # PRED 3 cycles [3]
>         00007f0ab2d17105        jae intel_check_word.constprop.0+0x128
>         00007f0ab2d17107        movq %rax, %rcx
>         00007f0ab2d1710a        cmpq %rcx, %rdx
>         00007f0ab2d1710d        jae intel_check_word.constprop.0+0x75
>         00007f0ab2d1710f        leaq (%rcx, %rdx), %rax
>         00007f0ab2d17113        shrq $1, %rax
>         00007f0ab2d17116        leaq (%r9, %rax, 8), %rsi
>         00007f0ab2d1711a        cmpb (%rsi), %r8b
>         00007f0ab2d1711d        jne intel_check_word.constprop.0+0xd5   # PRED 3 cycles [6] 3.00 IPC
>         00007f0ab2d17105        jae intel_check_word.constprop.0+0x128  # PRED 1 cycles [7] 1.00 IPC
>         00007f0ab2d17158        leaq 1(%rax), %rdx
>         00007f0ab2d1715c        jmp intel_check_word.constprop.0+0xc0   # PRED 1 cycles [8] 2.00 IPC
>         00007f0ab2d170f0        cmpq %rcx, %rdx
>         00007f0ab2d170f3        jae intel_check_word.constprop.0+0x75   # PRED 1 cycles [9] 2.00 IPC
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Comma in subject is slightly odd, and needs fixes to build with NO_CAPSTONE=1

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index db18d2c54c59..59933bd52e0f 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1165,7 +1165,7 @@ static int print_srccode(struct thread *thread, u8 cpumode, uint64_t addr)
 	return ret;
 }
 
-static const char *any_dump_insn(struct perf_event_attr *attr,
+static const char *any_dump_insn(struct perf_event_attr *attr __maybe_unused,
 			struct perf_insn *x, uint64_t ip,
 			u8 *inbuf, int inlen, int *lenp)
 {
diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
index bca4449f0fa8..8825330d435f 100644
--- a/tools/perf/util/print_insn.c
+++ b/tools/perf/util/print_insn.c
@@ -196,7 +196,8 @@ size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *threa
 size_t sample__fprintf_insn_asm(struct perf_sample *sample __maybe_unused,
 				struct thread *thread __maybe_unused,
 				struct machine *machine __maybe_unused,
-				FILE *fp __maybe_unused)
+				FILE *fp __maybe_unused,
+				struct addr_location *al __maybe_unused)
 {
 	return 0;
 }


Otherwise:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> 
> ---
> 
> v2: Use brstackdisasm instead of keying of disasm
> ---
>  tools/perf/Documentation/perf-script.txt |  7 +++-
>  tools/perf/builtin-script.c              | 32 +++++++++++----
>  tools/perf/util/dump-insn.h              |  1 +
>  tools/perf/util/print_insn.c             | 52 ++++++++++++++++++++++++
>  tools/perf/util/print_insn.h             |  3 ++
>  5 files changed, 86 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index 005e51df855e..ff086ef05a0c 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -132,9 +132,9 @@ 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, disasm,
> +        brstackinsn, brstackinsnlen, brstackdisasm, 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.
> +        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.
> @@ -257,6 +257,9 @@ OPTIONS
>  	can’t know the next sequential instruction after an unconditional branch unless
>  	you calculate that based on its length.
>  
> +	brstackdisasm acts like brstackinsn, but will print disassembled instructions if
> +	perf is built with the capstone library.
> +
>  	The brstackoff field will print an offset into a specific dso/binary.
>  
>  	With the metric option perf script can compute metrics for
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 0299b1ed8744..db18d2c54c59 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -136,6 +136,7 @@ enum perf_output_field {
>  	PERF_OUTPUT_RETIRE_LAT      = 1ULL << 40,
>  	PERF_OUTPUT_DSOFF           = 1ULL << 41,
>  	PERF_OUTPUT_DISASM          = 1ULL << 42,
> +	PERF_OUTPUT_BRSTACKDISASM   = 1ULL << 43,
>  };
>  
>  struct perf_script {
> @@ -210,6 +211,7 @@ struct output_option {
>  	{.str = "vcpu", .field = PERF_OUTPUT_VCPU},
>  	{.str = "cgroup", .field = PERF_OUTPUT_CGROUP},
>  	{.str = "retire_lat", .field = PERF_OUTPUT_RETIRE_LAT},
> +	{.str = "brstackdisasm", .field = PERF_OUTPUT_BRSTACKDISASM},
>  };
>  
>  enum {
> @@ -510,7 +512,8 @@ static int evsel__check_attr(struct evsel *evsel, struct perf_session *session)
>  		       "selected. Hence, no address to lookup the source line number.\n");
>  		return -EINVAL;
>  	}
> -	if ((PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN)) && !allow_user_set &&
> +	if ((PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN) || PRINT_FIELD(BRSTACKDISASM))
> +	    && !allow_user_set &&
>  	    !(evlist__combined_branch_type(session->evlist) & PERF_SAMPLE_BRANCH_ANY)) {
>  		pr_err("Display of branch stack assembler requested, but non all-branch filter set\n"
>  		       "Hint: run 'perf record -b ...'\n");
> @@ -1162,6 +1165,20 @@ static int print_srccode(struct thread *thread, u8 cpumode, uint64_t addr)
>  	return ret;
>  }
>  
> +static const char *any_dump_insn(struct perf_event_attr *attr,
> +			struct perf_insn *x, uint64_t ip,
> +			u8 *inbuf, int inlen, int *lenp)
> +{
> +#ifdef HAVE_LIBCAPSTONE_SUPPORT
> +	if (PRINT_FIELD(BRSTACKDISASM)) {
> +		const char *p = cs_dump_insn(x, ip, inbuf, inlen, lenp);
> +		if (p)
> +			return p;
> +	}
> +#endif
> +	return dump_insn(x, ip, inbuf, inlen, lenp);
> +}
> +
>  static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
>  			    struct perf_insn *x, u8 *inbuf, int len,
>  			    int insn, FILE *fp, int *total_cycles,
> @@ -1170,7 +1187,7 @@ static int ip__fprintf_jump(uint64_t ip, struct branch_entry *en,
>  {
>  	int ilen = 0;
>  	int printed = fprintf(fp, "\t%016" PRIx64 "\t%-30s\t", ip,
> -			      dump_insn(x, ip, inbuf, len, &ilen));
> +			      any_dump_insn(attr, x, ip, inbuf, len, &ilen));
>  
>  	if (PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += fprintf(fp, "ilen: %d\t", ilen);
> @@ -1262,6 +1279,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  		nr = max_blocks + 1;
>  
>  	x.thread = thread;
> +	x.machine = machine;
>  	x.cpu = sample->cpu;
>  
>  	printed += fprintf(fp, "%c", '\n');
> @@ -1313,7 +1331,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  			} else {
>  				ilen = 0;
>  				printed += fprintf(fp, "\t%016" PRIx64 "\t%s", ip,
> -						   dump_insn(&x, ip, buffer + off, len - off, &ilen));
> +						   any_dump_insn(attr, &x, ip, buffer + off, len - off, &ilen));
>  				if (PRINT_FIELD(BRSTACKINSNLEN))
>  					printed += fprintf(fp, "\tilen: %d", ilen);
>  				printed += fprintf(fp, "\n");
> @@ -1361,7 +1379,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  			goto out;
>  		ilen = 0;
>  		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", sample->ip,
> -			dump_insn(&x, sample->ip, buffer, len, &ilen));
> +			any_dump_insn(attr, &x, sample->ip, buffer, len, &ilen));
>  		if (PRINT_FIELD(BRSTACKINSNLEN))
>  			printed += fprintf(fp, "\tilen: %d", ilen);
>  		printed += fprintf(fp, "\n");
> @@ -1372,7 +1390,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
>  	for (off = 0; off <= end - start; off += ilen) {
>  		ilen = 0;
>  		printed += fprintf(fp, "\t%016" PRIx64 "\t%s", start + off,
> -				   dump_insn(&x, start + off, buffer + off, len - off, &ilen));
> +				   any_dump_insn(attr, &x, start + off, buffer + off, len - off, &ilen));
>  		if (PRINT_FIELD(BRSTACKINSNLEN))
>  			printed += fprintf(fp, "\tilen: %d", ilen);
>  		printed += fprintf(fp, "\n");
> @@ -1534,7 +1552,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  		printed += fprintf(fp, "\t\t");
>  		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
>  	}
> -	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
> +	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN) || PRINT_FIELD(BRSTACKDISASM))
>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>  
>  	return printed;
> @@ -3940,7 +3958,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,disasm,insnlen,synth,"
> +		     "brstackinsnlen,brstackdisasm,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),
> diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h
> index 650125061530..4a7797dd6d09 100644
> --- a/tools/perf/util/dump-insn.h
> +++ b/tools/perf/util/dump-insn.h
> @@ -11,6 +11,7 @@ struct thread;
>  struct perf_insn {
>  	/* Initialized by callers: */
>  	struct thread *thread;
> +	struct machine *machine;
>  	u8	      cpumode;
>  	bool	      is64bit;
>  	int	      cpu;
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> index 8e4e3cffd677..bca4449f0fa8 100644
> --- a/tools/perf/util/print_insn.c
> +++ b/tools/perf/util/print_insn.c
> @@ -12,6 +12,7 @@
>  #include "machine.h"
>  #include "thread.h"
>  #include "print_insn.h"
> +#include "dump-insn.h"
>  #include "map.h"
>  #include "dso.h"
>  
> @@ -71,6 +72,57 @@ static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
>  	return 0;
>  }
>  
> +static void dump_insn_x86(struct thread *thread, cs_insn *insn, struct perf_insn *x)
> +{
> +	struct addr_location al;
> +	bool printed = false;
> +
> +	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, x->cpumode, op->imm, &al) &&
> +		    al.sym &&
> +		    al.addr < al.sym->end) {
> +			snprintf(x->out, sizeof(x->out), "%s %s+%#" PRIx64 " [%#" PRIx64 "]", insn[0].mnemonic,
> +					al.sym->name, al.addr - al.sym->start, op->imm);
> +			printed = true;
> +		}
> +		addr_location__exit(&al);
> +	}
> +
> +	if (!printed)
> +		snprintf(x->out, sizeof(x->out), "%s %s", insn[0].mnemonic, insn[0].op_str);
> +}
> +
> +const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
> +			 u8 *inbuf, int inlen, int *lenp)
> +{
> +	int ret;
> +	int count;
> +	cs_insn *insn;
> +	csh cs_handle;
> +
> +	ret = capstone_init(x->machine, &cs_handle, x->is64bit);
> +	if (ret < 0)
> +		return NULL;
> +
> +	count = cs_disasm(cs_handle, (uint8_t *)inbuf, inlen, ip, 1, &insn);
> +	if (count > 0) {
> +		if (machine__normalized_is(x->machine, "x86"))
> +			dump_insn_x86(x->thread, &insn[0], x);
> +		else
> +			snprintf(x->out, sizeof(x->out), "%s %s",
> +					insn[0].mnemonic, insn[0].op_str);
> +		*lenp = insn->size;
> +		cs_free(insn, count);
> +	} else {
> +		return NULL;
> +	}
> +	return x->out;
> +}
> +
>  static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>  			     cs_insn *insn, FILE *fp)
>  {
> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> index 6447dd41b543..c2a6391a45ce 100644
> --- a/tools/perf/util/print_insn.h
> +++ b/tools/perf/util/print_insn.h
> @@ -8,9 +8,12 @@
>  struct perf_sample;
>  struct thread;
>  struct machine;
> +struct perf_insn;
>  
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
>  				struct machine *machine, FILE *fp, struct addr_location *al);
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
> +const char *cs_dump_insn(struct perf_insn *x, uint64_t ip,
> +                         u8 *inbuf, int inlen, int *lenp);
>  
>  #endif /* PERF_PRINT_INSN_H */

Re: [PATCH v4 1/2] perf, capstone: Support 32bit code under 64bit OS

[Adrian Hunter]

On 8/03/24 19:06, Andi Kleen wrote:
> Use the DSO to resolve whether an IP is 32bit or 64bit and use that to
> configure capstone to the correct mode. This allows to correctly
> disassemble 32bit code under a 64bit OS.
> 
> % cat > loop.c
> volatile int var;
> int main(void)
> {
> 	int i;
> 	for (i = 0; i < 100000; i++)
> 		var++;
> }
> % gcc -m32 -o loop loop.c
> % perf record -e cycles:u ./loop
> % perf script -F +disasm
>             loop   82665 1833176.618023:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618029:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618031:          7 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618034:         91 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618036:       1242 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> ---
> 
> v2: Factor out DSO lookup into separate function
> v3: Pass down al
> v4: Simplify is64bitip
> ---
>  tools/perf/builtin-script.c  |  9 +++++----
>  tools/perf/util/print_insn.c | 24 ++++++++++++++++++++----
>  tools/perf/util/print_insn.h |  2 +-
>  3 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 37088cc0ff1b..0299b1ed8744 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1517,7 +1517,8 @@ void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
>  static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  				     struct perf_event_attr *attr,
>  				     struct thread *thread,
> -				     struct machine *machine, FILE *fp)
> +				     struct machine *machine, FILE *fp,
> +				     struct addr_location *al)

perf's fprintf functions usually have the "FILE *fp" as the last
parameter, so better to add "struct addr_location *al" as second-to-last.

>  {
>  	int printed = 0;
>  
> @@ -1531,7 +1532,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  	}
>  	if (PRINT_FIELD(DISASM) && sample->insn_len) {
>  		printed += fprintf(fp, "\t\t");
> -		printed += sample__fprintf_insn_asm(sample, thread, machine, fp);
> +		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
>  	}
>  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> @@ -1606,7 +1607,7 @@ static int perf_sample__fprintf_bts(struct perf_sample *sample,
>  	if (print_srcline_last)
>  		printed += map__fprintf_srcline(al->map, al->addr, "\n  ", fp);
>  
> -	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
> +	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
>  	printed += fprintf(fp, "\n");
>  	if (PRINT_FIELD(SRCCODE)) {
>  		int ret = map__fprintf_srccode(al->map, al->addr, stdout,
> @@ -2259,7 +2260,7 @@ static void process_event(struct perf_script *script,
>  
>  	if (evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
>  		perf_sample__fprintf_bpf_output(sample, fp);
> -	perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
> +	perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
>  
>  	if (PRINT_FIELD(PHYS_ADDR))
>  		fprintf(fp, "%16" PRIx64, sample->phys_addr);
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> index 459e0e93d7b1..8e4e3cffd677 100644
> --- a/tools/perf/util/print_insn.c
> +++ b/tools/perf/util/print_insn.c
> @@ -12,6 +12,8 @@
>  #include "machine.h"
>  #include "thread.h"
>  #include "print_insn.h"
> +#include "map.h"
> +#include "dso.h"
>  
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>  {
> @@ -28,12 +30,12 @@ size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>  #ifdef HAVE_LIBCAPSTONE_SUPPORT
>  #include <capstone/capstone.h>
>  
> -static int capstone_init(struct machine *machine, csh *cs_handle)
> +static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
>  {
>  	cs_arch arch;
>  	cs_mode mode;
>  
> -	if (machine__is(machine, "x86_64")) {
> +	if (machine__is(machine, "x86_64") && is64) {
>  		arch = CS_ARCH_X86;
>  		mode = CS_MODE_64;
>  	} else if (machine__normalized_is(machine, "x86")) {
> @@ -93,17 +95,31 @@ static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>  	return printed;
>  }
>  
> +static bool is64bitip(struct machine *machine, struct addr_location *al)
> +{
> +	const struct dso *dso = al->map ? map__dso(al->map) : NULL;
> +
> +	if (dso)
> +		return dso->is_64_bit;
> +
> +	return machine__is(machine, "x86_64") ||
> +		machine__normalized_is(machine, "arm64") ||
> +		machine__normalized_is(machine, "s390");
> +}
> +
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
> -				struct machine *machine, FILE *fp)
> +				struct machine *machine, FILE *fp,
> +				struct addr_location *al)
>  {
>  	csh cs_handle;
>  	cs_insn *insn;
>  	size_t count;
>  	size_t printed = 0;
>  	int ret;
> +	bool is64bit = is64bitip(machine, al);
>  
>  	/* TODO: Try to initiate capstone only once but need a proper place. */
> -	ret = capstone_init(machine, &cs_handle);
> +	ret = capstone_init(machine, &cs_handle, is64bit);
>  	if (ret < 0) {
>  		/* fallback */
>  		return sample__fprintf_insn_raw(sample, fp);


sample__fprintf_insn_asm() stub needs changing too

> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> index 465bdcfcc2fd..6447dd41b543 100644
> --- a/tools/perf/util/print_insn.h
> +++ b/tools/perf/util/print_insn.h
> @@ -10,7 +10,7 @@ struct thread;
>  struct machine;
>  
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
> -				struct machine *machine, FILE *fp);
> +				struct machine *machine, FILE *fp, struct addr_location *al);
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
>  
>  #endif /* PERF_PRINT_INSN_H */

Re: [PATCH v4 1/2] perf, capstone: Support 32bit code under 64bit OS

[Adrian Hunter]

On 8/03/24 19:06, Andi Kleen wrote:
> Use the DSO to resolve whether an IP is 32bit or 64bit and use that to
> configure capstone to the correct mode. This allows to correctly
> disassemble 32bit code under a 64bit OS.
> 
> % cat > loop.c
> volatile int var;
> int main(void)
> {
> 	int i;
> 	for (i = 0; i < 100000; i++)
> 		var++;
> }
> % gcc -m32 -o loop loop.c
> % perf record -e cycles:u ./loop
> % perf script -F +disasm
>             loop   82665 1833176.618023:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618029:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618031:          7 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618034:         91 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>             loop   82665 1833176.618036:       1242 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Comma in subject is slightly odd, otherwise:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> 
> ---
> 
> v2: Factor out DSO lookup into separate function
> v3: Pass down al
> v4: Simplify is64bitip
> ---
>  tools/perf/builtin-script.c  |  9 +++++----
>  tools/perf/util/print_insn.c | 24 ++++++++++++++++++++----
>  tools/perf/util/print_insn.h |  2 +-
>  3 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 37088cc0ff1b..0299b1ed8744 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1517,7 +1517,8 @@ void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
>  static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  				     struct perf_event_attr *attr,
>  				     struct thread *thread,
> -				     struct machine *machine, FILE *fp)
> +				     struct machine *machine, FILE *fp,
> +				     struct addr_location *al)
>  {
>  	int printed = 0;
>  
> @@ -1531,7 +1532,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>  	}
>  	if (PRINT_FIELD(DISASM) && sample->insn_len) {
>  		printed += fprintf(fp, "\t\t");
> -		printed += sample__fprintf_insn_asm(sample, thread, machine, fp);
> +		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
>  	}
>  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
> @@ -1606,7 +1607,7 @@ static int perf_sample__fprintf_bts(struct perf_sample *sample,
>  	if (print_srcline_last)
>  		printed += map__fprintf_srcline(al->map, al->addr, "\n  ", fp);
>  
> -	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
> +	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
>  	printed += fprintf(fp, "\n");
>  	if (PRINT_FIELD(SRCCODE)) {
>  		int ret = map__fprintf_srccode(al->map, al->addr, stdout,
> @@ -2259,7 +2260,7 @@ static void process_event(struct perf_script *script,
>  
>  	if (evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
>  		perf_sample__fprintf_bpf_output(sample, fp);
> -	perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
> +	perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
>  
>  	if (PRINT_FIELD(PHYS_ADDR))
>  		fprintf(fp, "%16" PRIx64, sample->phys_addr);
> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
> index 459e0e93d7b1..8e4e3cffd677 100644
> --- a/tools/perf/util/print_insn.c
> +++ b/tools/perf/util/print_insn.c
> @@ -12,6 +12,8 @@
>  #include "machine.h"
>  #include "thread.h"
>  #include "print_insn.h"
> +#include "map.h"
> +#include "dso.h"
>  
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>  {
> @@ -28,12 +30,12 @@ size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>  #ifdef HAVE_LIBCAPSTONE_SUPPORT
>  #include <capstone/capstone.h>
>  
> -static int capstone_init(struct machine *machine, csh *cs_handle)
> +static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
>  {
>  	cs_arch arch;
>  	cs_mode mode;
>  
> -	if (machine__is(machine, "x86_64")) {
> +	if (machine__is(machine, "x86_64") && is64) {
>  		arch = CS_ARCH_X86;
>  		mode = CS_MODE_64;
>  	} else if (machine__normalized_is(machine, "x86")) {
> @@ -93,17 +95,31 @@ static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>  	return printed;
>  }
>  
> +static bool is64bitip(struct machine *machine, struct addr_location *al)
> +{
> +	const struct dso *dso = al->map ? map__dso(al->map) : NULL;
> +
> +	if (dso)
> +		return dso->is_64_bit;
> +
> +	return machine__is(machine, "x86_64") ||
> +		machine__normalized_is(machine, "arm64") ||
> +		machine__normalized_is(machine, "s390");
> +}
> +
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
> -				struct machine *machine, FILE *fp)
> +				struct machine *machine, FILE *fp,
> +				struct addr_location *al)
>  {
>  	csh cs_handle;
>  	cs_insn *insn;
>  	size_t count;
>  	size_t printed = 0;
>  	int ret;
> +	bool is64bit = is64bitip(machine, al);
>  
>  	/* TODO: Try to initiate capstone only once but need a proper place. */
> -	ret = capstone_init(machine, &cs_handle);
> +	ret = capstone_init(machine, &cs_handle, is64bit);
>  	if (ret < 0) {
>  		/* fallback */
>  		return sample__fprintf_insn_raw(sample, fp);
> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
> index 465bdcfcc2fd..6447dd41b543 100644
> --- a/tools/perf/util/print_insn.h
> +++ b/tools/perf/util/print_insn.h
> @@ -10,7 +10,7 @@ struct thread;
>  struct machine;
>  
>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
> -				struct machine *machine, FILE *fp);
> +				struct machine *machine, FILE *fp, struct addr_location *al);
>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
>  
>  #endif /* PERF_PRINT_INSN_H */

Re: [PATCH v4 1/2] perf, capstone: Support 32bit code under 64bit OS

[Thomas Richter]

On 3/22/24 11:21, Adrian Hunter wrote:
> On 8/03/24 19:06, Andi Kleen wrote:
>> Use the DSO to resolve whether an IP is 32bit or 64bit and use that to
>> configure capstone to the correct mode. This allows to correctly
>> disassemble 32bit code under a 64bit OS.
>>
>> % cat > loop.c
>> volatile int var;
>> int main(void)
>> {
>> 	int i;
>> 	for (i = 0; i < 100000; i++)
>> 		var++;
>> }
>> % gcc -m32 -o loop loop.c
>> % perf record -e cycles:u ./loop
>> % perf script -F +disasm
>>             loop   82665 1833176.618023:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>>             loop   82665 1833176.618029:          1 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>>             loop   82665 1833176.618031:          7 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>>             loop   82665 1833176.618034:         91 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>>             loop   82665 1833176.618036:       1242 cycles:u:          f7eed500 _start+0x0 (/usr/lib/ld-linux.so.2)             movl %esp, %eax
>>
>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> Comma in subject is slightly odd, otherwise:
> 
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> 
>>
>> ---
>>
>> v2: Factor out DSO lookup into separate function
>> v3: Pass down al
>> v4: Simplify is64bitip
>> ---
>>  tools/perf/builtin-script.c  |  9 +++++----
>>  tools/perf/util/print_insn.c | 24 ++++++++++++++++++++----
>>  tools/perf/util/print_insn.h |  2 +-
>>  3 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
>> index 37088cc0ff1b..0299b1ed8744 100644
>> --- a/tools/perf/builtin-script.c
>> +++ b/tools/perf/builtin-script.c
>> @@ -1517,7 +1517,8 @@ void script_fetch_insn(struct perf_sample *sample, struct thread *thread,
>>  static int perf_sample__fprintf_insn(struct perf_sample *sample,
>>  				     struct perf_event_attr *attr,
>>  				     struct thread *thread,
>> -				     struct machine *machine, FILE *fp)
>> +				     struct machine *machine, FILE *fp,
>> +				     struct addr_location *al)
>>  {
>>  	int printed = 0;
>>  
>> @@ -1531,7 +1532,7 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample,
>>  	}
>>  	if (PRINT_FIELD(DISASM) && sample->insn_len) {
>>  		printed += fprintf(fp, "\t\t");
>> -		printed += sample__fprintf_insn_asm(sample, thread, machine, fp);
>> +		printed += sample__fprintf_insn_asm(sample, thread, machine, fp, al);
>>  	}
>>  	if (PRINT_FIELD(BRSTACKINSN) || PRINT_FIELD(BRSTACKINSNLEN))
>>  		printed += perf_sample__fprintf_brstackinsn(sample, thread, attr, machine, fp);
>> @@ -1606,7 +1607,7 @@ static int perf_sample__fprintf_bts(struct perf_sample *sample,
>>  	if (print_srcline_last)
>>  		printed += map__fprintf_srcline(al->map, al->addr, "\n  ", fp);
>>  
>> -	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
>> +	printed += perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
>>  	printed += fprintf(fp, "\n");
>>  	if (PRINT_FIELD(SRCCODE)) {
>>  		int ret = map__fprintf_srccode(al->map, al->addr, stdout,
>> @@ -2259,7 +2260,7 @@ static void process_event(struct perf_script *script,
>>  
>>  	if (evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
>>  		perf_sample__fprintf_bpf_output(sample, fp);
>> -	perf_sample__fprintf_insn(sample, attr, thread, machine, fp);
>> +	perf_sample__fprintf_insn(sample, attr, thread, machine, fp, al);
>>  
>>  	if (PRINT_FIELD(PHYS_ADDR))
>>  		fprintf(fp, "%16" PRIx64, sample->phys_addr);
>> diff --git a/tools/perf/util/print_insn.c b/tools/perf/util/print_insn.c
>> index 459e0e93d7b1..8e4e3cffd677 100644
>> --- a/tools/perf/util/print_insn.c
>> +++ b/tools/perf/util/print_insn.c
>> @@ -12,6 +12,8 @@
>>  #include "machine.h"
>>  #include "thread.h"
>>  #include "print_insn.h"
>> +#include "map.h"
>> +#include "dso.h"
>>  
>>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>>  {
>> @@ -28,12 +30,12 @@ size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp)
>>  #ifdef HAVE_LIBCAPSTONE_SUPPORT
>>  #include <capstone/capstone.h>
>>  
>> -static int capstone_init(struct machine *machine, csh *cs_handle)
>> +static int capstone_init(struct machine *machine, csh *cs_handle, bool is64)
>>  {
>>  	cs_arch arch;
>>  	cs_mode mode;
>>  
>> -	if (machine__is(machine, "x86_64")) {
>> +	if (machine__is(machine, "x86_64") && is64) {
>>  		arch = CS_ARCH_X86;
>>  		mode = CS_MODE_64;
>>  	} else if (machine__normalized_is(machine, "x86")) {
>> @@ -93,17 +95,31 @@ static size_t print_insn_x86(struct perf_sample *sample, struct thread *thread,
>>  	return printed;
>>  }
>>  
>> +static bool is64bitip(struct machine *machine, struct addr_location *al)
>> +{
>> +	const struct dso *dso = al->map ? map__dso(al->map) : NULL;
>> +
>> +	if (dso)
>> +		return dso->is_64_bit;
>> +
>> +	return machine__is(machine, "x86_64") ||
>> +		machine__normalized_is(machine, "arm64") ||
>> +		machine__normalized_is(machine, "s390");
>> +}
>> +
>>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
>> -				struct machine *machine, FILE *fp)
>> +				struct machine *machine, FILE *fp,
>> +				struct addr_location *al)
>>  {
>>  	csh cs_handle;
>>  	cs_insn *insn;
>>  	size_t count;
>>  	size_t printed = 0;
>>  	int ret;
>> +	bool is64bit = is64bitip(machine, al);
>>  
>>  	/* TODO: Try to initiate capstone only once but need a proper place. */
>> -	ret = capstone_init(machine, &cs_handle);
>> +	ret = capstone_init(machine, &cs_handle, is64bit);
>>  	if (ret < 0) {
>>  		/* fallback */
>>  		return sample__fprintf_insn_raw(sample, fp);
>> diff --git a/tools/perf/util/print_insn.h b/tools/perf/util/print_insn.h
>> index 465bdcfcc2fd..6447dd41b543 100644
>> --- a/tools/perf/util/print_insn.h
>> +++ b/tools/perf/util/print_insn.h
>> @@ -10,7 +10,7 @@ struct thread;
>>  struct machine;
>>  
>>  size_t sample__fprintf_insn_asm(struct perf_sample *sample, struct thread *thread,
>> -				struct machine *machine, FILE *fp);
>> +				struct machine *machine, FILE *fp, struct addr_location *al);
>>  size_t sample__fprintf_insn_raw(struct perf_sample *sample, FILE *fp);
>>  
>>  #endif /* PERF_PRINT_INSN_H */
> 
> 

Acked-by: Thomas Richter <tmricht@linux.ibm.com>

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294