perf parse-events: Print all errors
authorIan Rogers <irogers@google.com>
Wed, 31 Jan 2024 13:49:40 +0000 (05:49 -0800)
committerNamhyung Kim <namhyung@kernel.org>
Fri, 2 Feb 2024 21:08:05 +0000 (13:08 -0800)
Prior to this patch the first and the last error encountered during
parsing are printed. To see other errors verbose needs
enabling. Unfortunately this can drop useful errors, in particular on
terms. This patch changes the errors so that instead of the first and
last all errors are recorded and printed, the underlying data
structure is changed to a list.

Before:
```
$ perf stat -e 'slots/edge=2/' true
event syntax error: 'slots/edge=2/'
                                \___ Bad event or PMU

Unable to find PMU or event on a PMU of 'slots'

Initial error:
event syntax error: 'slots/edge=2/'
                     \___ Cannot find PMU `slots'. Missing kernel support?
Run 'perf list' for a list of valid events

 Usage: perf stat [<options>] [<command>]

    -e, --event <event>   event selector. use 'perf list' to list available events
```

After:
```
$ perf stat -e 'slots/edge=2/' true
event syntax error: 'slots/edge=2/'
                     \___ Bad event or PMU

Unable to find PMU or event on a PMU of 'slots'

event syntax error: 'slots/edge=2/'
                                \___ value too big for format (edge), maximum is 1

event syntax error: 'slots/edge=2/'
                     \___ Cannot find PMU `slots'. Missing kernel support?
Run 'perf list' for a list of valid events

 Usage: perf stat [<options>] [<command>]

    -e, --event <event>   event selector. use 'perf list' to list available events
```

Signed-off-by: Ian Rogers <irogers@google.com>
Reviewed-by: James Clark <james.clark@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: tchen168@asu.edu
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Link: https://lore.kernel.org/r/20240131134940.593788-3-irogers@google.com
tools/perf/arch/x86/tests/hybrid.c
tools/perf/tests/expand-cgroup.c
tools/perf/tests/parse-events.c
tools/perf/util/parse-events.c
tools/perf/util/parse-events.h
tools/perf/util/parse-events.y

index 40f5d17..e221ea1 100644 (file)
@@ -259,11 +259,10 @@ static int test_event(const struct evlist_test *e)
        parse_events_error__init(&err);
        ret = parse_events(evlist, e->name, &err);
        if (ret) {
-               pr_debug("failed to parse event '%s', err %d, str '%s'\n",
-                        e->name, ret, err.str);
+               pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
                parse_events_error__print(&err, e->name);
                ret = TEST_FAIL;
-               if (strstr(err.str, "can't access trace events"))
+               if (parse_events_error__contains(&err, "can't access trace events"))
                        ret = TEST_SKIP;
        } else {
                ret = e->check(evlist);
index 9c1a1f1..31966ff 100644 (file)
@@ -127,8 +127,7 @@ static int expand_group_events(void)
        parse_events_error__init(&err);
        ret = parse_events(evlist, event_str, &err);
        if (ret < 0) {
-               pr_debug("failed to parse event '%s', err %d, str '%s'\n",
-                        event_str, ret, err.str);
+               pr_debug("failed to parse event '%s', err %d\n", event_str, ret);
                parse_events_error__print(&err, event_str);
                goto out;
        }
index fbdf710..feb5727 100644 (file)
@@ -2506,11 +2506,10 @@ static int test_event(const struct evlist_test *e)
        parse_events_error__init(&err);
        ret = parse_events(evlist, e->name, &err);
        if (ret) {
-               pr_debug("failed to parse event '%s', err %d, str '%s'\n",
-                        e->name, ret, err.str);
+               pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
                parse_events_error__print(&err, e->name);
                ret = TEST_FAIL;
-               if (err.str && strstr(err.str, "can't access trace events"))
+               if (parse_events_error__contains(&err, "can't access trace events"))
                        ret = TEST_SKIP;
        } else {
                ret = e->check(evlist);
@@ -2535,8 +2534,8 @@ static int test_event_fake_pmu(const char *str)
        ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err,
                             &perf_pmu__fake, /*warn_if_reordered=*/true);
        if (ret) {
-               pr_debug("failed to parse event '%s', err %d, str '%s'\n",
-                        str, ret, err.str);
+               pr_debug("failed to parse event '%s', err %d\n",
+                        str, ret);
                parse_events_error__print(&err, str);
        }
 
index 66eabce..6f8b0fa 100644 (file)
@@ -2181,50 +2181,53 @@ int parse_event(struct evlist *evlist, const char *str)
        return ret;
 }
 
