On 27/11/2024 11:00, Jiri Olsa wrote:
On Tue, Nov 26, 2024 at 05:50:06PM -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 using elf_getdata_rawchunk()
function to read .BTF_ids section data. When called with ELF_T_WORD as
'type' parameter it does necessary byte order conversion
(only if host and elf endianness do not match).
Cc: Alan Maguire <alan.maguire@xxxxxxxxxx>
Cc: Andrii Nakryiko <andrii@xxxxxxxxxx>
Cc: Daniel Xu <dxu@xxxxxxxxx>
Cc: Jiri Olsa <olsajiri@xxxxxxxxx>
Cc: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
Cc: Vadim Fedorenko <vadfed@xxxxxxxx>
Fixes: 72e88f29c6f7 ("pahole: Inject kfunc decl tags into BTF")
Signed-off-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
---
btf_encoder.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/btf_encoder.c b/btf_encoder.c
index e1adddf..3754884 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1904,18 +1904,32 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
goto out;
}
- data = elf_getdata(scn, 0);
- if (!data) {
- elf_error("Failed to get ELF section(%d) data", i);
- goto out;
- }
-
if (shdr.sh_type == SHT_SYMTAB) {
+ data = elf_getdata(scn, 0);
+ if (!data) {
+ elf_error("Failed to get ELF section(%d) data", i);
+ goto out;
+ }
+
symbols_shndx = i;
symscn = scn;
symbols = data;
strtabidx = shdr.sh_link;
} else if (!strcmp(secname, BTF_IDS_SECTION)) {
+ /* .BTF_ids section consists of uint32_t elements,
+ * and thus might need byte order conversion.
+ * However, it has type PROGBITS, hence elf_getdata()
+ * won't automatically do the conversion.
+ * Use elf_getdata_rawchunk() instead,
+ * ELF_T_WORD tells it to do the necessary conversion.
+ */
+ data = elf_getdata_rawchunk(elf, shdr.sh_offset, shdr.sh_size, ELF_T_WORD);
looks good, I'm just curious about one thing..
so ELF_T_WORD enum has this comment: /* Elf32_Word, Elf64_Word, ... */
I did just quick check, ***so I might be easily wrong***, but I wonder the
code in __elf_xfctstom (which I assume is the one called for conversion)
chooses to swap 32/64 bits values based on elf->class .. so for 64bit ELF
class we swap 64bit values? ... while .BTF_ids has always 32 bit values
Well according to the doc:
ELF_T_WORD Unsigned 32-bit words.
ELF_T_XWORD Unsigned 64-bit words.
It shouldn't use 64 bits swap:
const xfct_t __elf_xfctstom[EV_NUM - 1][EV_NUM - 1][ELFCLASSNUM -
1][ELF_T_NUM] =
....
[ELF_T_WORD] = ElfW2(Bits, cvt_Word),
[ELF_T_XWORD] = ElfW2(Bits, cvt_Xword),
...
Are you looking somewhere else?
thanks,
jirka
+ if (!data) {
+ elf_error("Failed to get %s ELF section(%d) data",
+ BTF_IDS_SECTION, i);
+ goto out;
+ }
+
idlist_shndx = i;
idlist_addr = shdr.sh_addr;
idlist = data;
--
2.47.0