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>

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 7a50ea2ac6b1c0a5001a80e575918089ae2147fe..bbaf2e16b7ae73b108dc39de85621ba9004ce3e3 100644 (file)
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -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);