Instead of using multiple flags, make struct btf_id tagged with an
enum value indicating its kind in the context of resolve_btfids.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
tools/bpf/resolve_btfids/main.c | 83 ++++++++++++++++++++++++---------
1 file changed, 60 insertions(+), 23 deletions(-)
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
@@ -98,6 +98,13 @@
# error "Unknown machine endianness!"
#endif
+enum btf_id_kind {
+ BTF_ID_KIND_NONE,
+ BTF_ID_KIND_SYM,
+ BTF_ID_KIND_SET,
+ BTF_ID_KIND_SET8
+};
+
struct btf_id {
struct rb_node rb_node;
char *name;
@@ -105,9 +112,8 @@ struct btf_id {
int id;
int cnt;
};
+ enum btf_id_kind kind;
int addr_cnt;
- bool is_set;
- bool is_set8;
Elf64_Addr addr[ADDR_CNT];
};
@@ -197,8 +203,10 @@ static struct btf_id *btf_id__find(struct rb_root *root, const char *name)
return NULL;
}
-static struct btf_id *
-btf_id__add(struct rb_root *root, char *name, bool unique)
+static struct btf_id *__btf_id__add(struct rb_root *root,
+ char *name,
+ enum btf_id_kind kind,
+ bool unique)
{
struct rb_node **p = &root->rb_node;
struct rb_node *parent = NULL;
@@ -221,12 +229,23 @@ btf_id__add(struct rb_root *root, char *name, bool unique)
if (id) {
pr_debug("adding symbol %s\n", name);
id->name = name;
+ id->kind = kind;
rb_link_node(&id->rb_node, parent, p);
rb_insert_color(&id->rb_node, root);
}
return id;
}
+static inline struct btf_id *btf_id__add(struct rb_root *root, char *name, enum btf_id_kind kind)
+{
+ return __btf_id__add(root, name, kind, false);
+}
+
+static inline struct btf_id *btf_id__add_unique(struct rb_root *root, char *name, enum btf_id_kind kind)
+{
+ return __btf_id__add(root, name, kind, true);
+}
+
static char *get_id(const char *prefix_end)
{
/*
@@ -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;
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);
}
static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
@@ -288,7 +321,7 @@ static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
return NULL;
}
- return btf_id__add(root, id, false);
+ return btf_id__add(root, id, BTF_ID_KIND_SYM);
}
/* Older libelf.h and glibc elf.h might not yet define the ELF compression types. */
@@ -491,35 +524,31 @@ static int symbols_collect(struct object *obj)
id = add_symbol(&obj->funcs, prefix, sizeof(BTF_FUNC) - 1);
/* set8 */
} else if (!strncmp(prefix, BTF_SET8, sizeof(BTF_SET8) - 1)) {
- id = add_set(obj, prefix, true);
+ id = add_set(obj, prefix, BTF_ID_KIND_SET8);
/*
* SET8 objects store list's count, which is encoded
* in symbol's size, together with 'cnt' field hence
* that - 1.
*/
- if (id) {
+ if (id)
id->cnt = sym.st_size / sizeof(uint64_t) - 1;
- id->is_set8 = true;
- }
/* set */
} else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
- id = add_set(obj, prefix, false);
+ id = add_set(obj, prefix, BTF_ID_KIND_SET);
/*
* SET objects store list's count, which is encoded
* in symbol's size, together with 'cnt' field hence
* that - 1.
*/
- if (id) {
+ if (id)
id->cnt = sym.st_size / sizeof(int) - 1;
- id->is_set = true;
- }
} else {
pr_err("FAILED unsupported prefix %s\n", prefix);
return -1;
}
if (!id)
- return -ENOMEM;
+ return -EINVAL;
if (id->addr_cnt >= ADDR_CNT) {
pr_err("FAILED symbol %s crossed the number of allowed lists\n",
@@ -643,7 +672,7 @@ static int id_patch(struct object *obj, struct btf_id *id)
int i;
/* For set, set8, id->id may be 0 */
- if (!id->id && !id->is_set && !id->is_set8) {
+ if (!id->id && id->kind != BTF_ID_KIND_SET && id->kind != BTF_ID_KIND_SET8) {
pr_err("WARN: resolve_btfids: unresolved symbol %s\n", id->name);
warnings++;
}
@@ -696,6 +725,7 @@ static int sets_patch(struct object *obj)
{
Elf_Data *data = obj->efile.idlist;
struct rb_node *next;
+ int cnt;
next = rb_first(&obj->sets);
while (next) {
@@ -715,11 +745,15 @@ static int sets_patch(struct object *obj)
return -1;
}
- if (id->is_set) {
+ switch (id->kind) {
+ case BTF_ID_KIND_SET:
set = data->d_buf + off;
+ cnt = set->cnt;
qsort(set->ids, set->cnt, sizeof(set->ids[0]), cmp_id);
- } else {
+ break;
+ case BTF_ID_KIND_SET8:
set8 = data->d_buf + off;
+ cnt = set8->cnt;
/*
* Make sure id is at the beginning of the pairs
* struct, otherwise the below qsort would not work.
@@ -744,10 +778,13 @@ static int sets_patch(struct object *obj)
bswap_32(set8->pairs[i].flags);
}
}
+ break;
+ default:
+ pr_err("Unexpected btf_id_kind %d for set '%s'\n", id->kind, id->name);
+ return -1;
}
- pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
- off, id->is_set ? set->cnt : set8->cnt, id->name);
+ pr_debug("sorting addr %5lu: cnt %6d [%s]\n", off, cnt, id->name);
next = rb_next(next);
}
--
2.52.0> 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;
> 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
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
On Thu, Dec 18, 2025 at 9:47 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
> 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:
the bug is not in determining prefix length, but in using it to find
where id starts in the string, so the fix should be
id = name + prefixlen;
prefixlen is calculated correctly, IMO
>
> 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
>
On 12/18/25 11:58 AM, Andrii Nakryiko wrote:
> On Thu, Dec 18, 2025 at 9:47 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>>
>> 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:
>
> the bug is not in determining prefix length, but in using it to find
> where id starts in the string, so the fix should be
>
> id = name + prefixlen;
>
> prefixlen is calculated correctly, IMO
Aaahh, because the null byte is counted by sizeof, right... I missed that.
>
>>
>> [...]
>>
© 2016 - 2026 Red Hat, Inc.