Re: [PATCH bpf-next v4 3/8] resolve_btfids: Introduce enum btf_id_kind

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 12/17/25 4:47 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
>> index b4caae1170dd..da8c7d127632 100644
>> --- a/tools/bpf/resolve_btfids/main.c
>> +++ b/tools/bpf/resolve_btfids/main.c
> 
> [ ... ]
> 
>> @@ -260,22 +279,36 @@ static char *get_id(const char *prefix_end)
>>  	return id;
>>  }
>>
>> -static struct btf_id *add_set(struct object *obj, char *name, bool is_set8)
>> +static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind kind)
>>  {
>> +	int len = strlen(name);
>> +	int prefixlen;
>> +	char *id;
>> +
>>  	/*
>>  	 * __BTF_ID__set__name
>>  	 * name =    ^
>>  	 * id   =         ^
>>  	 */
>> -	char *id = name + (is_set8 ? sizeof(BTF_SET8 "__") : sizeof(BTF_SET "__")) - 1;
>> -	int len = strlen(name);
>> +	switch (kind) {
>> +	case BTF_ID_KIND_SET:
>> +		prefixlen = sizeof(BTF_SET "__") - 1;
>> +		break;
>> +	case BTF_ID_KIND_SET8:
>> +		prefixlen = sizeof(BTF_SET8 "__") - 1;
>> +		break;
>> +	default:
>> +		pr_err("Unexpected kind %d passed to %s() for symbol %s\n", kind, __func__, name);
>> +		return NULL;
>> +	}
>> +
>> +	id = name + prefixlen - 1;
> 
> Is this off by one? The original code computed:
> 
>     id = name + sizeof(BTF_SET "__") - 1
> 
> For BTF_SET ("set"), this gives: name + sizeof("set__") - 1 = name + 6 - 1 = name + 5
> 
> The new code computes:
> 
>     prefixlen = sizeof(BTF_SET "__") - 1 = 6 - 1 = 5
>     id = name + prefixlen - 1 = name + 5 - 1 = name + 4
> 
> The -1 appears to be applied twice now, once in the prefixlen assignment and
> again when computing id. Should this line be:
> 
>     id = name + prefixlen;

Yes, this is an off-by-one bug.

Good catch, Opus 4.5. Sonnet 4.5 missed this in v3.

This was "harmless", since the names stored in the trees don't affect
the output.  The comparison between the names still works, as they all
simply got a "_" prefix.  But this only makes the bug sneaky, which is
very bad.

The suggested fix is meh though, IMO a better one is:

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 3e88dc862d87..ffc15c249ed4 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -295,10 +295,10 @@ static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind k
         */
        switch (kind) {
        case BTF_ID_KIND_SET:
-               prefixlen = sizeof(BTF_SET "__") - 1;
+               prefixlen = sizeof(BTF_SET "__");
                break;
        case BTF_ID_KIND_SET8:
-               prefixlen = sizeof(BTF_SET8 "__") - 1;
+               prefixlen = sizeof(BTF_SET8 "__");
                break;
        default:
                pr_err("Unexpected kind %d passed to %s() for symbol %s\n", kind, __func__, name);

> 
>>  	if (id >= name + len) {
>>  		pr_err("FAILED to parse set name: %s\n", name);
>>  		return NULL;
>>  	}
>>
>> -	return btf_id__add(&obj->sets, id, true);
>> +	return btf_id__add_unique(&obj->sets, id, kind);
>>  }
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20321956014