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 issu=
e(s) to consider:
> - [Medium] The commit changes the verifier's 'stack depth' log format fro=
m a single-line format (`stack depth %d+%d...+%d max %d`) to a multi-line f=
ormat (`stack depth max %d` followed by per-subprogram lines). However, it =
fails to update the `veristat` tool in the BPF selftests, which relies on p=
arsing the old format to collect stack depth statistics. When `veristat` at=
tempts to parse the new verifier log, the hardcoded `sscanf` string (`"stac=
k depth %511s max %ld"`) matches only the first word (`"max"`) but fails on=
 the rest, leading to broken and unpopulated `STACK` and `MAX_STACK` metric=
s.
> --
>
> --- Patch [1]: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stac=
k 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 lin=
es.
> 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 bp=
f_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 =3D 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 =3D 0; i < subprog_cnt; i++) {
>> +			const char *name =3D env->subprog_info[i].name;
>> +
>> +			if (!name || !name[0])
>> +				name =3D "<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 statis=
tics?
>
> Looking at parse_verif_log() in tools/testing/selftests/bpf/veristat.c, t=
he
> parser still expects the old single-line format:
>
>     if (2 =3D=3D sscanf(cur, "stack depth %511s max %ld", stack, &s->stat=
s[MAX_STACK]))
>         continue;
>
>     while ((token =3D strtok_r(cnt++ ? NULL : stack, "+", &state))) {
>         if (sscanf(token, "%d", &sub_stack) =3D=3D 0)
>             break;
>         s->stats[STACK] +=3D sub_stack;
>     }
>
> With the new multi-line format emitted here, it appears this parsing logi=
c
> 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 th=
e
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 cate=
gory.
This is a recurring source of various reports now. It isn't a big deal, but
would be good to improve upon.

Thanks