Messages in this thread Patch in this message | | | Date | Tue, 6 Feb 2024 10:00:56 +0800 | From | Changbin Du <> | Subject | Re: [PATCH v5 3/5] perf: script: add field 'disasm' to display mnemonic instructions |
| |
On Mon, Feb 05, 2024 at 03:10:58PM +0200, Adrian Hunter wrote: > On 5/02/24 14:19, Changbin Du wrote: > > On Mon, Feb 05, 2024 at 11:23:21AM +0200, Adrian Hunter wrote: > >>> struct perf_script { > >>> @@ -190,6 +191,7 @@ struct output_option { > >>> {.str = "bpf-output", .field = PERF_OUTPUT_BPF_OUTPUT}, > >>> {.str = "callindent", .field = PERF_OUTPUT_CALLINDENT}, > >>> {.str = "insn", .field = PERF_OUTPUT_INSN}, > >>> + {.str = "disasm", .field = PERF_OUTPUT_DISASM}, > >>> {.str = "insnlen", .field = PERF_OUTPUT_INSNLEN}, > >>> {.str = "brstackinsn", .field = PERF_OUTPUT_BRSTACKINSN}, > >>> {.str = "brstackoff", .field = PERF_OUTPUT_BRSTACKOFF}, > >>> @@ -1515,6 +1517,10 @@ static int perf_sample__fprintf_insn(struct perf_sample *sample, > >>> printed += fprintf(fp, " insn: "); > >>> printed += sample__fprintf_insn_raw(sample, fp); > >>> } > >>> + if (PRINT_FIELD(DISASM) && sample->insn_len) { > >>> + printed += fprintf(fp, "\t\t"); > >> > >> This is good, except if both 'insn' and 'disasm' are used together. > >> It either: > >> a) without libcapstone, prints insn bytes twice > >> > >> Probably simpler to make 'disasm' without libcapstone > >> a fatal error explaining that perf needs to be built > >> with libcapstone support for 'disasm' to work. > >> > > Instead of fatal error, I print a warning message for this. Because > > perf_sample__fprintf_insn() cannot return negtive error number. > > It could be validated in advance, perhaps in parse_output_fields(). > I did as below:
--- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -3112,6 +3112,13 @@ static int parse_output_fields(const struct option *opt __maybe_unused, rc = -EINVAL; goto out; } +#ifndef HAVE_LIBCAPSTONE_SUPPORT + if (change != REMOVE && strcmp(tok, "disasm") == 0) { + fprintf(stderr, "Field \"disasm\" requires perf to be built with libcapstone support.\n"); + rc = -EINVAL; + goto out; + } +#endif
Then, $ sudo perf script -F +disasm Field "disasm" requires perf to be built with libcapstone support.
Usage: perf script [<options>] ... > > > >> b) with libcapstone, disassembly does not line up nicely > >> > > > > >
-- Cheers, Changbin Du
| |