Re: [PATCH v2 6/9] tracing/filters: Skip second character of '&&' and '||' in calc_stack()

[Steven Rostedt <rostedt@goodmis.org>]

On Sun, 25 Oct 2026 17:38:05 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> In calc_stack(), when encountering '||' or '&&' (where str[i+1] == str[i]),
> the loop sets 'is_pred = false' and continues without advancing 'i' past
> the second character of the operator. On the next iteration, the second '|'
> or '&' sees str[i+1] != str[i], breaks out of the switch statement, and is
> counted as the start of a new predicate (incrementing nr_preds and setting
> is_pred = true).
> 
> Consequently, a trailing logical operator (e.g. 'common_pid == 1 ||') is
> counted as having 2 predicates instead of 1, and any subsequent non-space
> character before a delimiter fails to increment nr_preds.

 "common_pid ||' is an invalid expression.

> 
> Increment 'i' when matching '||' or '&&' so the entire two-character
> operator is skipped.

This is just wrong.

Masami, don't blindly trust AI.

I purposely did it this way. Because && and || creates a new predicate.

 If you have "x||y"
              ^
              i

 Start with i = 0; is_pred = false; nr_preds = 0;

   str[i] == 'x'
   if (!is_pred) {
	nr_preds++;  // nr_preds = 1
        is_pred = true;
   }

 in for loop i++;  // i = 1; str[i] = '|'

   switch (str[i])
   case '|':
      if (str[i+1] != str[i]) // false
      is_pred = false;
      continue;

 in for loop i++; // i = 2; str[i] == '|';

   switch (str[i])
   case '|':
      if (str[i+1] != str[i]) // true
          break;

   if (!is_pred) {
	nr_preds++;  // nr_preds = 2
        is_pred = true;
   }

 in for loop i++; // i = 3; str[i] == 'y';
 
   if (!is_pred) { // false because is_pred == true

Loop ends.

nr_preds == 2. This is correct.

This patch is garbage.

-- Stevwe



> 
> Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/trace/trace_events_filter.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 3bd0b47a627f..bcd37fd6c109 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -2208,6 +2208,7 @@ static int calc_stack(const char *str, int *parens, int *preds, int *err)
>  		case '&':
>  			if (str[i+1] != str[i])
>  				break;
> +			i++;
>  			is_pred = false;
>  			continue;
>  		case '(':