Commit c5eac6ee authored by Kalesh Singh's avatar Kalesh Singh Committed by Steven Rostedt (VMware)
Browse files

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: default avatarKalesh Singh <kaleshsingh@google.com>
Suggested-by: default avatarSteven Rostedt <rostedt@goodmis.org>
Signed-off-by: default avatarSteven Rostedt (VMware) <rostedt@goodmis.org>
parent 9710b2f3

kernel/trace/trace_events_hist.c

+17 −26
Original line number Diff line number Diff line
@@ -68,8 +68,7 @@ 
	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(EXPECT_NUMBER,	"Expecting numeric literal"),		\

	C(UNARY_MINUS_SUBEXPR,	"Unary minus not supported in sub-expressions"), \

	C(SYM_OFFSET_SUBEXPR,	".sym-offset not supported in sub-expressions"),

	C(UNARY_MINUS_SUBEXPR,	"Unary minus not supported in sub-expressions"),



#undef C

#define C(a, b)		HIST_ERR_##a
@@ -1672,10 +1671,6 @@ static int contains_operator(char *str, char **sep) 
	 */

	minus_op = strrchr(str, '-');

	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

		 * 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;

		else if (strcmp(modifier, "sym") == 0)

			*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;

		else if ((strcmp(modifier, "execname") == 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);

	}



	/*

	 * ".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);



	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;

	const char *se_name;

	bool remove = false;

	char *trigger, *p;

	char *trigger, *p, *start;

	int ret = 0;



	lockdep_assert_held(&event_mutex);
@@ -6047,6 +6028,16 @@ static int event_hist_trigger_func(struct event_command *cmd_ops, 
		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);

	if (IS_ERR(attrs))

		return PTR_ERR(attrs);