tracing/histogram: Simplify handling of .sym-offset in expressions

The '-' in .sym-offset can confuse the hist trigger arithmetic
expression parsing. Simplify the handling of this by replacing the
'sym-offset' with 'symXoffset'. This allows us to correctly evaluate
expressions where the user may have inadvertently added a .sym-offset
modifier to one of the operands in an expression, instead of bailing
out. In this case the .sym-offset has no effect on the evaluation of the
expression. The only valid use of the .sym-offset is as a hist key
modifier.

Link: https://lkml.kernel.org/r/20211025200852.3002369-5-kaleshsingh@google.com

Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
This commit is contained in:
Kalesh Singh 2021-10-25 13:08:36 -07:00 committed by Steven Rostedt (VMware)
parent 9710b2f341
commit c5eac6ee8b

View File

@ -68,8 +68,7 @@
C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \ C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \
C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), \ C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), \
C(EXPECT_NUMBER, "Expecting numeric literal"), \ C(EXPECT_NUMBER, "Expecting numeric literal"), \
C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"), \ C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"),
C(SYM_OFFSET_SUBEXPR, ".sym-offset not supported in sub-expressions"),
#undef C #undef C
#define C(a, b) HIST_ERR_##a #define C(a, b) HIST_ERR_##a
@ -1672,10 +1671,6 @@ static int contains_operator(char *str, char **sep)
*/ */
minus_op = strrchr(str, '-'); minus_op = strrchr(str, '-');
if (minus_op) { if (minus_op) {
/* Unfortunately, the modifier ".sym-offset" can confuse things. */
if (minus_op - str >= 4 && !strncmp(minus_op - 4, ".sym-offset", 11))
goto out;
/* /*
* Unary minus is not supported in sub-expressions. If * Unary minus is not supported in sub-expressions. If
* present, it is always the next root operator. * present, it is always the next root operator.
@ -2138,7 +2133,11 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
*flags |= HIST_FIELD_FL_HEX; *flags |= HIST_FIELD_FL_HEX;
else if (strcmp(modifier, "sym") == 0) else if (strcmp(modifier, "sym") == 0)
*flags |= HIST_FIELD_FL_SYM; *flags |= HIST_FIELD_FL_SYM;
else if (strcmp(modifier, "sym-offset") == 0) /*
* 'sym-offset' occurrences in the trigger string are modified
* to 'symXoffset' to simplify arithmetic expression parsing.
*/
else if (strcmp(modifier, "symXoffset") == 0)
*flags |= HIST_FIELD_FL_SYM_OFFSET; *flags |= HIST_FIELD_FL_SYM_OFFSET;
else if ((strcmp(modifier, "execname") == 0) && else if ((strcmp(modifier, "execname") == 0) &&
(strcmp(field_name, "common_pid") == 0)) (strcmp(field_name, "common_pid") == 0))
@ -2463,24 +2462,6 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
return ERR_PTR(-EINVAL); return ERR_PTR(-EINVAL);
} }
/*
* ".sym-offset" in expressions has no effect on their evaluation,
* but can confuse operator parsing.
*/
if (*n_subexprs == 0) {
sep = strstr(str, ".sym-offset");
if (sep) {
*sep = '\0';
if (strpbrk(str, "+-/*") || strpbrk(sep + 11, "+-/*")) {
*sep = '.';
hist_err(file->tr, HIST_ERR_SYM_OFFSET_SUBEXPR,
errpos(sep));
return ERR_PTR(-EINVAL);
}
*sep = '.';
}
}
field_op = contains_operator(str, &sep); field_op = contains_operator(str, &sep);
if (field_op == FIELD_OP_NONE) if (field_op == FIELD_OP_NONE)
@ -5999,7 +5980,7 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
struct synth_event *se; struct synth_event *se;
const char *se_name; const char *se_name;
bool remove = false; bool remove = false;
char *trigger, *p; char *trigger, *p, *start;
int ret = 0; int ret = 0;
lockdep_assert_held(&event_mutex); lockdep_assert_held(&event_mutex);
@ -6047,6 +6028,16 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
trigger = strstrip(trigger); trigger = strstrip(trigger);
} }
/*
* To simplify arithmetic expression parsing, replace occurrences of
* '.sym-offset' modifier with '.symXoffset'
*/
start = strstr(trigger, ".sym-offset");
while (start) {
*(start + 4) = 'X';
start = strstr(start + 11, ".sym-offset");
};
attrs = parse_hist_trigger_attrs(file->tr, trigger); attrs = parse_hist_trigger_attrs(file->tr, trigger);
if (IS_ERR(attrs)) if (IS_ERR(attrs))
return PTR_ERR(attrs); return PTR_ERR(attrs);