* [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations
@ 2013-07-02 19:27 David Ahern
2013-07-02 19:27 ` [PATCH 1/6] perf evsel: fix count parameter to read call in event_format__new David Ahern
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel; +Cc: David Ahern
David Ahern (6):
perf evsel: fix count parameter to read call in event_format__new
perf evlist: fix use of uninitialized variable
perf event: initialize allocated memory for synthesized events
perf: don't free list head in parse_events__free_terms
perf test: make terms a stack variable in test_term
perf parse events: demystify memory allocations
tools/perf/tests/parse-events.c | 14 ++++-----
tools/perf/util/event.c | 8 ++---
tools/perf/util/evlist.c | 2 +-
tools/perf/util/evsel.c | 2 +-
tools/perf/util/parse-events.c | 54 ++++++++++------------------------
tools/perf/util/parse-events.h | 10 +++----
tools/perf/util/parse-events.y | 62 +++++++++++++++++++++++++--------------
7 files changed, 72 insertions(+), 80 deletions(-)
--
1.7.10.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] perf evsel: fix count parameter to read call in event_format__new
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
@ 2013-07-02 19:27 ` David Ahern
2013-07-12 8:51 ` [tip:perf/urgent] perf evsel: Fix " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 2/6] perf evlist: fix use of uninitialized variable David Ahern
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel
Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra,
Jiri Olsa, Namhyung Kim
per realloc above the length of the buffer is alloc_size, not BUFSIZ.
Adjust length per size as done for buf start.
Addresses some valgrind complaints:
==1870== Syscall param read(buf) points to unaddressable byte(s)
==1870== at 0x4E3F610: __read_nocancel (in /lib64/libpthread-2.14.90.so)
==1870== by 0x44AEE1: event_format__new (unistd.h:45)
==1870== by 0x44B025: perf_evsel__newtp (evsel.c:158)
==1870== by 0x451919: add_tracepoint_event (parse-events.c:395)
==1870== by 0x479815: parse_events_parse (parse-events.y:292)
==1870== by 0x45463A: parse_events_option (parse-events.c:861)
==1870== by 0x44FEE4: get_value (parse-options.c:113)
==1870== by 0x450767: parse_options_step (parse-options.c:192)
==1870== by 0x450C40: parse_options (parse-options.c:422)
==1870== by 0x42735F: cmd_record (builtin-record.c:918)
==1870== by 0x419D72: run_builtin (perf.c:319)
==1870== by 0x4195F2: main (perf.c:376)
==1870== Address 0xcffebf0 is 0 bytes after a block of size 8,192 alloc'd
==1870== at 0x4C2A62F: malloc (vg_replace_malloc.c:270)
==1870== by 0x4C2A7A3: realloc (vg_replace_malloc.c:662)
==1870== by 0x44AF07: event_format__new (evsel.c:121)
==1870== by 0x44B025: perf_evsel__newtp (evsel.c:158)
==1870== by 0x451919: add_tracepoint_event (parse-events.c:395)
==1870== by 0x479815: parse_events_parse (parse-events.y:292)
==1870== by 0x45463A: parse_events_option (parse-events.c:861)
==1870== by 0x44FEE4: get_value (parse-options.c:113)
==1870== by 0x450767: parse_options_step (parse-options.c:192)
==1870== by 0x450C40: parse_options (parse-options.c:422)
==1870== by 0x42735F: cmd_record (builtin-record.c:918)
==1870== by 0x419D72: run_builtin (perf.c:319)
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evsel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index dea0684..01fdfc6 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -124,7 +124,7 @@ struct event_format *event_format__new(const char *sys, const char *name)
bf = nbf;
}
- n = read(fd, bf + size, BUFSIZ);
+ n = read(fd, bf + size, alloc_size - size);
if (n < 0)
goto out_free_bf;
size += n;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] perf evlist: fix use of uninitialized variable
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
2013-07-02 19:27 ` [PATCH 1/6] perf evsel: fix count parameter to read call in event_format__new David Ahern
@ 2013-07-02 19:27 ` David Ahern
2013-07-19 7:44 ` [tip:perf/core] perf evlist: Fix " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 3/6] perf event: initialize allocated memory for synthesized events David Ahern
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel
Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra,
Jiri Olsa, Namhyung Kim
Fixes valgrind complaint:
==1870== Syscall param write(buf) points to uninitialised byte(s)
==1870== at 0x4E3F5B0: __write_nocancel (in /lib64/libpthread-2.14.90.so)
==1870== by 0x449D7C: perf_evlist__start_workload (evlist.c:846)
==1870== by 0x427BC1: cmd_record (builtin-record.c:561)
==1870== by 0x419D72: run_builtin (perf.c:319)
==1870== by 0x4195F2: main (perf.c:376)
==1870== Address 0x7feffcdd7 is on thread 1's stack
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 4a901be..d8f34e0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -838,7 +838,7 @@ out_close_ready_pipe:
int perf_evlist__start_workload(struct perf_evlist *evlist)
{
if (evlist->workload.cork_fd > 0) {
- char bf;
+ char bf = 0;
int ret;
/*
* Remove the cork, let it rip!
--
1.7.10.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] perf event: initialize allocated memory for synthesized events
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
2013-07-02 19:27 ` [PATCH 1/6] perf evsel: fix count parameter to read call in event_format__new David Ahern
2013-07-02 19:27 ` [PATCH 2/6] perf evlist: fix use of uninitialized variable David Ahern
@ 2013-07-02 19:27 ` David Ahern
2013-07-11 16:35 ` David Ahern
2013-07-02 19:27 ` [PATCH 4/6] perf: don't free list head in parse_events__free_terms David Ahern
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel
Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra,
Jiri Olsa, Namhyung Kim
Fixes valgrind complaint:
=3118== Syscall param write(buf) points to uninitialised byte(s)
==3118== at 0x4E3F5B0: __write_nocancel (in /lib64/libpthread-2.14.90.so)
==3118== by 0x42F0EB: process_synthesized_event (builtin-record.c:89)
==3118== by 0x44E81C: perf_event__synthesize_mmap_events (event.c:230)
==3118== by 0x44F27C: perf_event__synthesize_threads (event.c:307)
==3118== by 0x42FEEA: cmd_record (builtin-record.c:530)
==3118== by 0x419D22: run_builtin (perf.c:319)
==3118== by 0x4195A2: main (perf.c:376)
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/event.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 5cd13d7..556b999 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -316,11 +316,11 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
union perf_event *comm_event, *mmap_event;
int err = -1, thread, j;
- comm_event = malloc(sizeof(comm_event->comm) + machine->id_hdr_size);
+ comm_event = zalloc(sizeof(comm_event->comm) + machine->id_hdr_size);
if (comm_event == NULL)
goto out;
- mmap_event = malloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
+ mmap_event = zalloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
if (mmap_event == NULL)
goto out_free_comm;
@@ -375,11 +375,11 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
union perf_event *comm_event, *mmap_event;
int err = -1;
- comm_event = malloc(sizeof(comm_event->comm) + machine->id_hdr_size);
+ comm_event = zalloc(sizeof(comm_event->comm) + machine->id_hdr_size);
if (comm_event == NULL)
goto out;
- mmap_event = malloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
+ mmap_event = zalloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
if (mmap_event == NULL)
goto out_free_comm;
--
1.7.10.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] perf: don't free list head in parse_events__free_terms
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
` (2 preceding siblings ...)
2013-07-02 19:27 ` [PATCH 3/6] perf event: initialize allocated memory for synthesized events David Ahern
@ 2013-07-02 19:27 ` David Ahern
2013-07-19 7:45 ` [tip:perf/core] perf tools: Don' t " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 5/6] perf test: make terms a stack variable in test_term David Ahern
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel
Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra,
Jiri Olsa, Namhyung Kim, Adrian Hunter
Function should only be freeing the entries in the list, not
the list_head itself.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
tools/perf/util/parse-events.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1ae166c..3a34f7b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1258,6 +1258,4 @@ void parse_events__free_terms(struct list_head *terms)
list_for_each_entry_safe(term, h, terms, list)
free(term);
-
- free(terms);
}
--
1.7.10.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] perf test: make terms a stack variable in test_term
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
` (3 preceding siblings ...)
2013-07-02 19:27 ` [PATCH 4/6] perf: don't free list head in parse_events__free_terms David Ahern
@ 2013-07-02 19:27 ` David Ahern
2013-07-19 7:45 ` [tip:perf/core] perf tests: Make " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 6/6] perf parse events: demystify memory allocations David Ahern
2013-07-05 14:34 ` [PATCH 0/6] perf: bug fixes, cleanup of parse events " Arnaldo Carvalho de Melo
6 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel
Cc: David Ahern, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra,
Jiri Olsa, Namhyung Kim, Adrian Hunter
No need to malloc the memory for it.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
tools/perf/tests/parse-events.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index ad950f5..344c844 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1246,24 +1246,20 @@ static int test_events(struct evlist_test *events, unsigned cnt)
static int test_term(struct terms_test *t)
{
- struct list_head *terms;
+ struct list_head terms;
int ret;
- terms = malloc(sizeof(*terms));
- if (!terms)
- return -ENOMEM;
-
- INIT_LIST_HEAD(terms);
+ INIT_LIST_HEAD(&terms);
- ret = parse_events_terms(terms, t->str);
+ ret = parse_events_terms(&terms, t->str);
if (ret) {
pr_debug("failed to parse terms '%s', err %d\n",
t->str , ret);
return ret;
}
- ret = t->check(terms);
- parse_events__free_terms(terms);
+ ret = t->check(&terms);
+ parse_events__free_terms(&terms);
return ret;
}
--
1.7.10.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] perf parse events: demystify memory allocations
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
` (4 preceding siblings ...)
2013-07-02 19:27 ` [PATCH 5/6] perf test: make terms a stack variable in test_term David Ahern
@ 2013-07-02 19:27 ` David Ahern
2013-07-07 15:26 ` Jiri Olsa
2013-07-19 7:45 ` [tip:perf/core] perf parse events: Demystify " tip-bot for David Ahern
2013-07-05 14:34 ` [PATCH 0/6] perf: bug fixes, cleanup of parse events " Arnaldo Carvalho de Melo
6 siblings, 2 replies; 17+ messages in thread
From: David Ahern @ 2013-07-02 19:27 UTC (permalink / raw)
To: acme, linux-kernel
Cc: David Ahern, Jiri Olsa, Ingo Molnar, Frederic Weisbecker,
Peter Zijlstra, Namhyung Kim, Adrian Hunter
List heads are currently allocated way down the function chain in __add_event
and add_tracepoint and then freed when the scanner code calls
parse_events_update_lists.
Be more explicit with where memory is allocated and who should free it. With
this patch the list_head is allocated in the scanner code and freed when the
scanner code calls parse_events_update_lists.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
---
tools/perf/util/parse-events.c | 52 +++++++++++----------------------
tools/perf/util/parse-events.h | 10 +++----
tools/perf/util/parse-events.y | 62 ++++++++++++++++++++++++++--------------
3 files changed, 61 insertions(+), 63 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3a34f7b..8e3e270 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -264,40 +264,29 @@ const char *event_type(int type)
-static int __add_event(struct list_head **_list, int *idx,
+static int __add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr,
char *name, struct cpu_map *cpus)
{
struct perf_evsel *evsel;
- struct list_head *list = *_list;
-
- if (!list) {
- list = malloc(sizeof(*list));
- if (!list)
- return -ENOMEM;
- INIT_LIST_HEAD(list);
- }
event_attr_init(attr);
evsel = perf_evsel__new(attr, (*idx)++);
- if (!evsel) {
- free(list);
+ if (!evsel)
return -ENOMEM;
- }
evsel->cpus = cpus;
if (name)
evsel->name = strdup(name);
list_add_tail(&evsel->node, list);
- *_list = list;
return 0;
}
-static int add_event(struct list_head **_list, int *idx,
+static int add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr, char *name)
{
- return __add_event(_list, idx, attr, name, NULL);
+ return __add_event(list, idx, attr, name, NULL);
}
static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
@@ -318,7 +307,7 @@ static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES]
return -1;
}
-int parse_events_add_cache(struct list_head **list, int *idx,
+int parse_events_add_cache(struct list_head *list, int *idx,
char *type, char *op_result1, char *op_result2)
{
struct perf_event_attr attr;
@@ -379,31 +368,21 @@ int parse_events_add_cache(struct list_head **list, int *idx,
return add_event(list, idx, &attr, name);
}
-static int add_tracepoint(struct list_head **listp, int *idx,
+static int add_tracepoint(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
struct perf_evsel *evsel;
- struct list_head *list = *listp;
-
- if (!list) {
- list = malloc(sizeof(*list));
- if (!list)
- return -ENOMEM;
- INIT_LIST_HEAD(list);
- }
evsel = perf_evsel__newtp(sys_name, evt_name, (*idx)++);
- if (!evsel) {
- free(list);
+ if (!evsel)
return -ENOMEM;
- }
list_add_tail(&evsel->node, list);
- *listp = list;
+
return 0;
}
-static int add_tracepoint_multi_event(struct list_head **list, int *idx,
+static int add_tracepoint_multi_event(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
char evt_path[MAXPATHLEN];
@@ -435,7 +414,7 @@ static int add_tracepoint_multi_event(struct list_head **list, int *idx,
return ret;
}
-static int add_tracepoint_event(struct list_head **list, int *idx,
+static int add_tracepoint_event(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
return strpbrk(evt_name, "*?") ?
@@ -443,7 +422,7 @@ static int add_tracepoint_event(struct list_head **list, int *idx,
add_tracepoint(list, idx, sys_name, evt_name);
}
-static int add_tracepoint_multi_sys(struct list_head **list, int *idx,
+static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
struct dirent *events_ent;
@@ -475,7 +454,7 @@ static int add_tracepoint_multi_sys(struct list_head **list, int *idx,
return ret;
}
-int parse_events_add_tracepoint(struct list_head **list, int *idx,
+int parse_events_add_tracepoint(struct list_head *list, int *idx,
char *sys, char *event)
{
int ret;
@@ -530,7 +509,7 @@ do { \
return 0;
}
-int parse_events_add_breakpoint(struct list_head **list, int *idx,
+int parse_events_add_breakpoint(struct list_head *list, int *idx,
void *ptr, char *type)
{
struct perf_event_attr attr;
@@ -611,7 +590,7 @@ static int config_attr(struct perf_event_attr *attr,
return 0;
}
-int parse_events_add_numeric(struct list_head **list, int *idx,
+int parse_events_add_numeric(struct list_head *list, int *idx,
u32 type, u64 config,
struct list_head *head_config)
{
@@ -644,7 +623,7 @@ static char *pmu_event_name(struct list_head *head_terms)
return NULL;
}
-int parse_events_add_pmu(struct list_head **list, int *idx,
+int parse_events_add_pmu(struct list_head *list, int *idx,
char *name, struct list_head *head_config)
{
struct perf_event_attr attr;
@@ -687,6 +666,7 @@ void parse_events__set_leader(char *name, struct list_head *list)
leader->group_name = name ? strdup(name) : NULL;
}
+/* list_event is assumed to point to malloc'ed memory */
void parse_events_update_lists(struct list_head *list_event,
struct list_head *list_all)
{
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 080f7cf..f1cb4c4 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -85,16 +85,16 @@ void parse_events__free_terms(struct list_head *terms);
int parse_events__modifier_event(struct list_head *list, char *str, bool add);
int parse_events__modifier_group(struct list_head *list, char *event_mod);
int parse_events_name(struct list_head *list, char *name);
-int parse_events_add_tracepoint(struct list_head **list, int *idx,
+int parse_events_add_tracepoint(struct list_head *list, int *idx,
char *sys, char *event);
-int parse_events_add_numeric(struct list_head **list, int *idx,
+int parse_events_add_numeric(struct list_head *list, int *idx,
u32 type, u64 config,
struct list_head *head_config);
-int parse_events_add_cache(struct list_head **list, int *idx,
+int parse_events_add_cache(struct list_head *list, int *idx,
char *type, char *op_result1, char *op_result2);
-int parse_events_add_breakpoint(struct list_head **list, int *idx,
+int parse_events_add_breakpoint(struct list_head *list, int *idx,
void *ptr, char *type);
-int parse_events_add_pmu(struct list_head **list, int *idx,
+int parse_events_add_pmu(struct list_head *list, int *idx,
char *pmu , struct list_head *head_config);
void parse_events__set_leader(char *name, struct list_head *list);
void parse_events_update_lists(struct list_head *list_event,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index afc44c1..4eb67ec 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -22,6 +22,13 @@ do { \
YYABORT; \
} while (0)
+#define ALLOC_LIST(list) \
+do { \
+ list = malloc(sizeof(*list)); \
+ ABORT_ON(!list); \
+ INIT_LIST_HEAD(list); \
+} while (0)
+
static inc_group_count(struct list_head *list,
struct parse_events_evlist *data)
{
@@ -196,9 +203,10 @@ event_pmu:
PE_NAME '/' event_config '/'
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_pmu(&list, &data->idx, $1, $3));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_pmu(list, &data->idx, $1, $3));
parse_events__free_terms($3);
$$ = list;
}
@@ -212,11 +220,12 @@ event_legacy_symbol:
value_sym '/' event_config '/'
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
int type = $1 >> 16;
int config = $1 & 255;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx,
type, config, $3));
parse_events__free_terms($3);
$$ = list;
@@ -225,11 +234,12 @@ value_sym '/' event_config '/'
value_sym sep_slash_dc
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
int type = $1 >> 16;
int config = $1 & 255;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx,
type, config, NULL));
$$ = list;
}
@@ -238,27 +248,30 @@ event_legacy_cache:
PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_cache(&list, &data->idx, $1, $3, $5));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, $5));
$$ = list;
}
|
PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_cache(&list, &data->idx, $1, $3, NULL));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, NULL));
$$ = list;
}
|
PE_NAME_CACHE_TYPE
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_cache(&list, &data->idx, $1, NULL, NULL));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_cache(list, &data->idx, $1, NULL, NULL));
$$ = list;
}
@@ -266,9 +279,10 @@ event_legacy_mem:
PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
(void *) $2, $4));
$$ = list;
}
@@ -276,9 +290,10 @@ PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
PE_PREFIX_MEM PE_VALUE sep_dc
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
(void *) $2, NULL));
$$ = list;
}
@@ -287,9 +302,10 @@ event_legacy_tracepoint:
PE_NAME ':' PE_NAME
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_tracepoint(&list, &data->idx, $1, $3));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_tracepoint(list, &data->idx, $1, $3));
$$ = list;
}
@@ -297,9 +313,10 @@ event_legacy_numeric:
PE_VALUE ':' PE_VALUE
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx, (u32)$1, $3, NULL));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx, (u32)$1, $3, NULL));
$$ = list;
}
@@ -307,9 +324,10 @@ event_legacy_raw:
PE_RAW
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx,
PERF_TYPE_RAW, $1, NULL));
$$ = list;
}
--
1.7.10.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
` (5 preceding siblings ...)
2013-07-02 19:27 ` [PATCH 6/6] perf parse events: demystify memory allocations David Ahern
@ 2013-07-05 14:34 ` Arnaldo Carvalho de Melo
6 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-07-05 14:34 UTC (permalink / raw)
To: David Ahern; +Cc: linux-kernel
Em Tue, Jul 02, 2013 at 01:27:19PM -0600, David Ahern escreveu:
> David Ahern (6):
> perf evsel: fix count parameter to read call in event_format__new
> perf evlist: fix use of uninitialized variable
> perf event: initialize allocated memory for synthesized events
> perf: don't free list head in parse_events__free_terms
> perf test: make terms a stack variable in test_term
> perf parse events: demystify memory allocations
Thanks, applied,
- Arnaldo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] perf parse events: demystify memory allocations
2013-07-02 19:27 ` [PATCH 6/6] perf parse events: demystify memory allocations David Ahern
@ 2013-07-07 15:26 ` Jiri Olsa
2013-07-07 16:45 ` David Ahern
2013-07-19 7:45 ` [tip:perf/core] perf parse events: Demystify " tip-bot for David Ahern
1 sibling, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2013-07-07 15:26 UTC (permalink / raw)
To: David Ahern
Cc: acme, linux-kernel, Ingo Molnar, Frederic Weisbecker,
Peter Zijlstra, Namhyung Kim, Adrian Hunter
On Tue, Jul 02, 2013 at 01:27:25PM -0600, David Ahern wrote:
> List heads are currently allocated way down the function chain in __add_event
> and add_tracepoint and then freed when the scanner code calls
> parse_events_update_lists.
>
> Be more explicit with where memory is allocated and who should free it. With
> this patch the list_head is allocated in the scanner code and freed when the
> scanner code calls parse_events_update_lists.
>
SNIP
> @@ -266,9 +279,10 @@ event_legacy_mem:
> PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
> {
> struct parse_events_evlist *data = _data;
> - struct list_head *list = NULL;
> + struct list_head *list;
>
> - ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
> + ALLOC_LIST(list);
> + ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
> (void *) $2, $4));
> $$ = list;
> }
> @@ -276,9 +290,10 @@ PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
> PE_PREFIX_MEM PE_VALUE sep_dc
> {
> struct parse_events_evlist *data = _data;
> - struct list_head *list = NULL;
> + struct list_head *list;
>
> - ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
> + ALLOC_LIST(list);
> + ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
> (void *) $2, NULL));
so who now frees the list if there's an error
in parse_events_add_breakpoint?
ditto for other ABORT_ON cases
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] perf parse events: demystify memory allocations
2013-07-07 15:26 ` Jiri Olsa
@ 2013-07-07 16:45 ` David Ahern
2013-07-07 17:00 ` Jiri Olsa
0 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2013-07-07 16:45 UTC (permalink / raw)
To: Jiri Olsa
Cc: acme, linux-kernel, Ingo Molnar, Frederic Weisbecker,
Peter Zijlstra, Namhyung Kim, Adrian Hunter
On 7/7/13 9:26 AM, Jiri Olsa wrote:
> On Tue, Jul 02, 2013 at 01:27:25PM -0600, David Ahern wrote:
>> List heads are currently allocated way down the function chain in __add_event
>> and add_tracepoint and then freed when the scanner code calls
>> parse_events_update_lists.
>>
>> Be more explicit with where memory is allocated and who should free it. With
>> this patch the list_head is allocated in the scanner code and freed when the
>> scanner code calls parse_events_update_lists.
>>
>
> SNIP
>
>> @@ -266,9 +279,10 @@ event_legacy_mem:
>> PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
>> {
>> struct parse_events_evlist *data = _data;
>> - struct list_head *list = NULL;
>> + struct list_head *list;
>>
>> - ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
>> + ALLOC_LIST(list);
>> + ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
>> (void *) $2, $4));
>> $$ = list;
>> }
>> @@ -276,9 +290,10 @@ PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
>> PE_PREFIX_MEM PE_VALUE sep_dc
>> {
>> struct parse_events_evlist *data = _data;
>> - struct list_head *list = NULL;
>> + struct list_head *list;
>>
>> - ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
>> + ALLOC_LIST(list);
>> + ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
>> (void *) $2, NULL));
>
> so who now frees the list if there's an error
> in parse_events_add_breakpoint?
According to valgrind that memory is not freed prior to this patch, so
this one does not introduce new leaks.
>
> ditto for other ABORT_ON cases
I will whip up a patch to free memory on failure paths.
David
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] perf parse events: demystify memory allocations
2013-07-07 16:45 ` David Ahern
@ 2013-07-07 17:00 ` Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2013-07-07 17:00 UTC (permalink / raw)
To: David Ahern
Cc: acme, linux-kernel, Ingo Molnar, Frederic Weisbecker,
Peter Zijlstra, Namhyung Kim, Adrian Hunter
On Sun, Jul 07, 2013 at 10:45:13AM -0600, David Ahern wrote:
> On 7/7/13 9:26 AM, Jiri Olsa wrote:
> >On Tue, Jul 02, 2013 at 01:27:25PM -0600, David Ahern wrote:
> >>List heads are currently allocated way down the function chain in __add_event
> >>and add_tracepoint and then freed when the scanner code calls
> >>parse_events_update_lists.
> >>
> >>Be more explicit with where memory is allocated and who should free it. With
> >>this patch the list_head is allocated in the scanner code and freed when the
> >>scanner code calls parse_events_update_lists.
> >>
> >
> >SNIP
> >
> >>@@ -266,9 +279,10 @@ event_legacy_mem:
> >> PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
> >> {
> >> struct parse_events_evlist *data = _data;
> >>- struct list_head *list = NULL;
> >>+ struct list_head *list;
> >>
> >>- ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
> >>+ ALLOC_LIST(list);
> >>+ ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
> >> (void *) $2, $4));
> >> $$ = list;
> >> }
> >>@@ -276,9 +290,10 @@ PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
> >> PE_PREFIX_MEM PE_VALUE sep_dc
> >> {
> >> struct parse_events_evlist *data = _data;
> >>- struct list_head *list = NULL;
> >>+ struct list_head *list;
> >>
> >>- ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
> >>+ ALLOC_LIST(list);
> >>+ ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
> >> (void *) $2, NULL));
> >
> >so who now frees the list if there's an error
> >in parse_events_add_breakpoint?
>
> According to valgrind that memory is not freed prior to this patch,
> so this one does not introduce new leaks.
I thought this hunk did:
evsel = perf_evsel__new(attr, (*idx)++);
- if (!evsel) {
- free(list);
but I might have missed other cases..
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] perf event: initialize allocated memory for synthesized events
2013-07-02 19:27 ` [PATCH 3/6] perf event: initialize allocated memory for synthesized events David Ahern
@ 2013-07-11 16:35 ` David Ahern
0 siblings, 0 replies; 17+ messages in thread
From: David Ahern @ 2013-07-11 16:35 UTC (permalink / raw)
To: acme
Cc: linux-kernel, Ingo Molnar, Frederic Weisbecker, Peter Zijlstra,
Jiri Olsa, Namhyung Kim
Arnaldo:
Don't see this one in your perf/core branch.
On 7/2/13 1:27 PM, David Ahern wrote:
> Fixes valgrind complaint:
>
> =3118== Syscall param write(buf) points to uninitialised byte(s)
> ==3118== at 0x4E3F5B0: __write_nocancel (in /lib64/libpthread-2.14.90.so)
> ==3118== by 0x42F0EB: process_synthesized_event (builtin-record.c:89)
> ==3118== by 0x44E81C: perf_event__synthesize_mmap_events (event.c:230)
> ==3118== by 0x44F27C: perf_event__synthesize_threads (event.c:307)
> ==3118== by 0x42FEEA: cmd_record (builtin-record.c:530)
> ==3118== by 0x419D22: run_builtin (perf.c:319)
> ==3118== by 0x4195A2: main (perf.c:376)
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/event.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 5cd13d7..556b999 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -316,11 +316,11 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
> union perf_event *comm_event, *mmap_event;
> int err = -1, thread, j;
>
> - comm_event = malloc(sizeof(comm_event->comm) + machine->id_hdr_size);
> + comm_event = zalloc(sizeof(comm_event->comm) + machine->id_hdr_size);
> if (comm_event == NULL)
> goto out;
>
> - mmap_event = malloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
> + mmap_event = zalloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
> if (mmap_event == NULL)
> goto out_free_comm;
>
> @@ -375,11 +375,11 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
> union perf_event *comm_event, *mmap_event;
> int err = -1;
>
> - comm_event = malloc(sizeof(comm_event->comm) + machine->id_hdr_size);
> + comm_event = zalloc(sizeof(comm_event->comm) + machine->id_hdr_size);
> if (comm_event == NULL)
> goto out;
>
> - mmap_event = malloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
> + mmap_event = zalloc(sizeof(mmap_event->mmap) + machine->id_hdr_size);
> if (mmap_event == NULL)
> goto out_free_comm;
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [tip:perf/urgent] perf evsel: Fix count parameter to read call in event_format__new
2013-07-02 19:27 ` [PATCH 1/6] perf evsel: fix count parameter to read call in event_format__new David Ahern
@ 2013-07-12 8:51 ` tip-bot for David Ahern
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for David Ahern @ 2013-07-12 8:51 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, peterz, namhyung, jolsa, fweisbec,
dsahern, tglx
Commit-ID: 7cab84e8975cfb8a59ce3e79ce75e5eedd384484
Gitweb: http://git.kernel.org/tip/7cab84e8975cfb8a59ce3e79ce75e5eedd384484
Author: David Ahern <dsahern@gmail.com>
AuthorDate: Tue, 2 Jul 2013 13:27:20 -0600
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Jul 2013 17:40:39 -0300
perf evsel: Fix count parameter to read call in event_format__new
per realloc above the length of the buffer is alloc_size, not BUFSIZ.
Adjust length per size as done for buf start.
Addresses some valgrind complaints:
==1870== Syscall param read(buf) points to unaddressable byte(s)
==1870== at 0x4E3F610: __read_nocancel (in /lib64/libpthread-2.14.90.so)
==1870== by 0x44AEE1: event_format__new (unistd.h:45)
==1870== by 0x44B025: perf_evsel__newtp (evsel.c:158)
==1870== by 0x451919: add_tracepoint_event (parse-events.c:395)
==1870== by 0x479815: parse_events_parse (parse-events.y:292)
==1870== by 0x45463A: parse_events_option (parse-events.c:861)
==1870== by 0x44FEE4: get_value (parse-options.c:113)
==1870== by 0x450767: parse_options_step (parse-options.c:192)
==1870== by 0x450C40: parse_options (parse-options.c:422)
==1870== by 0x42735F: cmd_record (builtin-record.c:918)
==1870== by 0x419D72: run_builtin (perf.c:319)
==1870== by 0x4195F2: main (perf.c:376)
==1870== Address 0xcffebf0 is 0 bytes after a block of size 8,192 alloc'd
==1870== at 0x4C2A62F: malloc (vg_replace_malloc.c:270)
==1870== by 0x4C2A7A3: realloc (vg_replace_malloc.c:662)
==1870== by 0x44AF07: event_format__new (evsel.c:121)
==1870== by 0x44B025: perf_evsel__newtp (evsel.c:158)
==1870== by 0x451919: add_tracepoint_event (parse-events.c:395)
==1870== by 0x479815: parse_events_parse (parse-events.y:292)
==1870== by 0x45463A: parse_events_option (parse-events.c:861)
==1870== by 0x44FEE4: get_value (parse-options.c:113)
==1870== by 0x450767: parse_options_step (parse-options.c:192)
==1870== by 0x450C40: parse_options (parse-options.c:422)
==1870== by 0x42735F: cmd_record (builtin-record.c:918)
==1870== by 0x419D72: run_builtin (perf.c:319)
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1372793245-4136-2-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 63b6f8c..df99ebe 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -124,7 +124,7 @@ struct event_format *event_format__new(const char *sys, const char *name)
bf = nbf;
}
- n = read(fd, bf + size, BUFSIZ);
+ n = read(fd, bf + size, alloc_size - size);
if (n < 0)
goto out_free_bf;
size += n;
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf evlist: Fix use of uninitialized variable
2013-07-02 19:27 ` [PATCH 2/6] perf evlist: fix use of uninitialized variable David Ahern
@ 2013-07-19 7:44 ` tip-bot for David Ahern
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for David Ahern @ 2013-07-19 7:44 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, peterz, namhyung, jolsa, fweisbec,
dsahern, tglx
Commit-ID: b3824404ebbd489858f3a7097c715120118fa5cd
Gitweb: http://git.kernel.org/tip/b3824404ebbd489858f3a7097c715120118fa5cd
Author: David Ahern <dsahern@gmail.com>
AuthorDate: Tue, 2 Jul 2013 13:27:21 -0600
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Jul 2013 13:46:12 -0300
perf evlist: Fix use of uninitialized variable
Fixes valgrind complaint:
==1870== Syscall param write(buf) points to uninitialised byte(s)
==1870== at 0x4E3F5B0: __write_nocancel (in /lib64/libpthread-2.14.90.so)
==1870== by 0x449D7C: perf_evlist__start_workload (evlist.c:846)
==1870== by 0x427BC1: cmd_record (builtin-record.c:561)
==1870== by 0x419D72: run_builtin (perf.c:319)
==1870== by 0x4195F2: main (perf.c:376)
==1870== Address 0x7feffcdd7 is on thread 1's stack
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1372793245-4136-3-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 4a901be..d8f34e0 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -838,7 +838,7 @@ out_close_ready_pipe:
int perf_evlist__start_workload(struct perf_evlist *evlist)
{
if (evlist->workload.cork_fd > 0) {
- char bf;
+ char bf = 0;
int ret;
/*
* Remove the cork, let it rip!
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf tools: Don' t free list head in parse_events__free_terms
2013-07-02 19:27 ` [PATCH 4/6] perf: don't free list head in parse_events__free_terms David Ahern
@ 2013-07-19 7:45 ` tip-bot for David Ahern
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for David Ahern @ 2013-07-19 7:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, peterz, namhyung, jolsa, fweisbec,
dsahern, adrian.hunter, tglx
Commit-ID: 0142dab01cd690f6e376f1fb4d4462beb054dfaf
Gitweb: http://git.kernel.org/tip/0142dab01cd690f6e376f1fb4d4462beb054dfaf
Author: David Ahern <dsahern@gmail.com>
AuthorDate: Tue, 2 Jul 2013 13:27:23 -0600
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Jul 2013 13:46:14 -0300
perf tools: Don't free list head in parse_events__free_terms
Function should only be freeing the entries in the list in case of
failure, as those were allocated there, not the list_head itself.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1372793245-4136-5-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/parse-events.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index ef72e98..bcf83ee 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1260,6 +1260,4 @@ void parse_events__free_terms(struct list_head *terms)
list_for_each_entry_safe(term, h, terms, list)
free(term);
-
- free(terms);
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf tests: Make terms a stack variable in test_term
2013-07-02 19:27 ` [PATCH 5/6] perf test: make terms a stack variable in test_term David Ahern
@ 2013-07-19 7:45 ` tip-bot for David Ahern
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for David Ahern @ 2013-07-19 7:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, peterz, namhyung, jolsa, fweisbec,
dsahern, adrian.hunter, tglx
Commit-ID: c549aca50134640537bf0fbae43c08fd5ff91932
Gitweb: http://git.kernel.org/tip/c549aca50134640537bf0fbae43c08fd5ff91932
Author: David Ahern <dsahern@gmail.com>
AuthorDate: Tue, 2 Jul 2013 13:27:24 -0600
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Jul 2013 13:46:17 -0300
perf tests: Make terms a stack variable in test_term
No need to malloc the memory for it.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1372793245-4136-6-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/tests/parse-events.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index ad950f5..344c844 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1246,24 +1246,20 @@ static int test_events(struct evlist_test *events, unsigned cnt)
static int test_term(struct terms_test *t)
{
- struct list_head *terms;
+ struct list_head terms;
int ret;
- terms = malloc(sizeof(*terms));
- if (!terms)
- return -ENOMEM;
-
- INIT_LIST_HEAD(terms);
+ INIT_LIST_HEAD(&terms);
- ret = parse_events_terms(terms, t->str);
+ ret = parse_events_terms(&terms, t->str);
if (ret) {
pr_debug("failed to parse terms '%s', err %d\n",
t->str , ret);
return ret;
}
- ret = t->check(terms);
- parse_events__free_terms(terms);
+ ret = t->check(&terms);
+ parse_events__free_terms(&terms);
return ret;
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf parse events: Demystify memory allocations
2013-07-02 19:27 ` [PATCH 6/6] perf parse events: demystify memory allocations David Ahern
2013-07-07 15:26 ` Jiri Olsa
@ 2013-07-19 7:45 ` tip-bot for David Ahern
1 sibling, 0 replies; 17+ messages in thread
From: tip-bot for David Ahern @ 2013-07-19 7:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, hpa, mingo, peterz, namhyung, jolsa, fweisbec,
dsahern, adrian.hunter, tglx
Commit-ID: c5cd8ac07e7ad5f21b1930b23b2e1bb231958430
Gitweb: http://git.kernel.org/tip/c5cd8ac07e7ad5f21b1930b23b2e1bb231958430
Author: David Ahern <dsahern@gmail.com>
AuthorDate: Tue, 2 Jul 2013 13:27:25 -0600
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Jul 2013 13:52:05 -0300
perf parse events: Demystify memory allocations
List heads are currently allocated way down the function chain in
__add_event and add_tracepoint and then freed when the scanner code
calls parse_events_update_lists.
Be more explicit with where memory is allocated and who should free it. With
this patch the list_head is allocated in the scanner code and freed when the
scanner code calls parse_events_update_lists.
Signed-off-by: David Ahern <dsahern@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1372793245-4136-7-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/parse-events.c | 52 +++++++++++------------------------
tools/perf/util/parse-events.h | 10 +++----
tools/perf/util/parse-events.y | 62 +++++++++++++++++++++++++++---------------
3 files changed, 61 insertions(+), 63 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index bcf83ee..e853769 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -264,40 +264,29 @@ const char *event_type(int type)
-static int __add_event(struct list_head **_list, int *idx,
+static int __add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr,
char *name, struct cpu_map *cpus)
{
struct perf_evsel *evsel;
- struct list_head *list = *_list;
-
- if (!list) {
- list = malloc(sizeof(*list));
- if (!list)
- return -ENOMEM;
- INIT_LIST_HEAD(list);
- }
event_attr_init(attr);
evsel = perf_evsel__new(attr, (*idx)++);
- if (!evsel) {
- free(list);
+ if (!evsel)
return -ENOMEM;
- }
evsel->cpus = cpus;
if (name)
evsel->name = strdup(name);
list_add_tail(&evsel->node, list);
- *_list = list;
return 0;
}
-static int add_event(struct list_head **_list, int *idx,
+static int add_event(struct list_head *list, int *idx,
struct perf_event_attr *attr, char *name)
{
- return __add_event(_list, idx, attr, name, NULL);
+ return __add_event(list, idx, attr, name, NULL);
}
static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
@@ -318,7 +307,7 @@ static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES]
return -1;
}
-int parse_events_add_cache(struct list_head **list, int *idx,
+int parse_events_add_cache(struct list_head *list, int *idx,
char *type, char *op_result1, char *op_result2)
{
struct perf_event_attr attr;
@@ -379,31 +368,21 @@ int parse_events_add_cache(struct list_head **list, int *idx,
return add_event(list, idx, &attr, name);
}
-static int add_tracepoint(struct list_head **listp, int *idx,
+static int add_tracepoint(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
struct perf_evsel *evsel;
- struct list_head *list = *listp;
-
- if (!list) {
- list = malloc(sizeof(*list));
- if (!list)
- return -ENOMEM;
- INIT_LIST_HEAD(list);
- }
evsel = perf_evsel__newtp(sys_name, evt_name, (*idx)++);
- if (!evsel) {
- free(list);
+ if (!evsel)
return -ENOMEM;
- }
list_add_tail(&evsel->node, list);
- *listp = list;
+
return 0;
}
-static int add_tracepoint_multi_event(struct list_head **list, int *idx,
+static int add_tracepoint_multi_event(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
char evt_path[MAXPATHLEN];
@@ -435,7 +414,7 @@ static int add_tracepoint_multi_event(struct list_head **list, int *idx,
return ret;
}
-static int add_tracepoint_event(struct list_head **list, int *idx,
+static int add_tracepoint_event(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
return strpbrk(evt_name, "*?") ?
@@ -443,7 +422,7 @@ static int add_tracepoint_event(struct list_head **list, int *idx,
add_tracepoint(list, idx, sys_name, evt_name);
}
-static int add_tracepoint_multi_sys(struct list_head **list, int *idx,
+static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
char *sys_name, char *evt_name)
{
struct dirent *events_ent;
@@ -475,7 +454,7 @@ static int add_tracepoint_multi_sys(struct list_head **list, int *idx,
return ret;
}
-int parse_events_add_tracepoint(struct list_head **list, int *idx,
+int parse_events_add_tracepoint(struct list_head *list, int *idx,
char *sys, char *event)
{
int ret;
@@ -530,7 +509,7 @@ do { \
return 0;
}
-int parse_events_add_breakpoint(struct list_head **list, int *idx,
+int parse_events_add_breakpoint(struct list_head *list, int *idx,
void *ptr, char *type)
{
struct perf_event_attr attr;
@@ -611,7 +590,7 @@ static int config_attr(struct perf_event_attr *attr,
return 0;
}
-int parse_events_add_numeric(struct list_head **list, int *idx,
+int parse_events_add_numeric(struct list_head *list, int *idx,
u32 type, u64 config,
struct list_head *head_config)
{
@@ -644,7 +623,7 @@ static char *pmu_event_name(struct list_head *head_terms)
return NULL;
}
-int parse_events_add_pmu(struct list_head **list, int *idx,
+int parse_events_add_pmu(struct list_head *list, int *idx,
char *name, struct list_head *head_config)
{
struct perf_event_attr attr;
@@ -687,6 +666,7 @@ void parse_events__set_leader(char *name, struct list_head *list)
leader->group_name = name ? strdup(name) : NULL;
}
+/* list_event is assumed to point to malloc'ed memory */
void parse_events_update_lists(struct list_head *list_event,
struct list_head *list_all)
{
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 080f7cf..f1cb4c4 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -85,16 +85,16 @@ void parse_events__free_terms(struct list_head *terms);
int parse_events__modifier_event(struct list_head *list, char *str, bool add);
int parse_events__modifier_group(struct list_head *list, char *event_mod);
int parse_events_name(struct list_head *list, char *name);
-int parse_events_add_tracepoint(struct list_head **list, int *idx,
+int parse_events_add_tracepoint(struct list_head *list, int *idx,
char *sys, char *event);
-int parse_events_add_numeric(struct list_head **list, int *idx,
+int parse_events_add_numeric(struct list_head *list, int *idx,
u32 type, u64 config,
struct list_head *head_config);
-int parse_events_add_cache(struct list_head **list, int *idx,
+int parse_events_add_cache(struct list_head *list, int *idx,
char *type, char *op_result1, char *op_result2);
-int parse_events_add_breakpoint(struct list_head **list, int *idx,
+int parse_events_add_breakpoint(struct list_head *list, int *idx,
void *ptr, char *type);
-int parse_events_add_pmu(struct list_head **list, int *idx,
+int parse_events_add_pmu(struct list_head *list, int *idx,
char *pmu , struct list_head *head_config);
void parse_events__set_leader(char *name, struct list_head *list);
void parse_events_update_lists(struct list_head *list_event,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index afc44c1..4eb67ec 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -22,6 +22,13 @@ do { \
YYABORT; \
} while (0)
+#define ALLOC_LIST(list) \
+do { \
+ list = malloc(sizeof(*list)); \
+ ABORT_ON(!list); \
+ INIT_LIST_HEAD(list); \
+} while (0)
+
static inc_group_count(struct list_head *list,
struct parse_events_evlist *data)
{
@@ -196,9 +203,10 @@ event_pmu:
PE_NAME '/' event_config '/'
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_pmu(&list, &data->idx, $1, $3));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_pmu(list, &data->idx, $1, $3));
parse_events__free_terms($3);
$$ = list;
}
@@ -212,11 +220,12 @@ event_legacy_symbol:
value_sym '/' event_config '/'
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
int type = $1 >> 16;
int config = $1 & 255;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx,
type, config, $3));
parse_events__free_terms($3);
$$ = list;
@@ -225,11 +234,12 @@ value_sym '/' event_config '/'
value_sym sep_slash_dc
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
int type = $1 >> 16;
int config = $1 & 255;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx,
type, config, NULL));
$$ = list;
}
@@ -238,27 +248,30 @@ event_legacy_cache:
PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_cache(&list, &data->idx, $1, $3, $5));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, $5));
$$ = list;
}
|
PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_cache(&list, &data->idx, $1, $3, NULL));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, NULL));
$$ = list;
}
|
PE_NAME_CACHE_TYPE
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_cache(&list, &data->idx, $1, NULL, NULL));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_cache(list, &data->idx, $1, NULL, NULL));
$$ = list;
}
@@ -266,9 +279,10 @@ event_legacy_mem:
PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
(void *) $2, $4));
$$ = list;
}
@@ -276,9 +290,10 @@ PE_PREFIX_MEM PE_VALUE ':' PE_MODIFIER_BP sep_dc
PE_PREFIX_MEM PE_VALUE sep_dc
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_breakpoint(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_breakpoint(list, &data->idx,
(void *) $2, NULL));
$$ = list;
}
@@ -287,9 +302,10 @@ event_legacy_tracepoint:
PE_NAME ':' PE_NAME
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_tracepoint(&list, &data->idx, $1, $3));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_tracepoint(list, &data->idx, $1, $3));
$$ = list;
}
@@ -297,9 +313,10 @@ event_legacy_numeric:
PE_VALUE ':' PE_VALUE
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx, (u32)$1, $3, NULL));
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx, (u32)$1, $3, NULL));
$$ = list;
}
@@ -307,9 +324,10 @@ event_legacy_raw:
PE_RAW
{
struct parse_events_evlist *data = _data;
- struct list_head *list = NULL;
+ struct list_head *list;
- ABORT_ON(parse_events_add_numeric(&list, &data->idx,
+ ALLOC_LIST(list);
+ ABORT_ON(parse_events_add_numeric(list, &data->idx,
PERF_TYPE_RAW, $1, NULL));
$$ = list;
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-07-19 7:46 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-02 19:27 [PATCH 0/6] perf: bug fixes, cleanup of parse events memory allocations David Ahern
2013-07-02 19:27 ` [PATCH 1/6] perf evsel: fix count parameter to read call in event_format__new David Ahern
2013-07-12 8:51 ` [tip:perf/urgent] perf evsel: Fix " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 2/6] perf evlist: fix use of uninitialized variable David Ahern
2013-07-19 7:44 ` [tip:perf/core] perf evlist: Fix " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 3/6] perf event: initialize allocated memory for synthesized events David Ahern
2013-07-11 16:35 ` David Ahern
2013-07-02 19:27 ` [PATCH 4/6] perf: don't free list head in parse_events__free_terms David Ahern
2013-07-19 7:45 ` [tip:perf/core] perf tools: Don' t " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 5/6] perf test: make terms a stack variable in test_term David Ahern
2013-07-19 7:45 ` [tip:perf/core] perf tests: Make " tip-bot for David Ahern
2013-07-02 19:27 ` [PATCH 6/6] perf parse events: demystify memory allocations David Ahern
2013-07-07 15:26 ` Jiri Olsa
2013-07-07 16:45 ` David Ahern
2013-07-07 17:00 ` Jiri Olsa
2013-07-19 7:45 ` [tip:perf/core] perf parse events: Demystify " tip-bot for David Ahern
2013-07-05 14:34 ` [PATCH 0/6] perf: bug fixes, cleanup of parse events " Arnaldo Carvalho de Melo
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.