2012-05-24 16:40:37

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 0/6] perf/urgent fixes for libtraceevent

Hi Ingo,

Please consider pulling,

- Arnaldo


The following changes since commit e76df19bd986656e3c9f4a62e3dd15e7d69b607a:

Merge branch 'perf/core' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2012-05-24 12:28:14 +0200)

are available in the git repository at:


git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo

for you to fetch changes up to eaec12d7f526694f24d581a4ad23de6ce0315cd2:

tools lib traceevent: Fix signature of create_arg_item() (2012-05-24 11:36:05 -0300)

----------------------------------------------------------------
Fixes for the recently merged libtraceevent

. Selected fixes for libtraceevent, from various contributors, submitter by
Namhyung Kim in agreement with Steven Rostedt, all from the trace-cmd repo,
i.e. they have been in use for quite a while in trace-cmd.

Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

----------------------------------------------------------------
Namhyung Kim (5):
tools lib traceevent: Fix a possible memory leak
tools lib traceevent: Fix a possibly wrong memory dereference
tools lib traceevent: Fix freeing arg on process_dynamic_array()
tools lib traceevent: Use proper function parameter type
tools lib traceevent: Fix signature of create_arg_item()

Stefan Hajnoczi (1):
tools lib traceevent: Allow expressions in __print_symbolic() fields

tools/lib/traceevent/event-parse.c | 22 ++++++++++++++++++----
tools/lib/traceevent/parse-filter.c | 5 ++---
2 files changed, 20 insertions(+), 7 deletions(-)


2012-05-24 16:40:33

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/6] tools lib traceevent: Allow expressions in __print_symbolic() fields

From: Stefan Hajnoczi <[email protected]>

The __print_symbolic() function takes a sequence of key-value pairs for
pretty-printing a constant. The new kvm:kvm_exit print fmt uses the
expression:

__print_symbolic(..., { 0x040 + 1, "DB excp" }, ...)

Currently only atoms are supported and this print fmt fails to parse.
This patch adds support for expressions instead of just atoms so that
0x040 + 1 is parsed successfully.

Cc: Avi Kivity <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 9985349..33450c9 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2124,6 +2124,13 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **

free_token(token);
type = process_arg(event, arg, &token);
+
+ if (type == EVENT_OP)
+ type = process_op(event, arg, &token);
+
+ if (type == EVENT_ERROR)
+ goto out_free;
+
if (test_type_token(type, token, EVENT_DELIM, ","))
goto out_free;

--
1.7.9.2.358.g22243

2012-05-24 16:40:42

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 3/6] tools lib traceevent: Fix a possibly wrong memory dereference

From: Namhyung Kim <[email protected]>

If set_op_prio() failed, the token will be freed at out_free,
then arg->op.op would turn out to be a dangle pointer. After
returning EVENT_ERROR from process_op(), free_arg() will be
called and then it will finally see the dangling pointer.

Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index d598b37..445a43a 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1715,6 +1715,8 @@ process_op(struct event_format *event, struct print_arg *arg, char **tok)

if (set_op_prio(arg) == -1) {
event->flags |= EVENT_FL_FAILED;
+ /* arg->op.op (= token) will be freed at out_free */
+ arg->op.op = NULL;
goto out_free;
}

--
1.7.9.2.358.g22243

2012-05-24 16:40:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 5/6] tools lib traceevent: Use proper function parameter type

From: Namhyung Kim <[email protected]>

The param needs to be updated when setting args up so that
the loop in process_defined_func() can see the correct
param->type for the farg.

Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 3559027..5548282 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3375,6 +3375,7 @@ process_defined_func(struct trace_seq *s, void *data, int size,
break;
}
farg = farg->next;
+ param = param->next;
}

ret = (*func_handle->func)(s, args);
--
1.7.9.2.358.g22243

2012-05-24 16:40:35

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 6/6] tools lib traceevent: Fix signature of create_arg_item()

From: Namhyung Kim <[email protected]>

The @type should be a type of enum event_type not enum filter_arg_type.

This fixes following warning:

