[PATCH dwarves v2 0/1] btf_encoder: handle .BTF_ids section endianness

[PATCH dwarves v2 0/1] btf_encoder: handle .BTF_ids section endianness

[Eduard Zingerman]

Currently, pahole does not generate kfunc declaration tags when
generating BTF for ELF files with an endianness different from the
host system. For example, this issue occurs when processing a vmlinux
built for s390 on an x86 host.

To reproduce the bug:
- follow the instructions in [0] to build an s390 vmlinux;
- generate BTF requesting declaration tags for kfuncs:
  $ pahole --btf_features_strict=decl_tag_kfuncs,decl_tag \
           --btf_encode_detached=test.btf vmlinux
- observe that no kfuncs are generated:
  $ bpftool btf dump file test.btf format c | grep __ksym

This patch resolves the issue by adding the necessary byte-swapping
operations.

Changelog:
- v1 [1] -> v2:
  - avoid modifying the 'idlist' Elf_Data object directly.
    Instead, use struct local_elf_data (suggested by Jiri);
  - update the description of the .BTF_ids section (suggested by Jiri).

[0] https://docs.kernel.org/bpf/s390.html
[1] https://lore.kernel.org/dwarves/20241122070218.3832680-1-eddyz87@gmail.com/

Cc: Alan Maguire <alan.maguire@oracle.com>
Cc: Daniel Xu <dxu@dxuuu.xyz>
Cc: Jiri Olsa <olsajiri@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Vadim Fedorenko <vadfed@meta.com>

Eduard Zingerman (1):
  btf_encoder: handle .BTF_ids section endianness

 btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 6 deletions(-)

-- 
2.47.0

[PATCH dwarves v2 1/1] btf_encoder: handle .BTF_ids section endianness

[Eduard Zingerman]

btf_encoder__tag_kfuncs() reads .BTF_ids section to identify a set of
kfuncs present in the ELF file being processed.
This section consists of:
- arrays of uint32_t elements;
- arrays of records with the following structure:
  struct btf_id_and_flag {
      uint32_t id;
      uint32_t flags;
  };

When endianness of a binary operated by pahole differs from the host
system's endianness, these fields require byte-swapping before use.
Currently, this byte-swapping does not occur, resulting in kfuncs not
being marked with declaration tags.

This commit resolves the issue by introducing an endianness conversion
step for the .BTF_ids section data before the main processing stage.
Since the ELF file is opened in O_RDONLY mode, gelf_xlatetom()
cannot be used for endianness conversion.
Instead, a new type is introduced:

  struct local_elf_data {
	void *d_buf;
	size_t d_size;
	int64_t d_off;
	bool owns_buf;
  };

This structure is populated from the Elf_Data object representing
the .BTF_ids section. When byte-swapping is required, a local copy
of d_buf is created.

Cc: Alan Maguire <alan.maguire@oracle.com>
Cc: Daniel Xu <dxu@dxuuu.xyz>
Cc: Jiri Olsa <olsajiri@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Vadim Fedorenko <vadfed@meta.com>
Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF")
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index e1adddf..06d4a61 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -33,6 +33,7 @@
 #include <stdint.h>
 #include <search.h> /* for tsearch(), tfind() and tdestroy() */
 #include <pthread.h>
+#include <byteswap.h>
 
 #define BTF_IDS_SECTION		".BTF_ids"
 #define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
@@ -145,6 +146,14 @@ struct btf_kfunc_set_range {
 	uint64_t end;
 };
 
+/* Like Elf_Data, but when there is a need to change the data read from ELF */
+struct local_elf_data {
+	void *d_buf;
+	size_t d_size;
+	int64_t d_off;
+	bool owns_buf;
+};
+
 static LIST_HEAD(encoders);
 static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -1681,7 +1690,8 @@ out:
 }
 
 /* Returns if `sym` points to a kfunc set */
-static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr)
+static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, struct local_elf_data *idlist,
+			    size_t idlist_addr)
 {
 	void *ptr = idlist->d_buf;
 	struct btf_id_set8 *set;
@@ -1847,13 +1857,52 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
 	return 0;
 }
 
