On Mon, Feb 05, 2024 at 07:39:22PM +0100, Viktor Malik wrote:
> Instead of using magic offsets to access BTF ID set data, leverage types
> from btf_ids.h (btf_id_set and btf_id_set8) which define the actual
> layout of the data. Thanks to this change, set sorting should also
> continue working if the layout changes.
>
> This requires to sync the definition of 'struct btf_id_set8' from
> include/linux/btf_ids.h to tools/include/linux/btf_ids.h. We don't sync
> the rest of the file at the moment, b/c that would require to also sync
> multiple dependent headers and we don't need any other defs from
> btf_ids.h.
>
> Signed-off-by: Viktor Malik <vmalik@xxxxxxxxxx>
> ---
> tools/bpf/resolve_btfids/main.c | 29 +++++++++++++++++++----------
> tools/include/linux/btf_ids.h | 9 +++++++++
> 2 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index 27a23196d58e..9ffe8163081a 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -70,6 +70,7 @@
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <errno.h>
> +#include <linux/btf_ids.h>
> #include <linux/rbtree.h>
> #include <linux/zalloc.h>
> #include <linux/err.h>
> @@ -78,7 +79,7 @@
> #include <subcmd/parse-options.h>
>
> #define BTF_IDS_SECTION ".BTF_ids"
> -#define BTF_ID "__BTF_ID__"
> +#define BTF_ID_PREFIX "__BTF_ID__"
>
> #define BTF_STRUCT "struct"
> #define BTF_UNION "union"
> @@ -161,7 +162,7 @@ static int eprintf(int level, int var, const char *fmt, ...)
>
> static bool is_btf_id(const char *name)
> {
> - return name && !strncmp(name, BTF_ID, sizeof(BTF_ID) - 1);
> + return name && !strncmp(name, BTF_ID_PREFIX, sizeof(BTF_ID_PREFIX) - 1);
> }
>
> static struct btf_id *btf_id__find(struct rb_root *root, const char *name)
> @@ -441,7 +442,7 @@ static int symbols_collect(struct object *obj)
> * __BTF_ID__TYPE__vfs_truncate__0
> * prefix = ^
> */
> - prefix = name + sizeof(BTF_ID) - 1;
> + prefix = name + sizeof(BTF_ID_PREFIX) - 1;
>
> /* struct */
> if (!strncmp(prefix, BTF_STRUCT, sizeof(BTF_STRUCT) - 1)) {
> @@ -654,10 +655,10 @@ static int sets_patch(struct object *obj)
>
> next = rb_first(&obj->sets);
> while (next) {
> + struct btf_id_set8 *set8;
> + struct btf_id_set *set;
> unsigned long addr, idx;
> struct btf_id *id;
> - int *base;
> - int cnt;
>
> id = rb_entry(next, struct btf_id, rb_node);
> addr = id->addr[0];
> @@ -671,13 +672,21 @@ static int sets_patch(struct object *obj)
> }
>
> idx = idx / sizeof(int);
> - base = &ptr[idx] + (id->is_set8 ? 2 : 1);
> - cnt = ptr[idx];
> + if (id->is_set) {
> + set = (struct btf_id_set *)&ptr[idx];
Nit: should be able to simplify logic a bit like this:
int off = addr - obj->efile.idlist_addr;
set8 = data->d_buf + off;
Don't think that `idx`, `ptr` or casts are necessary anymore.
> + qsort(set->ids, set->cnt, sizeof(set->ids[0]), cmp_id);
> + } else {
> + set8 = (struct btf_id_set8 *)&ptr[idx];
> + /*
> + * Make sure id is at the beginning of the pairs
> + * struct, otherwise the below qsort would not work.
> + */
> + BUILD_BUG_ON(set8->pairs != &set8->pairs[0].id);
> + qsort(set8->pairs, set8->cnt, sizeof(set8->pairs[0]), cmp_id);
> + }
>
> pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
> - (idx + 1) * sizeof(int), cnt, id->name);
> -
> - qsort(base, cnt, id->is_set8 ? sizeof(uint64_t) : sizeof(int), cmp_id);
> + (idx + 1) * sizeof(int), id->is_set ? set->cnt : set8->cnt, id->name);
>
> next = rb_next(next);
> }
> diff --git a/tools/include/linux/btf_ids.h b/tools/include/linux/btf_ids.h
> index 2f882d5cb30f..72535f00572f 100644
> --- a/tools/include/linux/btf_ids.h
> +++ b/tools/include/linux/btf_ids.h
> @@ -8,6 +8,15 @@ struct btf_id_set {
> u32 ids[];
> };
>
> +struct btf_id_set8 {
> + u32 cnt;
> + u32 flags;
> + struct {
> + u32 id;
> + u32 flags;
> + } pairs[];
> +};
> +
> #ifdef CONFIG_DEBUG_INFO_BTF
>
> #include <linux/compiler.h> /* for __PASTE */
> --
> 2.43.0
>
