Re: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats

On Mon Aug 3, 2026 at 1:04 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit changes the verifier's 'stack depth' log format from a single-line format (`stack depth %d+%d...+%d max %d`) to a multi-line format (`stack depth max %d` followed by per-subprogram lines). However, it fails to update the `veristat` tool in the BPF selftests, which relies on parsing the old format to collect stack depth statistics. When `veristat` attempts to parse the new verifier log, the hardcoded `sscanf` string (`"stack depth %511s max %ld"`) matches only the first word (`"max"`) but fails on the rest, leading to broken and unpopulated `STACK` and `MAX_STACK` metrics.
> --
>
> --- Patch [1]: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats ---
> commit f4a761d5dad941fea12558fab4093ec6ef3e52da
> Author: Kumar Kartikeya Dwivedi <[email protected]>
>
> bpf: Show more useful info in stack depth stats
>
> This commit updates the BPF verifier's stack depth statistics logging to print
> the maximum stack depth and each subprogram's stack depth on separate lines.
> It also updates various selftests to match the new output format.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index e6f35f4e715b6..74a92036d84e7 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18703,10 +18703,15 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>>  	if (env->log.level & BPF_LOG_STATS) {
>>  		verbose(env, "verification time %lld usec\n",
>>  			div_u64(env->verification_time, 1000));
>> -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
>> -		for (i = 1; i < subprog_cnt; i++)
>> -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
>> -		verbose(env, " max %d\n", env->max_stack_depth);
>> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);
>> +		for (i = 0; i < subprog_cnt; i++) {
>> +			const char *name = env->subprog_info[i].name;
>> +
>> +			if (!name || !name[0])
>> +				name = "<unknown>";
>> +			verbose(env, "stack depth subprog %d %s %d\n", i, name,
>> +				env->subprog_info[i].stack_depth);
>> +		}
>
> [Severity: Medium]
> Will this break the veristat tool's ability to collect stack depth statistics?
>
> Looking at parse_verif_log() in tools/testing/selftests/bpf/veristat.c, the
> parser still expects the old single-line format:
>
>     if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
>         continue;
>
>     while ((token = strtok_r(cnt++ ? NULL : stack, "+", &state))) {
>         if (sscanf(token, "%d", &sub_stack) == 0)
>             break;
>         s->stats[STACK] += sub_stack;
>     }
>
> With the new multi-line format emitted here, it appears this parsing logic
> will silently fail to extract the stack depth numbers, leaving the STACK and
> MAX_STACK metrics unpopulated when comparing regressions. Does veristat.c
> need to be updated to match the new verifier output format?

Hi Roman,

I think Sashiko should look at patches later in the stack to see whether the
issue it is pointing out will already be addressed later. Bisection related
concerns are different, but this (and many others) do not fall in that category.
This is a recurring source of various reports now. It isn't a big deal, but
would be good to improve upon.

Thanks