lkml.org 
[lkml]   [2020]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] perf parse: Copy string to perf_evsel_config_term
On Tue, Jan 07, 2020 at 11:18:28AM +0800, Leo Yan wrote:
> perf with CoreSight fails to record trace data with command:
>
> perf record -e cs_etm/@tmc_etr0/u --per-thread ls
> failed to set sink "" on event cs_etm/@tmc_etr0/u with 21 (Is a
> directory)/perf/
>
> This failure is root caused with the commit 1dc925568f01 ("perf
> parse: Add a deep delete for parse event terms").
>
> The log shows, cs_etm fails to parse the sink attribution; cs_etm event
> relies on the event configuration to pass sink name, but the event
> specific configuration data cannot be passed properly with flow:
>
> get_config_terms()
> ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str);
> __t->val.drv_cfg = term->val.str;
> `> __t->val.drv_cfg is assigned to term->val.str;
>
> parse_events_terms__purge()
> parse_events_term__delete()
> zfree(&term->val.str);
> `> term->val.str is freed and assigned to NULL pointer;
>
> cs_etm_set_sink_attr()
> sink = __t->val.drv_cfg;
> `> sink string has been freed.
>
> To fix this issue, in the function get_config_terms(), this patch
> changes from directly assignment pointer value for the strings to
> use strdup() for allocation a new duplicate string for the cases:
>
> perf_evsel_config_term::val.callgraph
> perf_evsel_config_term::val.branch
> perf_evsel_config_term::val.drv_cfg.
>
> Fixes: 1dc925568f01 ("perf parse: Add a deep delete for parse event terms")
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
> tools/perf/util/parse-events.c | 55 +++++++++++++++++++++-------------
> 1 file changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index ed7c008b9c8b..49b26504bee3 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1220,7 +1220,6 @@ static int get_config_terms(struct list_head *head_config,
> struct list_head *head_terms __maybe_unused)
> {
> #define ADD_CONFIG_TERM(__type, __name, __val) \
> -do { \
> struct perf_evsel_config_term *__t; \
> \
> __t = zalloc(sizeof(*__t)); \
> @@ -1229,9 +1228,23 @@ do { \
> \
> INIT_LIST_HEAD(&__t->list); \
> __t->type = PERF_EVSEL__CONFIG_TERM_ ## __type; \
> - __t->val.__name = __val; \
> __t->weak = term->weak; \
> - list_add_tail(&__t->list, head_terms); \
> + list_add_tail(&__t->list, head_terms)
> +
> +#define ADD_CONFIG_TERM_VAL(__type, __name, __val) \
> +do { \
> + ADD_CONFIG_TERM(__type, __name, __val); \
> + __t->val.__name = __val; \
> +} while (0)
> +
> +#define ADD_CONFIG_TERM_STR(__type, __name, __val) \
> +do { \
> + ADD_CONFIG_TERM(__type, __name, __val); \
> + __t->val.__name = strdup(__val); \
> + if (!__t->val.__name) { \
> + zfree(&__t); \
> + return -ENOMEM; \
> + } \
> } while (0)

hum, I did not check yesterday how we release perf_evsel_config_term
objects, but looks like now we need to release those pointers in here:
perf_evsel__free_config_terms

jirka

\
 
 \ /
  Last update: 2020-01-07 10:16    [W:0.077 / U:2.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site