Re: [PATCH] perf stat: Fail on extra comma while parsing events

[Ian Rogers]

On Wed, May 20, 2020 at 12:41 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Ian reported that we allow to parse following:
>
>   $ perf stat -e ,cycles true
>
> which is wrong and we should fail, like we do with this fix:
>
>   $ perf stat -e ,cycles true
>   event syntax error: ',cycles'
>                         \___ parser error
>
> The reason is that we don't have rule for ',' in 'event'
> start condition and it's matched and accepted by default
> rule.
>
> Adding scanner debug support (that Ian already added for
> expr code), which was really useful for finding this. It's
> enabled together with bison debug via 'make PARSER_DEBUG=1'.
>
> Reported-by: Ian Rogers <irogers@xxxxxxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>

Acked-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks!
Ian

Ian

> ---
>  tools/perf/util/parse-events.c | 1 +
>  tools/perf/util/parse-events.l | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index e37a6a3e6217..d251c8a778b8 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -2014,6 +2014,7 @@ static int parse_events__scanner(const char *str, void *parse_state, int start_t
>
>  #ifdef PARSER_DEBUG
>         parse_events_debug = 1;
> +       parse_events_set_debug(1, scanner);
>  #endif
>         ret = parse_events_parse(parse_state, scanner);
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index c589fc42f058..394132254447 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -252,7 +252,9 @@ modifier_bp [rwx]{1,3}
>                         BEGIN(INITIAL);
>                         REWIND(0);
>                 }
> -
> +,              {
> +                       return ',';
> +               }
>  }
>
>  <array>{
> --
> 2.25.4
>