All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] tools lib traceevent: add checks for returned EVENT_ERROR type
@ 2015-08-10 12:22 Dean Nelson
  2015-08-20 15:23 ` Dean Nelson
  0 siblings, 1 reply; 2+ messages in thread
From: Dean Nelson @ 2015-08-10 12:22 UTC (permalink / raw)
  To: a.p.zijlstra, mingo, rostedt, acme; +Cc: namhyung, linux-kernel, jolsa

Running the following perf-stat command on an arm64 system produces the
following result...

  [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
    Warning: [kmem:mm_page_alloc] function sizeof not defined
    Warning: Error: expected type 4 but read 0
  Segmentation fault
  [root@aarch64 ~]#

The second warning message and SIGSEGV stem from the issue expressed in the
first warning message, and are the result of ignoring the EVENT_ERROR type
returned back through the call chain.

Dealing with the first warning message is beyond the scope of this patch. But
the second warning is addressed by this patch's first hunk. And the SIGSEGV is
eliminated by its second hunk.

Signed-off-by: Dean Nelson <dnelson@redhat.com>
---
Changes in v2:
  - Moved the second hunk's check for EVENT_ERROR to a separate line.

 tools/lib/traceevent/event-parse.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index cc25f05..dafddc6 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
 	type = process_arg(event, left, &token);
 
  again:
+	if (type == EVENT_ERROR)
+		goto out_free;
+
 	/* Handle other operations in the arguments */
 	if (type == EVENT_OP && strcmp(token, ":") != 0) {
 		type = process_op(event, left, &token);
@@ -1940,7 +1943,8 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
 
 		type = process_arg_token(event, right, tok, type);
 
-		if (right->type == PRINT_OP &&
+		if (type != EVENT_ERROR &&
+		    right->type == PRINT_OP &&
 		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
 			struct print_arg tmp;
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] tools lib traceevent: add checks for returned EVENT_ERROR type
  2015-08-10 12:22 [PATCH v2] tools lib traceevent: add checks for returned EVENT_ERROR type Dean Nelson
@ 2015-08-20 15:23 ` Dean Nelson
  0 siblings, 0 replies; 2+ messages in thread
From: Dean Nelson @ 2015-08-20 15:23 UTC (permalink / raw)
  To: a.p.zijlstra, mingo, rostedt, acme; +Cc: namhyung, linux-kernel, jolsa

On 08/10/2015 07:22 AM, Dean Nelson wrote:
> Running the following perf-stat command on an arm64 system produces the
> following result...
>
>    [root@aarch64 ~]# perf stat -e kmem:mm_page_alloc -a sleep 1
>      Warning: [kmem:mm_page_alloc] function sizeof not defined
>      Warning: Error: expected type 4 but read 0
>    Segmentation fault
>    [root@aarch64 ~]#
>
> The second warning message and SIGSEGV stem from the issue expressed in the
> first warning message, and are the result of ignoring the EVENT_ERROR type
> returned back through the call chain.
>
> Dealing with the first warning message is beyond the scope of this patch. But
> the second warning is addressed by this patch's first hunk. And the SIGSEGV is
> eliminated by its second hunk.
>
> Signed-off-by: Dean Nelson <dnelson@redhat.com>
> ---
> Changes in v2:
>    - Moved the second hunk's check for EVENT_ERROR to a separate line.

I misunderstood what Namhyung meant by his request that I put the check
on separate lines.

I've posted a v3 that hopefully corrects this misunderstanding. So
consider v2 NAK'd.


>   tools/lib/traceevent/event-parse.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index cc25f05..dafddc6 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -1680,6 +1680,9 @@ process_cond(struct event_format *event, struct print_arg *top, char **tok)
>   	type = process_arg(event, left, &token);
>
>    again:
> +	if (type == EVENT_ERROR)
> +		goto out_free;
> +
>   	/* Handle other operations in the arguments */
>   	if (type == EVENT_OP && strcmp(token, ":") != 0) {
>   		type = process_op(event, left, &token);
> @@ -1940,7 +1943,8 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)
>
>   		type = process_arg_token(event, right, tok, type);
>
> -		if (right->type == PRINT_OP &&
> +		if (type != EVENT_ERROR &&
> +		    right->type == PRINT_OP &&
>   		    get_op_prio(arg->op.op) < get_op_prio(right->op.op)) {
>   			struct print_arg tmp;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-08-20 15:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-10 12:22 [PATCH v2] tools lib traceevent: add checks for returned EVENT_ERROR type Dean Nelson
2015-08-20 15:23 ` Dean Nelson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.