perf parse-events: Fix propagation of term's no_value when cloning
The no_value field in 'struct parse_events_term' indicates that the val variable isn't used, the case for an event name. Cloning wasn't propagating this, making cloned event name terms appearing to have a constant assinged to them. Working around the bug would check for a value of 1 assigned to value, but then this meant a user value of 1 couldn't be differentiated causing the value to be lost in debug printing and perf list. The change fixes the cloning and updates the "val.num ==/!= 1" tests to use no_value instead. To better check the no_value is set appropriately parameter comments are added for constant values. This found that no_value wasn't set correctly in parse_events_multi_pmu_add, which matters now that no_value is used to indicate an event name. Fixes:7a6e916447
("perf parse-events: Make common term list to strbuf helper") Fixes:99e7138eb7
("perf tools: Fail on using multiple bits long terms without value") Signed-off-by: Ian Rogers <irogers@google.com> Tested-by: Kan Liang <kan.liang@linux.intel.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: James Clark <james.clark@arm.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Rob Herring <robh@kernel.org> Link: https://lore.kernel.org/r/20230831071421.2201358-4-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
This commit is contained in:
parent
58d3a4cea4
commit
64199ae4b8
@ -1510,8 +1510,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
|
||||
|
||||
if (parse_events_term__num(&term,
|
||||
PARSE_EVENTS__TERM_TYPE_USER,
|
||||
config, 1, false, NULL,
|
||||
NULL) < 0) {
|
||||
config, /*num=*/1, /*novalue=*/true,
|
||||
loc, /*loc_val=*/NULL) < 0) {
|
||||
zfree(&config);
|
||||
goto out_err;
|
||||
}
|
||||
@ -2482,7 +2482,7 @@ int parse_events_term__num(struct parse_events_term **term,
|
||||
.err_val = loc_val ? loc_val->first_column : 0,
|
||||
};
|
||||
|
||||
return new_term(term, &temp, NULL, num);
|
||||
return new_term(term, &temp, /*str=*/NULL, num);
|
||||
}
|
||||
|
||||
int parse_events_term__str(struct parse_events_term **term,
|
||||
@ -2501,7 +2501,7 @@ int parse_events_term__str(struct parse_events_term **term,
|
||||
.err_val = loc_val ? loc_val->first_column : 0,
|
||||
};
|
||||
|
||||
return new_term(term, &temp, str, 0);
|
||||
return new_term(term, &temp, str, /*num=*/0);
|
||||
}
|
||||
|
||||
int parse_events_term__term(struct parse_events_term **term,
|
||||
@ -2518,26 +2518,21 @@ int parse_events_term__clone(struct parse_events_term **new,
|
||||
struct parse_events_term *term)
|
||||
{
|
||||
char *str;
|
||||
struct parse_events_term temp = {
|
||||
.type_val = term->type_val,
|
||||
.type_term = term->type_term,
|
||||
.config = NULL,
|
||||
.err_term = term->err_term,
|
||||
.err_val = term->err_val,
|
||||
};
|
||||
struct parse_events_term temp = *term;
|
||||
|
||||
temp.used = false;
|
||||
if (term->config) {
|
||||
temp.config = strdup(term->config);
|
||||
if (!temp.config)
|
||||
return -ENOMEM;
|
||||
}
|
||||
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
|
||||
return new_term(new, &temp, NULL, term->val.num);
|
||||
return new_term(new, &temp, /*str=*/NULL, term->val.num);
|
||||
|
||||
str = strdup(term->val.str);
|
||||
if (!str)
|
||||
return -ENOMEM;
|
||||
return new_term(new, &temp, str, 0);
|
||||
return new_term(new, &temp, str, /*num=*/0);
|
||||
}
|
||||
|
||||
void parse_events_term__delete(struct parse_events_term *term)
|
||||
@ -2611,20 +2606,22 @@ int parse_events_term__to_strbuf(struct list_head *term_list, struct strbuf *sb)
|
||||
first = false;
|
||||
|
||||
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
|
||||
if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER && term->val.num == 1)
|
||||
if (term->no_value) {
|
||||
assert(term->type_term == PARSE_EVENTS__TERM_TYPE_USER);
|
||||
ret = strbuf_addf(sb, "%s", term->config);
|
||||
else
|
||||
} else
|
||||
ret = strbuf_addf(sb, "%s=%#"PRIx64, term->config, term->val.num);
|
||||
else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) {
|
||||
if (term->config) {
|
||||
ret = strbuf_addf(sb, "%s=", term->config);
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
} else if (term->type_term < __PARSE_EVENTS__TERM_TYPE_NR) {
|
||||
} else if ((unsigned int)term->type_term < __PARSE_EVENTS__TERM_TYPE_NR) {
|
||||
ret = strbuf_addf(sb, "%s=", config_term_name(term->type_term));
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
}
|
||||
assert(!term->no_value);
|
||||
ret = strbuf_addf(sb, "%s", term->val.str);
|
||||
}
|
||||
if (ret < 0)
|
||||
|
@ -712,7 +712,7 @@ name_or_raw '=' PE_VALUE
|
||||
{
|
||||
struct parse_events_term *term;
|
||||
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
|
||||
$1, $3, false, &@1, &@3);
|
||||
$1, $3, /*novalue=*/false, &@1, &@3);
|
||||
|
||||
if (err) {
|
||||
free($1);
|
||||
@ -739,7 +739,7 @@ PE_LEGACY_CACHE
|
||||
{
|
||||
struct parse_events_term *term;
|
||||
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE,
|
||||
$1, 1, true, &@1, NULL);
|
||||
$1, /*num=*/1, /*novalue=*/true, &@1, /*loc_val=*/NULL);
|
||||
|
||||
if (err) {
|
||||
free($1);
|
||||
@ -752,7 +752,7 @@ PE_NAME
|
||||
{
|
||||
struct parse_events_term *term;
|
||||
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
|
||||
$1, 1, true, &@1, NULL);
|
||||
$1, /*num=*/1, /*novalue=*/true, &@1, /*loc_val=*/NULL);
|
||||
|
||||
if (err) {
|
||||
free($1);
|
||||
@ -765,7 +765,8 @@ PE_TERM_HW
|
||||
{
|
||||
struct parse_events_term *term;
|
||||
int err = parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_HARDWARE,
|
||||
$1.str, $1.num & 255, false, &@1, NULL);
|
||||
$1.str, $1.num & 255, /*novalue=*/false,
|
||||
&@1, /*loc_val=*/NULL);
|
||||
|
||||
if (err) {
|
||||
free($1.str);
|
||||
|
@ -1396,7 +1396,7 @@ static struct perf_pmu_alias *pmu_find_alias(struct perf_pmu *pmu,
|
||||
return NULL;
|
||||
|
||||
if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) {
|
||||
if (term->val.num != 1)
|
||||
if (!term->no_value)
|
||||
return NULL;
|
||||
if (pmu_find_format(&pmu->format, term->config))
|
||||
return NULL;
|
||||
|
Loading…
Reference in New Issue
Block a user