Re: [PATCH 2/2] perf, tools: Don't force MetricExprs to lower case

Jiri Olsa
2017-10-12T15:13:39Z

On Mon, Oct 09, 2017 at 04:41:55PM +0200, Jiri Olsa wrote:
> On Mon, Oct 09, 2017 at 11:09:44AM -0300, Arnaldo Carvalho de Melo wrote:
>
> SNIP
>
> > [root@jouet bpf]# cat sys_read.c
> > #define SEC(NAME) __attribute__((section(NAME), used))
> > SEC("func=sys_read")
> > int bpf_func__sys_read(void *ctx)
> > {
> > return 1;
> > }
> > char _license[] SEC("license") = "GPL";
> > int _version SEC("version") = LINUX_VERSION_CODE;
> > [root@jouet bpf]# perf trace --no-syscalls -e sys_read.c/max-stack=5/ sleep 1
> > bpf: builtin compilation failed: -95, try external compiler
> > 0.000 perf_bpf_probe:func:(ffffffffb7263190))
> > sys_read ([kernel.kallsyms])
> > entry_SYSCALL_64_fastpath ([kernel.kallsyms])
> > __read (/usr/lib64/ld-2.25.so)
> > _dl_map_object (/usr/lib64/ld-2.25.so)
> > [root@jouet bpf]# perf trace --no-syscalls -e sys_read.c sleep 1
> > bpf: builtin compilation failed: -95, try external compiler
> > 0.000 perf_bpf_probe:func:(ffffffffb7263190))
> > [root@jouet bpf]#
>
> is this ok?
>
> >
> > [root@jouet bpf]# perf stat -e UOPS_EXECUTED.CORE sleep 1
> >
> > Performance counter stats for 'sleep 1':
> >
> > 1,205,347 UOPS_EXECUTED.CORE
> >
> > 1.001694225 seconds time elapsed
> >
> > But I noticed this problem:
> >
> > [root@jouet bpf]# perf stat -e cpu/uops_executed.core/,uops_executed.core,cpu/UOPS_EXECUTED.CORE sleep 1
> > event syntax error: '..d=48;5;232;38;5;3:or=48;5;232;38;5;9:mi=01;05;37;41:su=48;5;196;38;5;15:sg=48;5;11;38;5;16:ca=48;5;196;38;5;226:IY�'
> > \___ parser error
> > Run 'perf list' for a list of valid events
> >
>
> mama mia, this looks like unhandled case of error printing.. I'll check

I think I found the issue, could you please check following patch?

and wrt Andi's concern about wrong error message,
now if you make typo in your bpf file it says:

[jolsa@krava perf]$ ./perf stat -e krava.c// sleep 1
event syntax error: 'krava.c//'
\___ Cannot find PMU `krava.c'. Missing kernel support?
Run 'perf list' for a list of valid events
...

looks sane enough.. ;-)

thanks,
jirka

---
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index ea2426daf7e8..241396cd059d 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -8,6 +8,9 @@

%{
#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
#include "../perf.h"
#include "parse-events.h"
#include "parse-events-bison.h"
@@ -53,9 +56,8 @@ static int str(yyscan_t scanner, int token)
return token;
}

-static bool isbpf(yyscan_t scanner)
+static bool isbpf_suffix(char *text)
{
- char *text = parse_events_get_text(scanner);
int len = strlen(text);

if (len < 2)
@@ -68,6 +70,17 @@ static bool isbpf(yyscan_t scanner)
return false;
}

+static bool isbpf(yyscan_t scanner)
+{
+ char *text = parse_events_get_text(scanner);
+ struct stat st;
+
+ if (!isbpf_suffix(text))
+ return false;
+
+ return stat(text, &st) == 0;
+}
+
/*
* This function is called when the parser gets two kind of input:
*
@@ -141,6 +154,10 @@ do { \
yycolumn += yyleng; \
} while (0);

+#define USER_REJECT \
+ yycolumn -= yyleng; \
+ REJECT
+
%}

%x mem
@@ -323,8 +340,8 @@ r{num_raw_hex} { return raw(yyscanner); }
{num_hex} { return value(yyscanner, 16); }

{modifier_event} { return str(yyscanner, PE_MODIFIER_EVENT); }
-{bpf_object} { if (!isbpf(yyscanner)) REJECT; return str(yyscanner, PE_BPF_OBJECT); }
-{bpf_source} { if (!isbpf(yyscanner)) REJECT; return str(yyscanner, PE_BPF_SOURCE); }
+{bpf_object} { if (!isbpf(yyscanner)) USER_REJECT; return str(yyscanner, PE_BPF_OBJECT); }
+{bpf_source} { if (!isbpf(yyscanner)) USER_REJECT; return str(yyscanner, PE_BPF_SOURCE); }
{name} { return pmu_str_check(yyscanner); }
"/" { BEGIN(config); return '/'; }
- { return '-'; }