Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200520074050.156988-1-jolsa@kernel.org>
In-Reply-To: <20200520074050.156988-1-jolsa@kernel.org>
From:   Ian Rogers <irogers@google.com>
Date:   Wed, 20 May 2020 00:51:24 -0700
Message-ID: <CAP-5=fWaqofns-K1_q=0iAZOF1D0CVDEyE6v6r+3L=Sd9Mto6w@mail.gmail.com>
Subject: Re: [PATCH] perf stat: Fail on extra comma while parsing events
To:     Jiri Olsa <jolsa@kernel.org>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Michael Petlan <mpetlan@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, May 20, 2020 at 12:41 AM Jiri Olsa <jolsa@kernel.org> 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@google.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

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
>