[PATCH v2] libtraceevent: Handle parsing of "(REC)->" case

[PATCH v2] libtraceevent: Handle parsing of "(REC)->" case

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

If a trace event wraps the special __entry variable to dereference it with
parenthesis, it shows up in the trace event format file wrapping the
"(REC)" as well. For example, the "func_repeats" event passed the __entry
into a helper macro in the TP_printk() portion, and the macro correctly
wrapped its parameter in parenthesis. This caused the output to show:

 "(((u64)(REC)->top_delta_ts << 32) | (REC)->bottom_delta_ts)"

The parser then failed to parse the "(REC)->" portion, as it expected the
"->" to appear directly after the "REC". This is not a requirement, and
the parser should be able to handle such cases.

When this occurred, trace-cmd would error with the following message:

trace-cmd: No such file or directory
  Error: expected type 4 but read 5

Fixes: 6582b0a ("tools/events: Add files to create libtraceevent.a")
Reported-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---

Changes since v1:
    Update the change log to show the error message, in case
    it happens again, it can be something to reference.

 src/event-parse.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/event-parse.c b/src/event-parse.c
index 97c1a97..1217491 100644
--- a/src/event-parse.c
+++ b/src/event-parse.c
@@ -2311,8 +2311,19 @@ process_entry(struct tep_event *event __maybe_unused, struct tep_print_arg *arg,
 	char *field;
 	char *token;
 
-	if (read_expected(TEP_EVENT_OP, "->") < 0)
-		goto out_err;
+	type = read_token_item(&token);
+	/*
+	 * Check if REC happens to be surrounded by parenthesis, and
+	 * return if that's the case, as "(REC)->" is valid.
+	 * but return TEP_EVENT_ITEM.
+	 */
+	if (type == TEP_EVENT_DELIM && strcmp(token, ")") == 0) {
+		*tok = token;
+		return TEP_EVENT_ITEM;
+	}
+
+	if (test_type_token(type, token,  TEP_EVENT_OP, "->"))
+		goto out_free;
 
 	if (read_expect_type(TEP_EVENT_ITEM, &token) < 0)
 		goto out_free;
@@ -2338,7 +2349,6 @@ process_entry(struct tep_event *event __maybe_unused, struct tep_print_arg *arg,
 
  out_free:
 	free_token(token);
- out_err:
 	*tok = NULL;
 	return TEP_EVENT_ERROR;
 }
@@ -3033,6 +3043,17 @@ process_paren(struct tep_event *event, struct tep_print_arg *arg, char **tok)
 	if (type == TEP_EVENT_ERROR)
 		goto out_free;
 
+	/*
+	 * If REC is surrounded by parenthesis, the process_arg()
+	 * will return TEP_EVENT_ITEM with token == ")". In
+	 * this case, we need to continue processing the item
+	 * and return.
+	 */
+	if (type == TEP_EVENT_ITEM && strcmp(token, ")") == 0) {
+		free_token(token);
+		return process_entry(event, arg, tok);
+	}
+
 	if (test_type_token(type, token, TEP_EVENT_DELIM, ")"))
 		goto out_free;
 
-- 
2.29.2

Re: [PATCH v2] libtraceevent: Handle parsing of "(REC)->" case

[Julia Lawall]

On Tue, 8 Jun 2021, Steven Rostedt wrote:

> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> If a trace event wraps the special __entry variable to dereference it with
> parenthesis, it shows up in the trace event format file wrapping the
> "(REC)" as well. For example, the "func_repeats" event passed the __entry
> into a helper macro in the TP_printk() portion, and the macro correctly
> wrapped its parameter in parenthesis. This caused the output to show:
>
>  "(((u64)(REC)->top_delta_ts << 32) | (REC)->bottom_delta_ts)"
>
> The parser then failed to parse the "(REC)->" portion, as it expected the
> "->" to appear directly after the "REC". This is not a requirement, and
> the parser should be able to handle such cases.
>
> When this occurred, trace-cmd would error with the following message:
>
> trace-cmd: No such file or directory
>   Error: expected type 4 but read 5

For the record, I have tried tracing two programs with Linux v5.13-rc4 or
rc5 and both of them had this problem, while they didn't have this problem
when traced with the same arguments with earlier Linux versions.  So it
may be systematic for Linux v5.13.

julia

>
> Fixes: 6582b0a ("tools/events: Add files to create libtraceevent.a")
> Reported-by: Julia Lawall <julia.lawall@inria.fr>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>
> Changes since v1:
>     Update the change log to show the error message, in case
>     it happens again, it can be something to reference.
>
>  src/event-parse.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/src/event-parse.c b/src/event-parse.c
> index 97c1a97..1217491 100644
> --- a/src/event-parse.c
> +++ b/src/event-parse.c
> @@ -2311,8 +2311,19 @@ process_entry(struct tep_event *event __maybe_unused, struct tep_print_arg *arg,
>  	char *field;
>  	char *token;
>
> -	if (read_expected(TEP_EVENT_OP, "->") < 0)
> -		goto out_err;
> +	type = read_token_item(&token);
> +	/*
> +	 * Check if REC happens to be surrounded by parenthesis, and
> +	 * return if that's the case, as "(REC)->" is valid.
> +	 * but return TEP_EVENT_ITEM.
> +	 */
> +	if (type == TEP_EVENT_DELIM && strcmp(token, ")") == 0) {
> +		*tok = token;
> +		return TEP_EVENT_ITEM;
> +	}
> +
> +	if (test_type_token(type, token,  TEP_EVENT_OP, "->"))
> +		goto out_free;
>
>  	if (read_expect_type(TEP_EVENT_ITEM, &token) < 0)
>  		goto out_free;
> @@ -2338,7 +2349,6 @@ process_entry(struct tep_event *event __maybe_unused, struct tep_print_arg *arg,
>
>   out_free:
>  	free_token(token);
> - out_err:
>  	*tok = NULL;
>  	return TEP_EVENT_ERROR;
>  }
> @@ -3033,6 +3043,17 @@ process_paren(struct tep_event *event, struct tep_print_arg *arg, char **tok)
>  	if (type == TEP_EVENT_ERROR)
>  		goto out_free;
>
> +	/*
> +	 * If REC is surrounded by parenthesis, the process_arg()
> +	 * will return TEP_EVENT_ITEM with token == ")". In
> +	 * this case, we need to continue processing the item
> +	 * and return.
> +	 */
> +	if (type == TEP_EVENT_ITEM && strcmp(token, ")") == 0) {
> +		free_token(token);
> +		return process_entry(event, arg, tok);
> +	}
> +
>  	if (test_type_token(type, token, TEP_EVENT_DELIM, ")"))
>  		goto out_free;
>
> --
> 2.29.2
>
>

Re: [PATCH v2] libtraceevent: Handle parsing of "(REC)->" case

[Steven Rostedt]

On Wed, 9 Jun 2021 07:43:08 +0200 (CEST)
Julia Lawall <julia.lawall@inria.fr> wrote:

> > If a trace event wraps the special __entry variable to dereference it with
> > parenthesis, it shows up in the trace event format file wrapping the
> > "(REC)" as well. For example, the "func_repeats" event passed the __entry
> > into a helper macro in the TP_printk() portion, and the macro correctly
> > wrapped its parameter in parenthesis. This caused the output to show:
> >
> >  "(((u64)(REC)->top_delta_ts << 32) | (REC)->bottom_delta_ts)"
> >
> > The parser then failed to parse the "(REC)->" portion, as it expected the
> > "->" to appear directly after the "REC". This is not a requirement, and
> > the parser should be able to handle such cases.
> >
> > When this occurred, trace-cmd would error with the following message:
> >
> > trace-cmd: No such file or directory
> >   Error: expected type 4 but read 5  
> 
> For the record, I have tried tracing two programs with Linux v5.13-rc4 or
> rc5 and both of them had this problem, while they didn't have this problem
> when traced with the same arguments with earlier Linux versions.  So it
> may be systematic for Linux v5.13.

The event mentioned in the change log was added in 5.13. But it is a valid
pattern, and not a bug in the 5.13 kernel.

-- Steve