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

[Jiri Olsa <jolsa@redhat.com>]

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 '-'; }