From nobody Fri Jan 31 12:08:56 2025 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5A32DA29 for ; Fri, 24 Jan 2025 04:32:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737693168; cv=none; b=DtLlzLIsqvVhAorMVU28u3HOvSpDbVFuApk6EHUtikOgOFudc3bXbhcKx9zZ2gKaajP4y3uO5/dyC9QefbtdNPLaytaP2b3wVsdjJGjoM38l/Inz/EMhJJIMNrjDmCPkjJ2APgjMz82aS3EQKUIIWChxg0PgWzN/B3PuxoDZemw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737693168; c=relaxed/simple; bh=WsWfXBdAsZmpFLOg4Z45/1CHJDwZJcWImRjYcC6uJUc=; h=Date:Message-Id:Mime-Version:Subject:From:To:Content-Type; b=UlQ4gJm95TaQdcVjpHEV6AvmbA1DtmvzD83eSJO4VAogcoQOj/HLGtBi1vH0VW3/ahOZVKNyxyskl4BpK9YunOX/HUbkrWIUK4ujz6ExdS0754LtkvG4A8S6ItvZW39rntdBumwFp1ro5GIOfrzbrsYxkmY8X5cRAxSZQfN7Eng= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=xBGYt6JV; arc=none smtp.client-ip=209.85.219.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="xBGYt6JV" Received: by mail-yb1-f201.google.com with SMTP id 3f1490d57ef6-e5505f613e5so7180584276.0 for ; Thu, 23 Jan 2025 20:32:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1737693165; x=1738297965; darn=vger.kernel.org; h=to:from:subject:mime-version:message-id:date:from:to:cc:subject :date:message-id:reply-to; bh=eyhNieawePCYid/HCGGpD8Pj8YkzfwMDeYbku9p4Drk=; b=xBGYt6JVapxKefmIoU9g+aztErgLluFZK5rIoDpRIZLQ4mabkXEgqmPiS9GdFZlRWQ GumPIhiKB0GneLMa7h1WGhoZP7wA9fne2xRsa4lZS+NCh3tOskj3qGgQctc+ok4xi+Hj nD5QKAPrkgjLiBScmOeDtY6r0Im2I/wP/tbzkG4mScamJjSuUtimjtmoA3GOjiAHDVbm 9t1l02t3k13pXQPt7UHMuC1TMoCDreSWS4xFE2RImL3JA8FELNzqVuMH3xqq01Gti1cs apmcIy5dbQmWcth+eaFlsGIJLlD8AkFEwolDHkcHEqLePDg6YVYDWZGonaaYfqZDgpHn dh7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737693165; x=1738297965; h=to:from:subject:mime-version:message-id:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=eyhNieawePCYid/HCGGpD8Pj8YkzfwMDeYbku9p4Drk=; b=gP95LvRNVuqeQRltmkDi3KM7nG7SHAUH9Xi3ksxK5MtoguYKaRei5WLOpB0Ui9XBmC TNBrTnjjocQ3Z3xaI8lITwzuylJPgvmB6pXb6vBBu2g/MJYH8wjrIHqqd2dyeucMwaAb cvDoGGSNCHgCGbDMweS+HDrK6l+gzTDEOEdm6nL4GSVDe3w28cbg2zF5kHP8e3ySWfjg vmuE03xh66Hk+bgHratECEQqExXM7WpwIDeNo4zxKqUsTlpXYcHy8svS7BM21pSfSHwd YwFDBrtFLLFXna5UxxvkOoj1KWDtOYQaNoEZjNdPi1Sy1i5Awf8aCGOHKjad2mhfYwhY FO4g== X-Forwarded-Encrypted: i=1; AJvYcCVxZSUzAuykLSGhlmW8Gv3fXoDvmgNoz8yVMjyOy1Q49o/Z9HSBgh8//Ed6RYPQedg3a3Q2CZEFzqLQ3cw=@vger.kernel.org X-Gm-Message-State: AOJu0Yzo6Tx2mKayVPwIqxsiAcDvIIBOvs4ytFSh6tAxOEm6kD3GNLxF ePqxeZDiyOjkEWOg21RSsK6eJBnolILNE2Mtqk6PDItcrwWBkGLSuSaTswwKfvFNH14Yq9AWH8W DN4f+aw== X-Google-Smtp-Source: AGHT+IEkdH4HLSLsrk9Ouph4XIXNIr+Pd5puR+bmY9pVIeGpveIWKHIbaQAgzRYKWmJN9MYbP/0qg37sWHMS X-Received: from irogers.svl.corp.google.com ([2620:15c:2c5:11:37b6:cf70:1dbf:669c]) (user=irogers job=sendgmr) by 2002:a05:690c:6488:b0:6ee:fbef:239 with SMTP id 00721157ae682-6f748e02ae3mr201377b3.5.1737693165329; Thu, 23 Jan 2025 20:32:45 -0800 (PST) Date: Thu, 23 Jan 2025 20:32:41 -0800 Message-Id: <20250124043242.1174490-1-irogers@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-Mailer: git-send-email 2.48.1.262.g85cc9f2d1e-goog Subject: [PATCH v1] perf annotate: Use an array for the disassembler preference From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , Kan Liang , Justin Stitt , Athira Rajeev , Andi Kleen , Kajol Jain , Li Huafei , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Prior to this change a string was used which could cause issues with an unrecognized disassembler in symbol__disassembler. Change to initializing an array of perf_disassembler enum values. If a value already exists then adding it a second time is ignored to avoid array out of bounds problems present in the previous code, it also allows a statically sized array and removes memory allocation needs. Errors in the disassembler string are reported when the config is parsed during perf annotate or perf top start up. If the array is uninitialized after processing the config file the default llvm, capstone then objdump values are added but without a need to parse a string. Fixes: a6e8a58de629 ("perf disasm: Allow configuring what disassemblers to = use") Closes: https://lore.kernel.org/lkml/CAP-5=3DfUdfCyxmEiTpzS2uumUp3-SyQOseX2= xZo81-dQtWXj6vA@mail.gmail.com/ Signed-off-by: Ian Rogers --- tools/perf/util/annotate.c | 76 +++++++++++++++++++++++++++++++--- tools/perf/util/annotate.h | 14 +++++-- tools/perf/util/disasm.c | 83 +++++++------------------------------- 3 files changed, 96 insertions(+), 77 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 0d2ea22bd9e4..31bb326b07a6 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -2100,6 +2100,57 @@ int symbol__annotate2(struct map_symbol *ms, struct = evsel *evsel, return 0; } =20 +const char * const perf_disassembler__strs[] =3D { + [PERF_DISASM_UNKNOWN] =3D "unknown", + [PERF_DISASM_LLVM] =3D "llvm", + [PERF_DISASM_CAPSTONE] =3D "capstone", + [PERF_DISASM_OBJDUMP] =3D "objdump", +}; + + +static void annotation_options__add_disassembler(struct annotation_options= *options, + enum perf_disassembler dis) +{ + for (u8 i =3D 0; i < ARRAY_SIZE(options->disassemblers); i++) { + if (options->disassemblers[i] =3D=3D dis) { + /* Disassembler is already present then don't add again. */ + return; + } + if (options->disassemblers[i] =3D=3D PERF_DISASM_UNKNOWN) { + /* Found a free slot. */ + options->disassemblers[i] =3D dis; + return; + } + } + pr_err("Failed to add disassembler %d\n", dis); +} + +static int annotation_options__add_disassemblers_str(struct annotation_opt= ions *options, + const char *str) +{ + while (str && *str !=3D '\0') { + const char *comma =3D strchr(str, ','); + int len =3D comma ? comma - str : (int)strlen(str); + bool match =3D false; + + for (u8 i =3D 0; i < ARRAY_SIZE(perf_disassembler__strs); i++) { + const char *dis_str =3D perf_disassembler__strs[i]; + + if (len =3D=3D (int)strlen(dis_str) && !strncmp(str, dis_str, len)) { + annotation_options__add_disassembler(options, i); + match =3D true; + break; + } + } + if (!match) { + pr_err("Invalid disassembler '%.*s'\n", len, str); + return -1; + } + str =3D comma ? comma + 1 : NULL; + } + return 0; +} + static int annotation__config(const char *var, const char *value, void *da= ta) { struct annotation_options *opt =3D data; @@ -2115,11 +2166,10 @@ static int annotation__config(const char *var, cons= t char *value, void *data) else if (opt->offset_level < ANNOTATION__MIN_OFFSET_LEVEL) opt->offset_level =3D ANNOTATION__MIN_OFFSET_LEVEL; } else if (!strcmp(var, "annotate.disassemblers")) { - opt->disassemblers_str =3D strdup(value); - if (!opt->disassemblers_str) { - pr_err("Not enough memory for annotate.disassemblers\n"); - return -1; - } + int err =3D annotation_options__add_disassemblers_str(opt, value); + + if (err) + return err; } else if (!strcmp(var, "annotate.hide_src_code")) { opt->hide_src_code =3D perf_config_bool("hide_src_code", value); } else if (!strcmp(var, "annotate.jump_arrows")) { @@ -2185,9 +2235,25 @@ void annotation_options__exit(void) zfree(&annotate_opts.objdump_path); } =20 +static void annotation_options__default_init_disassemblers(struct annotati= on_options *options) +{ + if (options->disassemblers[0] !=3D PERF_DISASM_UNKNOWN) { + /* Already initialized. */ + return; + } +#ifdef HAVE_LIBLLVM_SUPPORT + annotation_options__add_disassembler(options, PERF_DISASM_LLVM); +#endif +#ifdef HAVE_LIBCAPSTONE_SUPPORT + annotation_options__add_disassembler(options, PERF_DISASM_CAPSTONE); +#endif + annotation_options__add_disassembler(options, PERF_DISASM_OBJDUMP); +} + void annotation_config__init(void) { perf_config(annotation__config, &annotate_opts); + annotation_options__default_init_disassemblers(&annotate_opts); } =20 static unsigned int parse_percent_type(char *str1, char *str2) diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 0ba5846dad4d..7ae89425b54c 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -34,8 +34,13 @@ struct annotated_data_type; #define ANNOTATION__BR_CNTR_WIDTH 30 #define ANNOTATION_DUMMY_LEN 256 =20 -// llvm, capstone, objdump -#define MAX_DISASSEMBLERS 3 +enum perf_disassembler { + PERF_DISASM_UNKNOWN =3D 0, + PERF_DISASM_LLVM, + PERF_DISASM_CAPSTONE, + PERF_DISASM_OBJDUMP, +}; +#define MAX_DISASSEMBLERS (PERF_DISASM_OBJDUMP + 1) =20 struct annotation_options { bool hide_src_code, @@ -53,13 +58,12 @@ struct annotation_options { full_addr; u8 offset_level; u8 nr_disassemblers; + u8 disassemblers[MAX_DISASSEMBLERS]; int min_pcnt; int max_lines; int context; char *objdump_path; char *disassembler_style; - const char *disassemblers_str; - const char *disassemblers[MAX_DISASSEMBLERS]; const char *prefix; const char *prefix_strip; unsigned int percent_type; @@ -134,6 +138,8 @@ struct disasm_line { struct annotation_line al; }; =20 +extern const char * const perf_disassembler__strs[]; + void annotation_line__add(struct annotation_line *al, struct list_head *he= ad); =20 static inline double annotation_data__percent(struct annotation_data *data, diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c index b7de4d9fd004..50c5c206b70e 100644 --- a/tools/perf/util/disasm.c +++ b/tools/perf/util/disasm.c @@ -2216,56 +2216,6 @@ static int symbol__disassemble_objdump(const char *f= ilename, struct symbol *sym, return err; } =20 -static int annotation_options__init_disassemblers(struct annotation_option= s *options) -{ - char *disassembler; - - if (options->disassemblers_str =3D=3D NULL) { - const char *default_disassemblers_str =3D -#ifdef HAVE_LIBLLVM_SUPPORT - "llvm," -#endif -#ifdef HAVE_LIBCAPSTONE_SUPPORT - "capstone," -#endif - "objdump"; - - options->disassemblers_str =3D strdup(default_disassemblers_str); - if (!options->disassemblers_str) - goto out_enomem; - } - - disassembler =3D strdup(options->disassemblers_str); - if (disassembler =3D=3D NULL) - goto out_enomem; - - while (1) { - char *comma =3D strchr(disassembler, ','); - - if (comma !=3D NULL) - *comma =3D '\0'; - - options->disassemblers[options->nr_disassemblers++] =3D strim(disassembl= er); - - if (comma =3D=3D NULL) - break; - - disassembler =3D comma + 1; - - if (options->nr_disassemblers >=3D MAX_DISASSEMBLERS) { - pr_debug("annotate.disassemblers can have at most %d entries, ignoring = \"%s\"\n", - MAX_DISASSEMBLERS, disassembler); - break; - } - } - - return 0; - -out_enomem: - pr_err("Not enough memory for annotate.disassemblers\n"); - return -1; -} - int symbol__disassemble(struct symbol *sym, struct annotate_args *args) { struct annotation_options *options =3D args->options; @@ -2274,7 +2224,6 @@ int symbol__disassemble(struct symbol *sym, struct an= notate_args *args) char symfs_filename[PATH_MAX]; bool delete_extract =3D false; struct kcore_extract kce; - const char *disassembler; bool decomp =3D false; int err =3D dso__disassemble_filename(dso, symfs_filename, sizeof(symfs_f= ilename)); =20 @@ -2334,28 +2283,26 @@ int symbol__disassemble(struct symbol *sym, struct = annotate_args *args) } } =20 - err =3D annotation_options__init_disassemblers(options); - if (err) - goto out_remove_tmp; - err =3D -1; + for (u8 i =3D 0; i < ARRAY_SIZE(options->disassemblers) && err !=3D 0; i+= +) { + enum perf_disassembler dis =3D options->disassemblers[i]; =20 - for (int i =3D 0; i < options->nr_disassemblers && err !=3D 0; ++i) { - disassembler =3D options->disassemblers[i]; - - if (!strcmp(disassembler, "llvm")) + switch (dis) { + case PERF_DISASM_LLVM: err =3D symbol__disassemble_llvm(symfs_filename, sym, args); - else if (!strcmp(disassembler, "capstone")) + break; + case PERF_DISASM_CAPSTONE: err =3D symbol__disassemble_capstone(symfs_filename, sym, args); - else if (!strcmp(disassembler, "objdump")) + break; + case PERF_DISASM_OBJDUMP: err =3D symbol__disassemble_objdump(symfs_filename, sym, args); - else - pr_debug("Unknown disassembler %s, skipping...\n", disassembler); - } - - if (err =3D=3D 0) { - pr_debug("Disassembled with %s\nannotate.disassemblers=3D%s\n", - disassembler, options->disassemblers_str); + break; + case PERF_DISASM_UNKNOWN: /* End of disassemblers. */ + default: + goto out_remove_tmp; + } + if (err =3D=3D 0) + pr_debug("Disassembled with %s\n", perf_disassembler__strs[dis]); } out_remove_tmp: if (decomp) --=20 2.48.1.262.g85cc9f2d1e-goog