+/* If byte order of 'elf' differs from current byte order, convert the data->d_buf.
+ * ELF file is opened in a readonly mode, so data->d_buf cannot be modified in place.
+ * Instead, allocate a new buffer if modification is necessary.
+ */
+static int convert_idlist_endianness(Elf *elf, Elf_Data *src, struct local_elf_data *dst)
+{
+	int byteorder, i;
+	char *elf_ident;
+	uint32_t *tmp;
+
+	dst->d_size = src->d_size;
+	dst->d_off = src->d_off;
+	elf_ident = elf_getident(elf, NULL);
+	if (elf_ident == NULL) {
+		fprintf(stderr, "Cannot get ELF identification from header\n");
+		return -EINVAL;
+	}
+	byteorder = elf_ident[EI_DATA];
+	if ((BYTE_ORDER == LITTLE_ENDIAN && byteorder == ELFDATA2LSB)
+	    || (BYTE_ORDER == BIG_ENDIAN && byteorder == ELFDATA2MSB)) {
+		dst->d_buf = src->d_buf;
+		dst->owns_buf = false;
+		return 0;
+	}
+	tmp = malloc(src->d_size);
+	if (tmp == NULL) {
+		fprintf(stderr, "Cannot allocate %lu bytes of memory\n", src->d_size);
+		return -ENOMEM;
+	}
+	memcpy(tmp, src->d_buf, src->d_size);
+	dst->d_buf = tmp;
+	dst->owns_buf = true;
+
+	/* .BTF_ids sections consist of u32 objects */
+	for (i = 0; i < dst->d_size / sizeof(uint32_t); i++)
+		tmp[i] = bswap_32(tmp[i]);
+	return 0;
+}
+
 static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 {
 	const char *filename = encoder->source_filename;
 	struct gobuffer btf_kfunc_ranges = {};
+	struct local_elf_data idlist = {};
 	struct gobuffer btf_funcs = {};
 	Elf_Data *symbols = NULL;
-	Elf_Data *idlist = NULL;
 	Elf_Scn *symscn = NULL;
 	int symbols_shndx = -1;
 	size_t idlist_addr = 0;
@@ -1918,7 +1967,9 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 		} else if (!strcmp(secname, BTF_IDS_SECTION)) {
 			idlist_shndx = i;
 			idlist_addr = shdr.sh_addr;
-			idlist = data;
+			err = convert_idlist_endianness(elf, data, &idlist);
+			if (err < 0)
+				goto out;
 		}
 	}
 
@@ -1960,7 +2011,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 			continue;
 
 		name = elf_strptr(elf, strtabidx, sym.st_name);
-		if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr))
+		if (!is_sym_kfunc_set(&sym, name, &idlist, idlist_addr))
 			continue;
 
 		range.start = sym.st_value;
@@ -2003,13 +2054,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 			if (ranges[j].start <= addr && addr < ranges[j].end) {
 				found = true;
 				off = addr - idlist_addr;
-				if (off < 0 || off + sizeof(*pair) > idlist->d_size) {
+				if (off < 0 || off + sizeof(*pair) > idlist.d_size) {
 					fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n",
 						__func__, func, BTF_IDS_SECTION);
 					free(func);
 					goto out;
 				}
-				pair = idlist->d_buf + off;
+				pair = idlist.d_buf + off;
 				break;
 			}
 		}
@@ -2031,6 +2082,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 out:
 	__gobuffer__delete(&btf_funcs);
 	__gobuffer__delete(&btf_kfunc_ranges);
+	if (idlist.owns_buf)
+		free(idlist.d_buf);
 	if (elf)
 		elf_end(elf);
 	if (fd != -1)
-- 
2.47.0

Re: [PATCH dwarves v2 1/1] btf_encoder: handle .BTF_ids section endianness

[Jiri Olsa]

