From: Leo Yan <leo.yan@linaro.org>
To: Arnaldo Carvalho de Melo <acme@redhat.com>,
Jiri Olsa <jolsa@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Namhyung Kim <namhyung@kernel.org>,
Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Andi Kleen <ak@linux.intel.com>,
linux-kernel@vger.kernel.org,
Mathieu Poirier <mathieu.poirier@linaro.org>,
Mike Leach <mike.leach@linaro.org>,
Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Subject: [PATCH v2] perf parse: Copy string to perf_evsel_config_term
Date: Tue, 7 Jan 2020 11:18:28 +0800 [thread overview]
Message-ID: <20200107031828.23103-1-leo.yan@linaro.org> (raw)
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)
struct parse_events_term *term;
@@ -1239,53 +1252,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,
- term->val.num ? true : false);
+ 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 +1335,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
next reply other threads:[~2020-01-07 3:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-07 3:18 Leo Yan [this message]
2020-01-07 9:16 ` [PATCH v2] perf parse: Copy string to perf_evsel_config_term Jiri Olsa
2020-01-07 10:09 ` Leo Yan
2020-01-07 10:28 ` Jiri Olsa
2020-01-07 11:42 ` Leo Yan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200107031828.23103-1-leo.yan@linaro.org \
--to=leo.yan@linaro.org \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.poirier@linaro.org \
--cc=mike.leach@linaro.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).