| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Zj-Xlj-hIRAv_EZL@x1>
Date: Sat, 11 May 2024 13:06:46 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Thomas Richter <tmricht@...ux.ibm.com>,
Bhaskara Budiredla <bbudiredla@...vell.com>,
Bharat Bhushan <bbhushan2@...vell.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>,
James Clark <james.clark@....com>,
Ravi Bangoria <ravi.bangoria@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Will Deacon <will@...nel.org>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v3 1/2] perf pmus: Sort/merge/aggregate PMUs like
mrvl_ddr_pmu
On Sat, May 04, 2024 at 08:16:23PM -0700, Ian Rogers wrote:
> The mrvl_ddr_pmu is uncore and has a hexadecimal address suffix while
> the previous PMU sorting/merging code assumes uncore PMU names start
> with uncore_ and have a decimal suffix. Because of the previous
> assumption it isn't possible to wildcard the mrvl_ddr_pmu.
>
> Modify pmu_name_len_no_suffix but also remove the suffix number out
> argument, this is because we don't know if a suffix number of say 10
> is in hexadecimal or decimal. As the only use of the suffix number is
> in comparisons, it is safe there to compare the values as hexadecimal.
Removed these patches due to:
util/pmus.c: In function ‘pmu_name_cmp’:
util/pmus.c:67:19: error: ‘strncmp’ specified bound [18446744071562067968, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Werror=stringop-overread]
67 | int ret = strncmp(lhs_pmu_name, rhs_pmu_name,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
68 | lhs_pmu_name_len < rhs_pmu_name_len ? lhs_pmu_name_len : rhs_pmu_name_len);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [/tmp/perf-6.9.0-rc5/tools/build/Makefile.build:105: util/pmus.o] Error 1
Noticed while doing a 'make -C tools/perf build-test', the tarbz test.
acme@x1:~/git/perf-tools-next$ head /etc/os-release
NAME="Fedora Linux"
VERSION="39 (Workstation Edition)"
ID=fedora
VERSION_ID=39
VERSION_CODENAME=""
PLATFORM_ID="platform:f39"
PRETTY_NAME="Fedora Linux 39 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:39"
acme@x1:~/git/perf-tools-next$
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
> tools/perf/util/pmu.c | 4 ++--
> tools/perf/util/pmus.c | 53 ++++++++++++++++++++++--------------------
> tools/perf/util/pmus.h | 7 +++++-
> 3 files changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index b3b072feef02..32e5a73273d7 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1761,7 +1761,7 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
> {
> struct parse_events_term *term;
> int pmu_name_len = skip_duplicate_pmus
> - ? pmu_name_len_no_suffix(pmu->name, /*num=*/NULL)
> + ? pmu_name_len_no_suffix(pmu->name)
> : (int)strlen(pmu->name);
> int used = snprintf(buf, len, "%.*s/%s", pmu_name_len, pmu->name, alias->name);
>
> @@ -1805,7 +1805,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
>
> info.pmu_name = event->pmu_name ?: pmu->name;
> pmu_name_len = skip_duplicate_pmus
> - ? pmu_name_len_no_suffix(info.pmu_name, /*num=*/NULL)
> + ? pmu_name_len_no_suffix(info.pmu_name)
> : (int)strlen(info.pmu_name);
> info.alias = NULL;
> if (event->desc) {
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index b9b4c5eb5002..28cfa7adbfd7 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -40,31 +40,44 @@ static bool read_sysfs_all_pmus;
>
> static void pmu_read_sysfs(bool core_only);
>
> -int pmu_name_len_no_suffix(const char *str, unsigned long *num)
> +int pmu_name_len_no_suffix(const char *str)
> {
> int orig_len, len;
>
> orig_len = len = strlen(str);
>
> - /* Non-uncore PMUs have their full length, for example, i915. */
> - if (!strstarts(str, "uncore_"))
> - return len;
> -
> /*
> * Count trailing digits and '_', if '_{num}' suffix isn't present use
> * the full length.
> */
> - while (len > 0 && isdigit(str[len - 1]))
> + while (len > 0 && isxdigit(str[len - 1]))
> len--;
>
> - if (len > 0 && len != orig_len && str[len - 1] == '_') {
> - if (num)
> - *num = strtoul(&str[len], NULL, 10);
> + if (len > 0 && len != orig_len && str[len - 1] == '_')
> return len - 1;
> - }
> +
> return orig_len;
> }
>
> +int pmu_name_cmp(const char *lhs_pmu_name, const char *rhs_pmu_name)
> +{
> + unsigned long lhs_num = 0, rhs_num = 0;
> + int lhs_pmu_name_len = pmu_name_len_no_suffix(lhs_pmu_name);
> + int rhs_pmu_name_len = pmu_name_len_no_suffix(rhs_pmu_name);
> + int ret = strncmp(lhs_pmu_name, rhs_pmu_name,
> + lhs_pmu_name_len < rhs_pmu_name_len ? lhs_pmu_name_len : rhs_pmu_name_len);
> +
> + if (lhs_pmu_name_len != rhs_pmu_name_len || ret != 0 || lhs_pmu_name_len == 0)
> + return ret;
> +
> + if (lhs_pmu_name_len + 1 < (int)strlen(lhs_pmu_name))
> + lhs_num = strtoul(&lhs_pmu_name[lhs_pmu_name_len + 1], NULL, 16);
> + if (rhs_pmu_name_len + 1 < (int)strlen(rhs_pmu_name))
> + rhs_num = strtoul(&rhs_pmu_name[rhs_pmu_name_len + 1], NULL, 16);
> +
> + return lhs_num < rhs_num ? -1 : (lhs_num > rhs_num ? 1 : 0);
> +}
> +
> void perf_pmus__destroy(void)
> {
> struct perf_pmu *pmu, *tmp;
> @@ -167,20 +180,10 @@ static struct perf_pmu *perf_pmu__find2(int dirfd, const char *name)
> static int pmus_cmp(void *priv __maybe_unused,
> const struct list_head *lhs, const struct list_head *rhs)
> {
> - unsigned long lhs_num = 0, rhs_num = 0;
> struct perf_pmu *lhs_pmu = container_of(lhs, struct perf_pmu, list);
> struct perf_pmu *rhs_pmu = container_of(rhs, struct perf_pmu, list);
> - const char *lhs_pmu_name = lhs_pmu->name ?: "";
> - const char *rhs_pmu_name = rhs_pmu->name ?: "";
> - int lhs_pmu_name_len = pmu_name_len_no_suffix(lhs_pmu_name, &lhs_num);
> - int rhs_pmu_name_len = pmu_name_len_no_suffix(rhs_pmu_name, &rhs_num);
> - int ret = strncmp(lhs_pmu_name, rhs_pmu_name,
> - lhs_pmu_name_len < rhs_pmu_name_len ? lhs_pmu_name_len : rhs_pmu_name_len);
>
> - if (lhs_pmu_name_len != rhs_pmu_name_len || ret != 0 || lhs_pmu_name_len == 0)
> - return ret;
> -
> - return lhs_num < rhs_num ? -1 : (lhs_num > rhs_num ? 1 : 0);
> + return pmu_name_cmp(lhs_pmu->name ?: "", rhs_pmu->name ?: "");
> }
>
> /* Add all pmus in sysfs to pmu list: */
> @@ -300,11 +303,11 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> pmu_read_sysfs(/*core_only=*/false);
> pmu = list_prepare_entry(pmu, &core_pmus, list);
> } else
> - last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", NULL);
> + last_pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "");
>
> if (use_core_pmus) {
> list_for_each_entry_continue(pmu, &core_pmus, list) {
> - int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", /*num=*/NULL);
> + int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "");
>
> if (last_pmu_name_len == pmu_name_len &&
> !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len))
> @@ -316,7 +319,7 @@ static struct perf_pmu *perf_pmus__scan_skip_duplicates(struct perf_pmu *pmu)
> pmu = list_prepare_entry(pmu, &other_pmus, list);
> }
> list_for_each_entry_continue(pmu, &other_pmus, list) {
> - int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "", /*num=*/NULL);
> + int pmu_name_len = pmu_name_len_no_suffix(pmu->name ?: "");
>
> if (last_pmu_name_len == pmu_name_len &&
> !strncmp(last_pmu_name, pmu->name ?: "", pmu_name_len))
> @@ -566,7 +569,7 @@ void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, voi
> .long_string = STRBUF_INIT,
> .num_formats = 0,
> };
> - int len = pmu_name_len_no_suffix(pmu->name, /*num=*/NULL);
> + int len = pmu_name_len_no_suffix(pmu->name);
> const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
>
> if (!pmu->is_core)
> diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> index 9d4ded80b8e9..69d64fbd4259 100644
> --- a/tools/perf/util/pmus.h
> +++ b/tools/perf/util/pmus.h
> @@ -2,10 +2,15 @@
> #ifndef __PMUS_H
> #define __PMUS_H
>
> +#include <stdbool.h>
> +#include <linux/list.h>
> +
> struct perf_pmu;
> struct print_callbacks;
>
> -int pmu_name_len_no_suffix(const char *str, unsigned long *num);
> +int pmu_name_len_no_suffix(const char *str);
> +/* Exposed for testing only. */
> +int pmu_name_cmp(const char *lhs_pmu_name, const char *rhs_pmu_name);
>
> void perf_pmus__destroy(void);
>
> --
> 2.45.0.rc1.225.g2a3ae87e7f-goog
Powered by blists - more mailing lists