Messages in this thread | | | Date | Thu, 18 Nov 2021 10:19:50 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH] perf evsel: Fix memory leaks relating to unit |
| |
Em Thu, Nov 18, 2021 at 12:47:49AM -0800, Ian Rogers escreveu: > unit may have a strdup pointer or be to a literal, consequently memory > assocciated with it isn't freed. Change it so the unit is always strdup > and so the memory can be safely freed. Fix related issue in > perf_event__process_event_update for name and own_cpus. Leaks were > spotted by leak sanitizer.
Thanks, applied.
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/tests/event_update.c | 5 ++--- > tools/perf/util/evsel.c | 18 +++++++++--------- > tools/perf/util/header.c | 8 +++++--- > tools/perf/util/parse-events.c | 9 ++++++--- > 4 files changed, 22 insertions(+), 18 deletions(-) > > diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c > index fbb68deba59f..d01532d40acb 100644 > --- a/tools/perf/tests/event_update.c > +++ b/tools/perf/tests/event_update.c > @@ -88,7 +88,6 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes > struct evsel *evsel; > struct event_name tmp; > struct evlist *evlist = evlist__new_default(); > - char *unit = strdup("KRAVA"); > > TEST_ASSERT_VAL("failed to get evlist", evlist); > > @@ -99,7 +98,8 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes > > perf_evlist__id_add(&evlist->core, &evsel->core, 0, 0, 123); > > - evsel->unit = unit; > + free((char *)evsel->unit); > + evsel->unit = strdup("KRAVA"); > > TEST_ASSERT_VAL("failed to synthesize attr update unit", > !perf_event__synthesize_event_update_unit(NULL, evsel, process_event_unit)); > @@ -119,7 +119,6 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes > TEST_ASSERT_VAL("failed to synthesize attr update cpus", > !perf_event__synthesize_event_update_cpus(&tmp.tool, evsel, process_event_cpus)); > > - free(unit); > evlist__delete(evlist); > return 0; > } > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index a59fb2ecb84e..ac0127be0459 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -241,7 +241,7 @@ void evsel__init(struct evsel *evsel, > { > perf_evsel__init(&evsel->core, attr, idx); > evsel->tracking = !idx; > - evsel->unit = ""; > + evsel->unit = strdup(""); > evsel->scale = 1.0; > evsel->max_events = ULONG_MAX; > evsel->evlist = NULL; > @@ -276,13 +276,8 @@ struct evsel *evsel__new_idx(struct perf_event_attr *attr, int idx) > } > > if (evsel__is_clock(evsel)) { > - /* > - * The evsel->unit points to static alias->unit > - * so it's ok to use static string in here. > - */ > - static const char *unit = "msec"; > - > - evsel->unit = unit; > + free((char *)evsel->unit); > + evsel->unit = strdup("msec"); > evsel->scale = 1e-6; > } > > @@ -420,7 +415,11 @@ struct evsel *evsel__clone(struct evsel *orig) > > evsel->max_events = orig->max_events; > evsel->tool_event = orig->tool_event; > - evsel->unit = orig->unit; > + free((char *)evsel->unit); > + evsel->unit = strdup(orig->unit); > + if (evsel->unit == NULL) > + goto out_err; > + > evsel->scale = orig->scale; > evsel->snapshot = orig->snapshot; > evsel->per_pkg = orig->per_pkg; > @@ -1441,6 +1440,7 @@ void evsel__exit(struct evsel *evsel) > zfree(&evsel->group_name); > zfree(&evsel->name); > zfree(&evsel->pmu_name); > + zfree(&evsel->unit); > zfree(&evsel->metric_id); > evsel__zero_per_pkg(evsel); > hashmap__free(evsel->per_pkg_mask); > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index fda8d14c891f..79cce216727e 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -4257,9 +4257,11 @@ int perf_event__process_event_update(struct perf_tool *tool __maybe_unused, > > switch (ev->type) { > case PERF_EVENT_UPDATE__UNIT: > + free((char *)evsel->unit); > evsel->unit = strdup(ev->data); > break; > case PERF_EVENT_UPDATE__NAME: > + free(evsel->name); > evsel->name = strdup(ev->data); > break; > case PERF_EVENT_UPDATE__SCALE: > @@ -4268,11 +4270,11 @@ int perf_event__process_event_update(struct perf_tool *tool __maybe_unused, > break; > case PERF_EVENT_UPDATE__CPUS: > ev_cpus = (struct perf_record_event_update_cpus *)ev->data; > - > map = cpu_map__new_data(&ev_cpus->cpus); > - if (map) > + if (map) { > + perf_cpu_map__put(evsel->core.own_cpus); > evsel->core.own_cpus = map; > - else > + } else > pr_err("failed to get event_update cpus\n"); > default: > break; > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index a2f4c086986f..12d925a6d27f 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -402,8 +402,10 @@ static int add_event_tool(struct list_head *list, int *idx, > if (!evsel) > return -ENOMEM; > evsel->tool_event = tool_event; > - if (tool_event == PERF_TOOL_DURATION_TIME) > - evsel->unit = "ns"; > + if (tool_event == PERF_TOOL_DURATION_TIME) { > + free((char *)evsel->unit); > + evsel->unit = strdup("ns"); > + } > return 0; > } > > @@ -1630,7 +1632,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, > if (parse_state->fake_pmu) > return 0; > > - evsel->unit = info.unit; > + free((char *)evsel->unit); > + evsel->unit = strdup(info.unit); > evsel->scale = info.scale; > evsel->per_pkg = info.per_pkg; > evsel->snapshot = info.snapshot; > -- > 2.34.0.rc1.387.gb447b232ab-goog
--
- Arnaldo
| |