$ make
COMPILE FPIC parse-events.o
COMPILE FPIC parse-filter.o
/home/namhyung/project/trace-cmd/parse-filter.c: In function ‘create_arg_item’:
/home/namhyung/project/trace-cmd/parse-filter.c:343:9: warning: comparison between ‘enum filter_arg_type’ and ‘enum event_type’ [-Wenum-compare]
/home/namhyung/project/trace-cmd/parse-filter.c:339:2: warning: case value ‘8’ not in enumerated type ‘enum filter_arg_type’ [-Wswitch]
BUILD STATIC LIB libparsevent.a
BUILD STATIC LIB libtracecmd.a
BUILD trace-cmd
/usr/bin/make -C /home/namhyung/project/trace-cmd/Documentation all
make[1]: Nothing to be done for `all'.
Note: to build the gui, type "make gui"

Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/parse-filter.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c
index 2d40c5e..e08d21f 100644
--- a/tools/lib/traceevent/parse-filter.c
+++ b/tools/lib/traceevent/parse-filter.c
@@ -325,9 +325,8 @@ static void free_events(struct event_list *events)
}

static struct filter_arg *
-create_arg_item(struct event_format *event,
- const char *token, enum filter_arg_type type,
- char **error_str)
+create_arg_item(struct event_format *event, const char *token,
+ enum event_type type, char **error_str)
{
struct format_field *field;
struct filter_arg *arg;
--
1.7.9.2.358.g22243

2012-05-24 16:41:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 4/6] tools lib traceevent: Fix freeing arg on process_dynamic_array()

From: Namhyung Kim <[email protected]>

The @arg paremeter should not be freed inside of process_XXX(),
because it'd be freed from the caller of process_arg(). We can
free it only after it was reused for local usage.

Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 445a43a..3559027 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2300,17 +2300,18 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
arg = alloc_arg();
type = process_arg(event, arg, &token);
if (type == EVENT_ERROR)
- goto out_free;
+ goto out_free_arg;

if (!test_type_token(type, token, EVENT_OP, "]"))
- goto out_free;
+ goto out_free_arg;

free_token(token);
type = read_token_item(tok);
return type;

+ out_free_arg:
+ free_arg(arg);
out_free:
- free(arg);
free_token(token);
*tok = NULL;
return EVENT_ERROR;
--
1.7.9.2.358.g22243

2012-05-24 16:41:53

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 2/6] tools lib traceevent: Fix a possible memory leak

From: Namhyung Kim <[email protected]>

If event_read_fields failed in the middle, each member of
struct format_field should be freed also.

Cc: Borislav Petkov <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Namhyung Kim <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 33450c9..d598b37 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -1434,8 +1434,11 @@ static int event_read_fields(struct event_format *event, struct format_field **f
fail:
free_token(token);
fail_expect:
- if (field)
+ if (field) {
+ free(field->type);
+ free(field->name);
free(field);
+ }
return -1;
}

--
1.7.9.2.358.g22243

2012-05-24 17:04:47

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 0/6] perf/urgent fixes for libtraceevent


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> Please consider pulling,
>
> - Arnaldo
>
>
> The following changes since commit e76df19bd986656e3c9f4a62e3dd15e7d69b607a:
>
> Merge branch 'perf/core' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2012-05-24 12:28:14 +0200)
>
> are available in the git repository at:
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo
>
> for you to fetch changes up to eaec12d7f526694f24d581a4ad23de6ce0315cd2:
>
> tools lib traceevent: Fix signature of create_arg_item() (2012-05-24 11:36:05 -0300)
>
> ----------------------------------------------------------------
> Fixes for the recently merged libtraceevent
>
> . Selected fixes for libtraceevent, from various contributors, submitter by
> Namhyung Kim in agreement with Steven Rostedt, all from the trace-cmd repo,
> i.e. they have been in use for quite a while in trace-cmd.
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Namhyung Kim (5):
> tools lib traceevent: Fix a possible memory leak
> tools lib traceevent: Fix a possibly wrong memory dereference
> tools lib traceevent: Fix freeing arg on process_dynamic_array()
> tools lib traceevent: Use proper function parameter type
> tools lib traceevent: Fix signature of create_arg_item()
>
> Stefan Hajnoczi (1):
> tools lib traceevent: Allow expressions in __print_symbolic() fields
>
> tools/lib/traceevent/event-parse.c | 22 ++++++++++++++++++----
> tools/lib/traceevent/parse-filter.c | 5 ++---
> 2 files changed, 20 insertions(+), 7 deletions(-)

Pulled, thanks Arnaldo!

Ingo