Messages in this thread | | | Date | Thu, 2 Jan 2020 23:26:41 +0800 | From | Leo Yan <> | Subject | Re: [PATCH] perf parse: Copy string to perf_evsel_config_term |
| |
On Thu, Jan 02, 2020 at 11:13:26PM +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>
Let me to loop Mathieu/Mike/Suzuki, sorry for spamming.
Thanks, Leo
> --- > tools/perf/util/parse-events.c | 49 ++++++++++++++++++++-------------- > 1 file changed, 29 insertions(+), 20 deletions(-) > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index ed7c008b9c8b..5972acdd40d6 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,19 @@ 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); \ > } while (0) > > struct parse_events_term *term; > @@ -1239,53 +1248,53 @@ do { \ > list_for_each_entry(term, head_config, list) { > switch (term->type_term) { > case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD: > - ADD_CONFIG_TERM(PERIOD, period, term->val.num); > + ADD_CONFIG_TERM_VAL(PERIOD, period, term->val.num); > break; > case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ: > - ADD_CONFIG_TERM(FREQ, freq, term->val.num); > + ADD_CONFIG_TERM_VAL(FREQ, freq, term->val.num); > break; > case PARSE_EVENTS__TERM_TYPE_TIME: > - ADD_CONFIG_TERM(TIME, time, term->val.num); > + ADD_CONFIG_TERM_VAL(TIME, time, term->val.num); > break; > case PARSE_EVENTS__TERM_TYPE_CALLGRAPH: > - ADD_CONFIG_TERM(CALLGRAPH, callgraph, term->val.str); > + ADD_CONFIG_TERM_STR(CALLGRAPH, callgraph, term->val.str); > break; > case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE: > - ADD_CONFIG_TERM(BRANCH, branch, term->val.str); > + ADD_CONFIG_TERM_STR(BRANCH, branch, term->val.str); > break; > case PARSE_EVENTS__TERM_TYPE_STACKSIZE: > - ADD_CONFIG_TERM(STACK_USER, stack_user, term->val.num); > + ADD_CONFIG_TERM_VAL(STACK_USER, stack_user, term->val.num); > break; > case PARSE_EVENTS__TERM_TYPE_INHERIT: > - ADD_CONFIG_TERM(INHERIT, inherit, term->val.num ? 1 : 0); > + ADD_CONFIG_TERM_VAL(INHERIT, inherit, term->val.num ? 1 : 0); > break; > case PARSE_EVENTS__TERM_TYPE_NOINHERIT: > - ADD_CONFIG_TERM(INHERIT, inherit, term->val.num ? 0 : 1); > + ADD_CONFIG_TERM_VAL(INHERIT, inherit, term->val.num ? 0 : 1); > break; > case PARSE_EVENTS__TERM_TYPE_MAX_STACK: > - ADD_CONFIG_TERM(MAX_STACK, max_stack, term->val.num); > + ADD_CONFIG_TERM_VAL(MAX_STACK, max_stack, term->val.num); > break; > case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS: > - ADD_CONFIG_TERM(MAX_EVENTS, max_events, term->val.num); > + ADD_CONFIG_TERM_VAL(MAX_EVENTS, max_events, term->val.num); > break; > case PARSE_EVENTS__TERM_TYPE_OVERWRITE: > - ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 1 : 0); > + ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite, term->val.num ? 1 : 0); > break; > case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE: > - ADD_CONFIG_TERM(OVERWRITE, overwrite, term->val.num ? 0 : 1); > + ADD_CONFIG_TERM_VAL(OVERWRITE, overwrite, term->val.num ? 0 : 1); > break; > case PARSE_EVENTS__TERM_TYPE_DRV_CFG: > - ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str); > + ADD_CONFIG_TERM_STR(DRV_CFG, drv_cfg, term->val.str); > break; > case PARSE_EVENTS__TERM_TYPE_PERCORE: > - ADD_CONFIG_TERM(PERCORE, percore, > + ADD_CONFIG_TERM_VAL(PERCORE, percore, > term->val.num ? true : false); > break; > case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT: > - ADD_CONFIG_TERM(AUX_OUTPUT, aux_output, term->val.num ? 1 : 0); > + ADD_CONFIG_TERM_VAL(AUX_OUTPUT, aux_output, term->val.num ? 1 : 0); > break; > case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE: > - ADD_CONFIG_TERM(AUX_SAMPLE_SIZE, aux_sample_size, term->val.num); > + ADD_CONFIG_TERM_VAL(AUX_SAMPLE_SIZE, aux_sample_size, term->val.num); > break; > default: > break; > @@ -1322,7 +1331,7 @@ static int get_config_chgs(struct perf_pmu *pmu, struct list_head *head_config, > } > > if (bits) > - ADD_CONFIG_TERM(CFG_CHG, cfg_chg, bits); > + ADD_CONFIG_TERM_VAL(CFG_CHG, cfg_chg, bits); > > #undef ADD_CONFIG_TERM > return 0; > -- > 2.17.1 >
| |