On Fri, Nov 22, 2024 at 01:44:31PM -0800, Eduard Zingerman wrote:
> btf_encoder__tag_kfuncs() reads .BTF_ids section to identify a set of
> kfuncs present in the ELF file being processed.
> This section consists of:
> - arrays of uint32_t elements;
> - arrays of records with the following structure:
>   struct btf_id_and_flag {
>       uint32_t id;
>       uint32_t flags;
>   };
> 
> When endianness of a binary operated by pahole differs from the host
> system's endianness, these fields require byte-swapping before use.
> Currently, this byte-swapping does not occur, resulting in kfuncs not
> being marked with declaration tags.
> 
> This commit resolves the issue by introducing an endianness conversion
> step for the .BTF_ids section data before the main processing stage.
> Since the ELF file is opened in O_RDONLY mode, gelf_xlatetom()
> cannot be used for endianness conversion.
> Instead, a new type is introduced:
> 
>   struct local_elf_data {
> 	void *d_buf;
> 	size_t d_size;
> 	int64_t d_off;
> 	bool owns_buf;
>   };
> 
> This structure is populated from the Elf_Data object representing
> the .BTF_ids section. When byte-swapping is required, a local copy
> of d_buf is created.
> 
> Cc: Alan Maguire <alan.maguire@oracle.com>
> Cc: Daniel Xu <dxu@dxuuu.xyz>
> Cc: Jiri Olsa <olsajiri@gmail.com>
> Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Cc: Vadim Fedorenko <vadfed@meta.com>
> Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF")
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 59 insertions(+), 6 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index e1adddf..06d4a61 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -33,6 +33,7 @@
>  #include <stdint.h>
>  #include <search.h> /* for tsearch(), tfind() and tdestroy() */
>  #include <pthread.h>
> +#include <byteswap.h>
>  
>  #define BTF_IDS_SECTION		".BTF_ids"
>  #define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
> @@ -145,6 +146,14 @@ struct btf_kfunc_set_range {
>  	uint64_t end;
>  };
>  
> +/* Like Elf_Data, but when there is a need to change the data read from ELF */
> +struct local_elf_data {
> +	void *d_buf;
> +	size_t d_size;
> +	int64_t d_off;
> +	bool owns_buf;
> +};
> +
>  static LIST_HEAD(encoders);
>  static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
>  
> @@ -1681,7 +1690,8 @@ out:
>  }
>  
>  /* Returns if `sym` points to a kfunc set */
> -static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr)
> +static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, struct local_elf_data *idlist,
> +			    size_t idlist_addr)
>  {
>  	void *ptr = idlist->d_buf;
>  	struct btf_id_set8 *set;
> @@ -1847,13 +1857,52 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
>  	return 0;
>  }
>  
> +/* If byte order of 'elf' differs from current byte order, convert the data->d_buf.
> + * ELF file is opened in a readonly mode, so data->d_buf cannot be modified in place.
> + * Instead, allocate a new buffer if modification is necessary.
> + */
> +static int convert_idlist_endianness(Elf *elf, Elf_Data *src, struct local_elf_data *dst)
> +{
> +	int byteorder, i;
> +	char *elf_ident;
> +	uint32_t *tmp;
> +
> +	dst->d_size = src->d_size;
> +	dst->d_off = src->d_off;
> +	elf_ident = elf_getident(elf, NULL);
> +	if (elf_ident == NULL) {
> +		fprintf(stderr, "Cannot get ELF identification from header\n");
> +		return -EINVAL;
> +	}
> +	byteorder = elf_ident[EI_DATA];
> +	if ((BYTE_ORDER == LITTLE_ENDIAN && byteorder == ELFDATA2LSB)
> +	    || (BYTE_ORDER == BIG_ENDIAN && byteorder == ELFDATA2MSB)) {
> +		dst->d_buf = src->d_buf;
> +		dst->owns_buf = false;
> +		return 0;
> +	}
> +	tmp = malloc(src->d_size);
> +	if (tmp == NULL) {
> +		fprintf(stderr, "Cannot allocate %lu bytes of memory\n", src->d_size);
> +		return -ENOMEM;
> +	}
> +	memcpy(tmp, src->d_buf, src->d_size);
> +	dst->d_buf = tmp;
> +	dst->owns_buf = true;
> +
> +	/* .BTF_ids sections consist of u32 objects */
> +	for (i = 0; i < dst->d_size / sizeof(uint32_t); i++)
> +		tmp[i] = bswap_32(tmp[i]);
> +	return 0;
> +}
> +
>  static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  {
>  	const char *filename = encoder->source_filename;
>  	struct gobuffer btf_kfunc_ranges = {};
> +	struct local_elf_data idlist = {};
>  	struct gobuffer btf_funcs = {};
>  	Elf_Data *symbols = NULL;
> -	Elf_Data *idlist = NULL;
>  	Elf_Scn *symscn = NULL;
>  	int symbols_shndx = -1;
>  	size_t idlist_addr = 0;
> @@ -1918,7 +1967,9 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  		} else if (!strcmp(secname, BTF_IDS_SECTION)) {
>  			idlist_shndx = i;
>  			idlist_addr = shdr.sh_addr;
> -			idlist = data;
> +			err = convert_idlist_endianness(elf, data, &idlist);
> +			if (err < 0)
> +				goto out;
>  		}
>  	}
>  
> @@ -1960,7 +2011,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  			continue;
>  
>  		name = elf_strptr(elf, strtabidx, sym.st_name);
> -		if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr))
> +		if (!is_sym_kfunc_set(&sym, name, &idlist, idlist_addr))
>  			continue;
>  
>  		range.start = sym.st_value;
> @@ -2003,13 +2054,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  			if (ranges[j].start <= addr && addr < ranges[j].end) {
>  				found = true;
>  				off = addr - idlist_addr;
> -				if (off < 0 || off + sizeof(*pair) > idlist->d_size) {
> +				if (off < 0 || off + sizeof(*pair) > idlist.d_size) {
>  					fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n",
>  						__func__, func, BTF_IDS_SECTION);
>  					free(func);
>  					goto out;
>  				}
> -				pair = idlist->d_buf + off;
> +				pair = idlist.d_buf + off;
>  				break;
>  			}
>  		}
> @@ -2031,6 +2082,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  out:
>  	__gobuffer__delete(&btf_funcs);
>  	__gobuffer__delete(&btf_kfunc_ranges);
> +	if (idlist.owns_buf)
> +		free(idlist.d_buf);
>  	if (elf)
>  		elf_end(elf);
>  	if (fd != -1)
> -- 
> 2.47.0
> 

