Re: [PATCH bpf-next v3 2/2] selftests/bpf: Match tests against regular expres

[Cupertino Miranda <cupertino.miranda@oracle.com>]

Eduard Zingerman writes:

> On Tue, 2024-06-11 at 18:40 +0100, Cupertino Miranda wrote:
>> This patch changes a few tests to make use of reg
>> would otherwise fail when compiled with GCC.
>>
>> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
>> Cc: jose.marchesi@oracle.com
>> Cc: david.faust@oracle.com
>> Cc: Yonghong Song <yonghong.song@linux.dev>
>> Cc: Eduard Zingerman <eddyz87@gmail.com>
>> Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>> ---
>
> Looks good, but I think that changes for 'off' for three cases below
> are not necessary.
>
> [...]
>
>> diff --git a/tools/testing/selftests/bpf/progs/rbtree_fail.c b/tools/testing/selftests/bpf/progs/rbtree_fail.c
>> index 3fecf1c6dfe5..8399304eca72 100644
>> --- a/tools/testing/selftests/bpf/progs/rbtree_fail.c
>> +++ b/tools/testing/selftests/bpf/progs/rbtree_fail.c
>> @@ -29,7 +29,7 @@ static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b)
>>  }
>>
>>  SEC("?tc")
>> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
>> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>
> This error message is reported in a single place in
> verifier.c:__process_kf_arg_ptr_to_graph_root():
>
> 	if (check_reg_allocation_locked(env, reg)) {
> 		verbose(env, "bpf_spin_lock at off=%d must be held for %s\n",
> 			rec->spin_lock_off, head_type_name);
> 		return -EINVAL;
> 	}
>
> Where `rec` is a description of the BTF type, `off` is an offset
> inside the structure, why do you need to change it to regex?
>
In GCC the off value would print something else.
Judging by the message I deduced that off was refering to an instruction
location and so, tight to the compiler.
Now I see the value is rather tight to BTF content.

I will remove the offset patching from the series and later on evaluate
what is happening in GCC for the result difference.

>
>>  long rbtree_api_nolock_add(void *ctx)
>>  {
>>  	struct node_data *n;
>> @@ -43,7 +43,7 @@ long rbtree_api_nolock_add(void *ctx)
>>  }
>>
>>  SEC("?tc")
>> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
>> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>>  long rbtree_api_nolock_remove(void *ctx)
>>  {
>>  	struct node_data *n;
>> @@ -61,7 +61,7 @@ long rbtree_api_nolock_remove(void *ctx)
>>  }
>>
>>  SEC("?tc")
>> -__failure __msg("bpf_spin_lock at off=16 must be held for bpf_rb_root")
>> +__failure __regex("bpf_spin_lock at off=[0-9]+ must be held for bpf_rb_root")
>>  long rbtree_api_nolock_first(void *ctx)
>>  {
>>  	bpf_rbtree_first(&groot);
>
> [...]