* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox