linux-kernel - Re: [PATCH v2 06/11] perf expr: parse numbers as doubles

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fUWtQBBNqyr=xhKUO+jHxq=04SrBYD2Yh6QiH55BvtEWw@mail.gmail.com>
Date:   Mon, 27 Apr 2020 11:03:13 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     kajoljain <kjain@...ux.ibm.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Haiyan Song <haiyanx.song@...el.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Song Liu <songliubraving@...com>,
        Ravi Bangoria <ravi.bangoria@...ux.ibm.com>,
        John Garry <john.garry@...wei.com>,
        Leo Yan <leo.yan@...aro.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Paul Clarke <pc@...ibm.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-perf-users <linux-perf-users@...r.kernel.org>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v2 06/11] perf expr: parse numbers as doubles

On Mon, Apr 27, 2020 at 4:12 AM kajoljain <kjain@...ux.ibm.com> wrote:
>
> On 4/23/20 3:34 AM, Ian Rogers wrote:
> > This is expected in expr.y and metrics use floating point values such as
> > x86 broadwell IFetch_Line_Utilization.
> >
> > Fixes: 26226a97724d (perf expr: Move expr lexer to flex)
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> >  tools/perf/util/expr.l | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
> > index 95bcf3629edf..0efda2ce2766 100644
> > --- a/tools/perf/util/expr.l
> > +++ b/tools/perf/util/expr.l
> > @@ -10,12 +10,12 @@
> >  char *expr_get_text(yyscan_t yyscanner);
> >  YYSTYPE *expr_get_lval(yyscan_t yyscanner);
> >
> > -static int __value(YYSTYPE *yylval, char *str, int base, int token)
> > +static double __value(YYSTYPE *yylval, char *str, int token)
> >  {
> > -     u64 num;
> > +     double num;
> >
> >       errno = 0;
> > -     num = strtoull(str, NULL, base);
> > +     num = strtod(str, NULL);
> >       if (errno)
> >               return EXPR_ERROR;
> >
> > @@ -23,12 +23,12 @@ static int __value(YYSTYPE *yylval, char *str, int base, int token)
> >       return token;
> >  }
> >
> > -static int value(yyscan_t scanner, int base)
> > +static int value(yyscan_t scanner)
> >  {
> >       YYSTYPE *yylval = expr_get_lval(scanner);
> >       char *text = expr_get_text(scanner);
> >
> > -     return __value(yylval, text, base, NUMBER);
> > +     return __value(yylval, text, NUMBER);
> >  }
> >
> >  /*
> > @@ -68,7 +68,7 @@ static int str(yyscan_t scanner, int token)
> >  }
> >  %}
> >
> > -number               [0-9]+
> > +number               [0-9]*\.?[0-9]+
> >
>
> Hi Ian,
>     In this patch I saw the parsing of expression with '+number         [0-9]*\.?[0-9]+'
> Could you please explain why '?' is introduced here, so that I can be sure that this is
> not conflicting with my change to add '?'

Hi Kajol,
the '?' here is part of the regular expression. Basically it is saying
that a number is a possible set of integers possibly followed by a '.'
and then 1 or more integers. The expression comes from having seen
Intel's metrics are of the form '.1234' in some of their topdown
spreadsheets, so we need to be able to handle cases like '1.2', '123'
and '.1234'. Having looked at your patch I don't believe it
interferes.

> In this patch : https://lkml.org/lkml/2020/4/1/1427
> I have used '?' symbol as part of metric expression in order to replace '?' with runtime
> parameter.

Interesting, I'll follow up with comments on that e-mail.

Thanks!
Ian

> Thanks,
> Kajol Jain
>
>
> >  sch          [-,=]
> >  spec         \\{sch}
> > @@ -92,7 +92,7 @@ min         { return MIN; }
> >  if           { return IF; }
> >  else         { return ELSE; }
> >  #smt_on              { return SMT_ON; }
> > -{number}     { return value(yyscanner, 10); }
> > +{number}     { return value(yyscanner); }
> >  {symbol}     { return str(yyscanner, ID); }
> >  "|"          { return '|'; }
> >  "^"          { return '^'; }
> >