| Message ID | 20201203220625.3704363-3-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | btf_encoder: Detect kernel modules | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Dec 3, 2020 at 2:08 PM Jiri Olsa <jolsa@kernel.org> wrote: > > We can't assume the address size is always size of unsigned > long, we have to use directly the ELF's address size. > > Changing addrs array to __u64 and convert 32 bit address > values when copying from ELF section. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- It looks ok to me, but I didn't expect that changes would be so numerous... Makes me wonder if pahole ever supported working with ELF files of different bitness. I'll defer to Arnaldo to make a call on whether this is necessary. > btf_encoder.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > [...]
On Thu, Dec 03, 2020 at 03:22:18PM -0800, Andrii Nakryiko wrote: > On Thu, Dec 3, 2020 at 2:08 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > We can't assume the address size is always size of unsigned > > long, we have to use directly the ELF's address size. > > > > Changing addrs array to __u64 and convert 32 bit address > > values when copying from ELF section. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > It looks ok to me, but I didn't expect that changes would be so > numerous... Makes me wonder if pahole ever supported working with ELF > files of different bitness. I'll defer to Arnaldo to make a call on > whether this is necessary. so to test this I built 32bit vmlinux and used 64bit pahole to generate BTF data on both vmlinux and modules, which I thought was valid use case jirka > > > btf_encoder.c | 24 +++++++++++++++++------- > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > [...] >
Em Fri, Dec 04, 2020 at 12:37:50AM +0100, Jiri Olsa escreveu: > On Thu, Dec 03, 2020 at 03:22:18PM -0800, Andrii Nakryiko wrote: > > On Thu, Dec 3, 2020 at 2:08 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > We can't assume the address size is always size of unsigned > > > long, we have to use directly the ELF's address size. > > > > > > Changing addrs array to __u64 and convert 32 bit address > > > values when copying from ELF section. > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > > It looks ok to me, but I didn't expect that changes would be so > > numerous... Makes me wonder if pahole ever supported working with ELF > > files of different bitness. I'll defer to Arnaldo to make a call on > > whether this is necessary. > > so to test this I built 32bit vmlinux and used 64bit pahole > to generate BTF data on both vmlinux and modules, which I > thought was valid use case It is valid, yeah. - Arnaldo > jirka > > > > > > btf_encoder.c | 24 +++++++++++++++++------- > > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > > > > [...] > > >
diff --git a/btf_encoder.c b/btf_encoder.c index 0a33388db675..398be0fbf7c7 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -93,8 +93,8 @@ static int collect_function(struct btf_elf *btfe, GElf_Sym *sym) static int addrs_cmp(const void *_a, const void *_b) { - const unsigned long *a = _a; - const unsigned long *b = _b; + const __u64 *a = _a; + const __u64 *b = _b; if (*a == *b) return 0; @@ -102,9 +102,10 @@ static int addrs_cmp(const void *_a, const void *_b) } static int get_vmlinux_addrs(struct btf_elf *btfe, struct funcs_layout *fl, - unsigned long **paddrs, unsigned long *pcount) + __u64 **paddrs, __u64 *pcount) { - unsigned long *addrs, count, offset; + __u64 *addrs, count, offset; + unsigned int addr_size, i; Elf_Data *data; GElf_Shdr shdr; Elf_Scn *sec; @@ -128,8 +129,11 @@ static int get_vmlinux_addrs(struct btf_elf *btfe, struct funcs_layout *fl, return -1; } + /* Get address size from processed file's ELF class. */ + addr_size = gelf_getclass(btfe->elf) == ELFCLASS32 ? 4 : 8; + offset = fl->mcount_start - shdr.sh_addr; - count = (fl->mcount_stop - fl->mcount_start) / 8; + count = (fl->mcount_stop - fl->mcount_start) / addr_size; data = elf_getdata(sec, 0); if (!data) { @@ -144,7 +148,13 @@ static int get_vmlinux_addrs(struct btf_elf *btfe, struct funcs_layout *fl, return -1; } - memcpy(addrs, data->d_buf + offset, count * sizeof(addrs[0])); + if (addr_size == sizeof(__u64)) { + memcpy(addrs, data->d_buf + offset, count * addr_size); + } else { + for (i = 0; i < count; i++) + addrs[i] = (__u64) *((__u32 *) (data->d_buf + offset + i * addr_size)); + } + *paddrs = addrs; *pcount = count; return 0; @@ -152,7 +162,7 @@ static int get_vmlinux_addrs(struct btf_elf *btfe, struct funcs_layout *fl, static int setup_functions(struct btf_elf *btfe, struct funcs_layout *fl) { - unsigned long *addrs, count, i; + __u64 *addrs, count, i; int functions_valid = 0; /*
We can't assume the address size is always size of unsigned long, we have to use directly the ELF's address size. Changing addrs array to __u64 and convert 32 bit address values when copying from ELF section. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- btf_encoder.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)