Re: [PATCH dwarves v2 1/1] btf_encoder: handle .BTF_ids section endianness

[Arnaldo Carvalho de Melo]

On Sat, Nov 23, 2024 at 02:24:56PM +0100, Jiri Olsa wrote:
> On Fri, Nov 22, 2024 at 01:44:31PM -0800, Eduard Zingerman wrote:
> > btf_encoder__tag_kfuncs() reads .BTF_ids section to identify a set of
> > kfuncs present in the ELF file being processed.
> > This section consists of:
> > - arrays of uint32_t elements;
> > - arrays of records with the following structure:
> >   struct btf_id_and_flag {
> >       uint32_t id;
> >       uint32_t flags;
> >   };
> > 
> > When endianness of a binary operated by pahole differs from the host
> > system's endianness, these fields require byte-swapping before use.
> > Currently, this byte-swapping does not occur, resulting in kfuncs not
> > being marked with declaration tags.
> > 
> > This commit resolves the issue by introducing an endianness conversion
> > step for the .BTF_ids section data before the main processing stage.
> > Since the ELF file is opened in O_RDONLY mode, gelf_xlatetom()
> > cannot be used for endianness conversion.
> > Instead, a new type is introduced:
> > 
> >   struct local_elf_data {
> > 	void *d_buf;
> > 	size_t d_size;
> > 	int64_t d_off;
> > 	bool owns_buf;
> >   };
> > 
> > This structure is populated from the Elf_Data object representing
> > the .BTF_ids section. When byte-swapping is required, a local copy
> > of d_buf is created.
> > 
> > Cc: Alan Maguire <alan.maguire@oracle.com>
> > Cc: Daniel Xu <dxu@dxuuu.xyz>
> > Cc: Jiri Olsa <olsajiri@gmail.com>
> > Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > Cc: Vadim Fedorenko <vadfed@meta.com>
> > Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF")
> > Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Looks ok to me as well, only byte swaps when needed, so affects only
cross BTF encoding.