+struct parse_events_error_entry {
+       /** @list: The list the error is part of. */
+       struct list_head list;
+       /** @idx: index in the parsed string */
+       int   idx;
+       /** @str: string to display at the index */
+       char *str;
+       /** @help: optional help string */
+       char *help;
+};
+
 void parse_events_error__init(struct parse_events_error *err)
 {
-       bzero(err, sizeof(*err));
+       INIT_LIST_HEAD(&err->list);
 }
 
 void parse_events_error__exit(struct parse_events_error *err)
 {
-       zfree(&err->str);
-       zfree(&err->help);
-       zfree(&err->first_str);
-       zfree(&err->first_help);
+       struct parse_events_error_entry *pos, *tmp;
+
+       list_for_each_entry_safe(pos, tmp, &err->list, list) {
+               zfree(&pos->str);
+               zfree(&pos->help);
+               list_del_init(&pos->list);
+               free(pos);
+       }
 }
 
 void parse_events_error__handle(struct parse_events_error *err, int idx,
                                char *str, char *help)
 {
+       struct parse_events_error_entry *entry;
+
        if (WARN(!str || !err, "WARNING: failed to provide error string or struct\n"))
                goto out_free;
-       switch (err->num_errors) {
-       case 0:
-               err->idx = idx;
-               err->str = str;
-               err->help = help;
-               break;
-       case 1:
-               err->first_idx = err->idx;
-               err->idx = idx;
-               err->first_str = err->str;
-               err->str = str;
-               err->first_help = err->help;
-               err->help = help;
-               break;
-       default:
-               pr_debug("Multiple errors dropping message: %s (%s)\n",
-                       err->str, err->help ?: "<no help>");
-               free(err->str);
-               err->str = str;
-               free(err->help);
-               err->help = help;
-               break;
+
+       entry = zalloc(sizeof(*entry));
+       if (!entry) {
+               pr_err("Failed to allocate memory for event parsing error: %s (%s)\n",
+                       str, help ?: "<no help>");
+               goto out_free;
        }
-       err->num_errors++;
+       entry->idx = idx;
+       entry->str = str;
+       entry->help = help;
+       list_add(&entry->list, &err->list);
        return;
-
 out_free:
        free(str);
        free(help);
@@ -2294,19 +2297,34 @@ static void __parse_events_error__print(int err_idx, const char *err_str,
        }
 }
 
-void parse_events_error__print(struct parse_events_error *err,
+void parse_events_error__print(const struct parse_events_error *err,
                               const char *event)
 {
-       if (!err->num_errors)
-               return;
+       struct parse_events_error_entry *pos;
+       bool first = true;
+
+       list_for_each_entry(pos, &err->list, list) {
+               if (!first)
+                       fputs("\n", stderr);
+               __parse_events_error__print(pos->idx, pos->str, pos->help, event);
+               first = false;
+       }
+}
 
-       __parse_events_error__print(err->idx, err->str, err->help, event);
+/*
+ * In the list of errors err, do any of the error strings (str) contain the
+ * given needle string?
+ */
+bool parse_events_error__contains(const struct parse_events_error *err,
+                                 const char *needle)
+{
+       struct parse_events_error_entry *pos;
 
-       if (err->num_errors > 1) {
-               fputs("\nInitial error:\n", stderr);
-               __parse_events_error__print(err->first_idx, err->first_str,
-                                       err->first_help, event);
+       list_for_each_entry(pos, &err->list, list) {
+               if (strstr(pos->str, needle) != NULL)
+                       return true;
        }
+       return false;
 }
 
 #undef MAX_WIDTH
index 63c0a36..809359e 100644 (file)
@@ -130,13 +130,8 @@ struct parse_events_term {
 };
 
 struct parse_events_error {
-       int   num_errors;       /* number of errors encountered */
-       int   idx;      /* index in the parsed string */
-       char *str;      /* string to display at the index */
-       char *help;     /* optional help string */
-       int   first_idx;/* as above, but for the first encountered error */
-       char *first_str;
-       char *first_help;
+       /** @list: The head of a list of errors. */
+       struct list_head list;
 };
 
 /* A wrapper around a list of terms for the sake of better type safety. */
@@ -247,9 +242,10 @@ void parse_events_error__init(struct parse_events_error *err);
 void parse_events_error__exit(struct parse_events_error *err);
 void parse_events_error__handle(struct parse_events_error *err, int idx,
                                char *str, char *help);
-void parse_events_error__print(struct parse_events_error *err,
+void parse_events_error__print(const struct parse_events_error *err,
                               const char *event);
-
+bool parse_events_error__contains(const struct parse_events_error *err,
+                                 const char *needle);
 #ifdef HAVE_LIBELF_SUPPORT
 /*
  * If the probe point starts with '%',
index de098ca..d70f5d8 100644 (file)
@@ -536,8 +536,6 @@ tracepoint_name opt_event_config
        list = alloc_list();
        if (!list)
                YYNOMEM;
-       if (error)
-               error->idx = @1.first_column;
 
        err = parse_events_add_tracepoint(list, &parse_state->idx, $1.sys, $1.event,
                                        error, $2, &@1);