Re: [PATCH V3 0/1] perf tools: Allow config terms with breakpoints

[Adrian Hunter <adrian.hunter@intel.com>]

On 6/06/23 22:37, Adrian Hunter wrote:
> On 6/06/23 22:22, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Jun 06, 2023 at 03:15:59PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Tue, Jun 06, 2023 at 08:00:33AM +0300, Adrian Hunter escreveu:
>>>> On 25/05/23 11:29, Adrian Hunter wrote:
>>>>> Hi
>>>>>
>>>>> Here is a patch (V3) to the event parser for breakpoint events.
>>>>> I am not that familiar with flex / bison, but it seemed to
>>>>> need trailing context to stop the mem event colon and slash
>>>>> delimiters from getting mixed up with delimiters for config
>>>>> terms or event modifiers.  Please look closely at that.
>>>>>
>>>>>
>>>>> Changes in V3:
>>>>>
>>>>>       Add Ian's Reviewed-by
>>>>>       Re-base
>>>>
>>>> Still applies.  Any more comments?
>>>
>>> Tried it now, twice, once after removing the O= build dir:
>>>
>>>   CC      /tmp/build/perf-tools-next/tests/event-times.o
>>>   CC      /tmp/build/perf-tools-next/tests/expr.o
>>>   BISON   /tmp/build/perf-tools-next/util/parse-events-bison.c
>>> util/parse-events.y:508.24-34: warning: unused value: $3 [-Wother]
>>>   508 | PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config
>>>       |                        ^~~~~~~~~~~
>>> util/parse-events.y:508.45-55: warning: unused value: $5 [-Wother]
>>>   508 | PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config
>>>       |                                             ^~~~~~~~~~~
>>> util/parse-events.y:526.24-34: warning: unused value: $3 [-Wother]
>>>   526 | PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE opt_event_config
>>>       |                        ^~~~~~~~~~~
>>> util/parse-events.y:543.24-34: warning: unused value: $3 [-Wother]
>>>   543 | PE_PREFIX_MEM PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config
>>>       |                        ^~~~~~~~~~~
>>>   CC      /tmp/build/perf-tools-next/tests/backward-ring-buffer.o
>>
>> But it doesn't _break_ the build, just warns thiis when generating the
>> .c file, the next build it notices it is already generated, no warnings.
> 
> The build script I was using has a flaw.  It doesn't show warnings
> unless the build fails, assuming warnings are errors.  But these
> are not.
> 
> The first hack that came to mind, and seems to work, is:
> 
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index bbfb8110947c..dd36be3832b6 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -510,6 +510,8 @@ PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event
>  	struct list_head *list;
>  	int err;
>  
> +	$3 = $3;
> +	$5 = $5;
>  	list = alloc_list();
>  	ABORT_ON(!list);
>  	err = parse_events_add_breakpoint(_parse_state, list,
> @@ -528,6 +530,7 @@ PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE opt_event_config
>  	struct list_head *list;
>  	int err;
>  
> +	$3 = $3;
>  	list = alloc_list();
>  	ABORT_ON(!list);
>  	err = parse_events_add_breakpoint(_parse_state, list,
> @@ -545,6 +548,7 @@ PE_PREFIX_MEM PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config
>  	struct list_head *list;
>  	int err;
>  
> +	$3 = $3;
>  	list = alloc_list();
>  	ABORT_ON(!list);
>  	err = parse_events_add_breakpoint(_parse_state, list,
> 
> 

Maybe this is cleaner:


From: Adrian Hunter <adrian.hunter@intel.com>
Date: Wed, 7 Jun 2023 08:12:29 +0300
Subject: [PATCH] perf tools: Suppress bison unused value warnings

Patch "perf tools: Allow config terms with breakpoints" introduced
parse tokens for colons and slashes within breakpoint parsing to
prevent mix up with colons and slashes related to config terms.

The token values are not needed but introduce bison "unused value"
warnings.

Suppress those warnings.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/parse-events.h | 4 ++++
 tools/perf/util/parse-events.y | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index b0eb95f93e9c..5fdc1f33f57e 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -228,6 +228,10 @@ void parse_events_error__handle(struct parse_events_error *err, int idx,
 void parse_events_error__print(struct parse_events_error *err,
 			       const char *event);
 
+static inline void parse_events_unused_value(const void *x __maybe_unused)
+{
+}
+
 #ifdef HAVE_LIBELF_SUPPORT
 /*
  * If the probe point starts with '%',
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index bbfb8110947c..0c3d086cc22a 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -510,6 +510,9 @@ PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event
 	struct list_head *list;
 	int err;
 
+	parse_events_unused_value(&$3);
+	parse_events_unused_value(&$5);
+
 	list = alloc_list();
 	ABORT_ON(!list);
 	err = parse_events_add_breakpoint(_parse_state, list,
@@ -528,6 +531,8 @@ PE_PREFIX_MEM PE_VALUE PE_BP_SLASH PE_VALUE opt_event_config
 	struct list_head *list;
 	int err;
 
+	parse_events_unused_value(&$3);
+
 	list = alloc_list();
 	ABORT_ON(!list);
 	err = parse_events_add_breakpoint(_parse_state, list,
@@ -545,6 +550,8 @@ PE_PREFIX_MEM PE_VALUE PE_BP_COLON PE_MODIFIER_BP opt_event_config
 	struct list_head *list;
 	int err;
 
+	parse_events_unused_value(&$3);
+
 	list = alloc_list();
 	ABORT_ON(!list);
 	err = parse_events_add_breakpoint(_parse_state, list,
-- 
2.34.1