Alan, have you looked at this as well?

I think I saw instructions in one of the messages in this thread to get
hold of a vmlinux for s390 and test it. Right?

One extra question: this solves the BTF encoder case, the loader already
supported loading BTF from a different endianness, right? Lemme
check.

cus__load_btf()
  cu->little_endian = btf__endianness(btf) == BTF_LITTLE_ENDIAN;

enum btf_endianness btf__endianness(const struct btf *btf)
{
        if (is_host_big_endian())
                return btf->swapped_endian ? BTF_LITTLE_ENDIAN : BTF_BIG_ENDIAN;
        else
                return btf->swapped_endian ? BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN;
}

So we have parts of BTF byte swapping happening in libbpf and with this
patch, parts of it done in pahole, have you tought about doing this in
libbpf instead?

- Arnaldo
 
> > ---
> >  btf_encoder.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 59 insertions(+), 6 deletions(-)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index e1adddf..06d4a61 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -33,6 +33,7 @@
> >  #include <stdint.h>
> >  #include <search.h> /* for tsearch(), tfind() and tdestroy() */
> >  #include <pthread.h>
> > +#include <byteswap.h>
> >  
> >  #define BTF_IDS_SECTION		".BTF_ids"
> >  #define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
> > @@ -145,6 +146,14 @@ struct btf_kfunc_set_range {
> >  	uint64_t end;
> >  };
> >  
> > +/* Like Elf_Data, but when there is a need to change the data read from ELF */
> > +struct local_elf_data {
> > +	void *d_buf;
> > +	size_t d_size;
> > +	int64_t d_off;
> > +	bool owns_buf;
> > +};
> > +
> >  static LIST_HEAD(encoders);
> >  static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
> >  
> > @@ -1681,7 +1690,8 @@ out:
> >  }
> >  
> >  /* Returns if `sym` points to a kfunc set */
> > -static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, Elf_Data *idlist, size_t idlist_addr)
> > +static int is_sym_kfunc_set(GElf_Sym *sym, const char *name, struct local_elf_data *idlist,
> > +			    size_t idlist_addr)
> >  {
> >  	void *ptr = idlist->d_buf;
> >  	struct btf_id_set8 *set;
> > @@ -1847,13 +1857,52 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer *
> >  	return 0;
> >  }
> >  
> > +/* If byte order of 'elf' differs from current byte order, convert the data->d_buf.
> > + * ELF file is opened in a readonly mode, so data->d_buf cannot be modified in place.
> > + * Instead, allocate a new buffer if modification is necessary.
> > + */
> > +static int convert_idlist_endianness(Elf *elf, Elf_Data *src, struct local_elf_data *dst)
> > +{
> > +	int byteorder, i;
> > +	char *elf_ident;
> > +	uint32_t *tmp;
> > +
> > +	dst->d_size = src->d_size;
> > +	dst->d_off = src->d_off;
> > +	elf_ident = elf_getident(elf, NULL);
> > +	if (elf_ident == NULL) {
> > +		fprintf(stderr, "Cannot get ELF identification from header\n");
> > +		return -EINVAL;
> > +	}
> > +	byteorder = elf_ident[EI_DATA];
> > +	if ((BYTE_ORDER == LITTLE_ENDIAN && byteorder == ELFDATA2LSB)
> > +	    || (BYTE_ORDER == BIG_ENDIAN && byteorder == ELFDATA2MSB)) {
> > +		dst->d_buf = src->d_buf;
> > +		dst->owns_buf = false;
> > +		return 0;
> > +	}
> > +	tmp = malloc(src->d_size);
> > +	if (tmp == NULL) {
> > +		fprintf(stderr, "Cannot allocate %lu bytes of memory\n", src->d_size);
> > +		return -ENOMEM;
> > +	}
> > +	memcpy(tmp, src->d_buf, src->d_size);
> > +	dst->d_buf = tmp;
> > +	dst->owns_buf = true;
> > +
> > +	/* .BTF_ids sections consist of u32 objects */
> > +	for (i = 0; i < dst->d_size / sizeof(uint32_t); i++)
> > +		tmp[i] = bswap_32(tmp[i]);
> > +	return 0;
> > +}
> > +
> >  static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> >  {
> >  	const char *filename = encoder->source_filename;
> >  	struct gobuffer btf_kfunc_ranges = {};
> > +	struct local_elf_data idlist = {};
> >  	struct gobuffer btf_funcs = {};
> >  	Elf_Data *symbols = NULL;
> > -	Elf_Data *idlist = NULL;
> >  	Elf_Scn *symscn = NULL;
> >  	int symbols_shndx = -1;
> >  	size_t idlist_addr = 0;
> > @@ -1918,7 +1967,9 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> >  		} else if (!strcmp(secname, BTF_IDS_SECTION)) {
> >  			idlist_shndx = i;
> >  			idlist_addr = shdr.sh_addr;
> > -			idlist = data;
> > +			err = convert_idlist_endianness(elf, data, &idlist);
> > +			if (err < 0)
> > +				goto out;
> >  		}
> >  	}
> >  
> > @@ -1960,7 +2011,7 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> >  			continue;
> >  
> >  		name = elf_strptr(elf, strtabidx, sym.st_name);
> > -		if (!is_sym_kfunc_set(&sym, name, idlist, idlist_addr))
> > +		if (!is_sym_kfunc_set(&sym, name, &idlist, idlist_addr))
> >  			continue;
> >  
> >  		range.start = sym.st_value;
> > @@ -2003,13 +2054,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> >  			if (ranges[j].start <= addr && addr < ranges[j].end) {
> >  				found = true;
> >  				off = addr - idlist_addr;
> > -				if (off < 0 || off + sizeof(*pair) > idlist->d_size) {
> > +				if (off < 0 || off + sizeof(*pair) > idlist.d_size) {
> >  					fprintf(stderr, "%s: kfunc '%s' offset outside section '%s'\n",
> >  						__func__, func, BTF_IDS_SECTION);
> >  					free(func);
> >  					goto out;
> >  				}
> > -				pair = idlist->d_buf + off;
> > +				pair = idlist.d_buf + off;
> >  				break;
> >  			}
> >  		}
> > @@ -2031,6 +2082,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
> >  out:
> >  	__gobuffer__delete(&btf_funcs);
> >  	__gobuffer__delete(&btf_kfunc_ranges);
> > +	if (idlist.owns_buf)
> > +		free(idlist.d_buf);
> >  	if (elf)
> >  		elf_end(elf);
> >  	if (fd != -1)
> > -- 
> > 2.47.0
> > 

Re: [PATCH dwarves v2 1/1] btf_encoder: handle .BTF_ids section endianness

[Eduard Zingerman]

On Tue, 2024-11-26 at 13:25 -0300, Arnaldo Carvalho de Melo wrote:

[...]

Hi Arnaldo,

> I think I saw instructions in one of the messages in this thread to get
> hold of a vmlinux for s390 and test it. Right?

Yes, in cover letter. Full vmlinux is not needed, a vmlinux binary for
s390 would be sufficient for testing. Repeating the recipe for convenience:

  To reproduce the bug:
  - follow the instructions in [0] to build an s390 vmlinux;
  - generate BTF requesting declaration tags for kfuncs:
    $ pahole --btf_features_strict=decl_tag_kfuncs,decl_tag \
             --btf_encode_detached=test.btf vmlinux
  - observe that no kfuncs are generated:
    $ bpftool btf dump file test.btf format c | grep __ksym

[0] https://docs.kernel.org/bpf/s390.html

> One extra question: this solves the BTF encoder case, the loader already
> supported loading BTF from a different endianness, right? Lemme
> check.
> 
> cus__load_btf()
>   cu->little_endian = btf__endianness(btf) == BTF_LITTLE_ENDIAN;
> 
> enum btf_endianness btf__endianness(const struct btf *btf)
> {
>         if (is_host_big_endian())
>                 return btf->swapped_endian ? BTF_LITTLE_ENDIAN : BTF_BIG_ENDIAN;
>         else
>                 return btf->swapped_endian ? BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN;
> }

I can switch to is_host_big_endian() instead of `BYTE_ORDER` macro
if you think that's better.

> So we have parts of BTF byte swapping happening in libbpf and with this
> patch, parts of it done in pahole, have you tought about doing this in
> libbpf instead?

BTF id lists handling is currently not a part of libbpf.

Thanks,
Eduard

Re: [PATCH dwarves v2 1/1] btf_encoder: handle .BTF_ids section endianness

[Arnaldo Carvalho de Melo]

On Tue, Nov 26, 2024 at 09:32:28AM -0800, Eduard Zingerman wrote:
> On Tue, 2024-11-26 at 13:25 -0300, Arnaldo Carvalho de Melo wrote:
 
> [...]
 
> Hi Arnaldo,
 
> > I think I saw instructions in one of the messages in this thread to get
> > hold of a vmlinux for s390 and test it. Right?
 
> Yes, in cover letter. Full vmlinux is not needed, a vmlinux binary for
> s390 would be sufficient for testing. Repeating the recipe for convenience:
 
>   To reproduce the bug:
>   - follow the instructions in [0] to build an s390 vmlinux;
>   - generate BTF requesting declaration tags for kfuncs:
>     $ pahole --btf_features_strict=decl_tag_kfuncs,decl_tag \
>              --btf_encode_detached=test.btf vmlinux
>   - observe that no kfuncs are generated:
>     $ bpftool btf dump file test.btf format c | grep __ksym
 
> [0] https://docs.kernel.org/bpf/s390.html

Thanks.
 
> > One extra question: this solves the BTF encoder case, the loader already
> > supported loading BTF from a different endianness, right? Lemme
> > check.
 
> > cus__load_btf()
> >   cu->little_endian = btf__endianness(btf) == BTF_LITTLE_ENDIAN;

> > enum btf_endianness btf__endianness(const struct btf *btf)
> > {
> >         if (is_host_big_endian())
> >                 return btf->swapped_endian ? BTF_LITTLE_ENDIAN : BTF_BIG_ENDIAN;
> >         else
> >                 return btf->swapped_endian ? BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN;
> > }
 
> I can switch to is_host_big_endian() instead of `BYTE_ORDER` macro
> if you think that's better.

No need for that, I think, is_host_big_endian() is a static function
inlib/bpf/src/btf.c.

I was just looking at how endianness was being handled and noticed
libbpf does it, but, as you say below...
 
> > So we have parts of BTF byte swapping happening in libbpf and with this
> > patch, parts of it done in pahole, have you tought about doing this in
> > libbpf instead?
 
> BTF id lists handling is currently not a part of libbpf.

So we should do it in pahole, as you did, so all is clarified now, I'm
now testing it with the provided instructions.

Thanks,

- Arnaldo

Re: [PATCH dwarves v2 1/1] btf_encoder: handle .BTF_ids section endianness

[Eduard Zingerman]

Hi Arnaldo,

Please note a message [0] from Andrii,
where he suggests using elf_getdata_rawchunk() libelf function.
I tried it, it works, makes the patch much smaller.
I'll send a v3 shortly.

[0] https://lore.kernel.org/dwarves/8744c86ba355245f7ecc14d00351c82285fbf644.camel@gmail.com/T/#me4c02e4a4e7fdd0e69dd52b2a6bf598966dc2145

Thanks,
Eduard