| Message ID | 20210119221220.1745061-1-jolsa@kernel.org |
|---|---|
| Headers | show |
| Series | dwarves,libbpf: Add support to use optional extended section index table | expand |
On Tue, Jan 19, 2021 at 11:12:17PM +0100, Jiri Olsa wrote: > hi, > kpatch guys hit an issue with pahole over their vmlinux, which > contains many (over 100000) sections, pahole crashes. > FWIW this is probably only going to be problem when building the kernel with -f[function|data]-sections and tipping over 65536 sections. We only use these option to determine code deltas, but other users (FG-ASLR, LTO?) may need to actually build runtime code. > With so many sections, ELF is using extended section index table, > which is used to hold values for some of the indexes and extra > code is needed to retrieve them. > > This patchset adds the support for pahole to properly read string > table index and symbol's section index, which are used in btf_encoder. > > This patchset also adds support for libbpf to properly parse .BTF > section on such object. > > This patchset based on previously posted fix [1]. > Hi Jiri, Thanks for posting a potential fix, here's what I saw when running it: 1-Installed your scratch build: % rpm -q --whatprovides $(which pahole) dwarves-1.19-2.el8.x86_64 2-From the kernel build: LD vmlinux.o MODPOST vmlinux.o BTF .btf.vmlinux.bin.o scripts/link-vmlinux.sh: line 127: 1851330 Segmentation fault (core dumped) LLVM_OBJCOPY=${OBJCOPY} ${PAHOLE} -J ${1} objcopy: --change-section-vma .BTF=0x0000000000000000 never used objcopy: --change-section-lma .BTF=0x0000000000000000 never used objcopy: error: the input file '.btf.vmlinux.bin' is empty Failed to generate BTF for vmlinux Try to disable CONFIG_DEBUG_INFO_BTF make: *** [Makefile:1050: vmlinux] Error 1 3-coredump backtrace: ... Core was generated by `pahole -J .tmp_vmlinux.btf'. ... (gdb) bt #0 0x00007fc0e81e31c0 in __memcpy_ssse3 () from /lib64/libc.so.6 #1 0x00007fc0e8b6700a in memcpy (__len=306248, __src=<optimized out>, __dest=0x7fc0e91a0010) at /usr/include/bits/string_fortified.h:34 #2 get_vmlinux_addrs (btfe=<optimized out>, pcount=<synthetic pointer>, paddrs=<synthetic pointer>, fl=<synthetic pointer>) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/btf_encoder.c:167 #3 setup_functions (fl=<synthetic pointer>, btfe=<optimized out>) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/btf_encoder.c:251 #4 collect_symbols (collect_percpu_vars=<optimized out>, btfe=<optimized out>) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/btf_encoder.c:645 #5 cu__encode_btf (cu=<optimized out>, verbose=0, force=false, skip_encoding_vars=<optimized out>) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/btf_encoder.c:694 #6 0x000055eb2e4b6cc5 in pahole_stealer (cu=0x55eb2ecbb920, conf_load=<optimized out>) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/pahole.c:2402 #7 0x00007fc0e8b6d2db in finalize_cu_immediately (conf=0x55eb2e6bc0e0 <conf_load>, dcu=0x7ffd88fda9d0, cu=0x55eb2ecbb920, cus=0x55eb2ecbb5d0) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarf_loader.c:2477 #8 cus__load_module (cus=cus@entry=0x55eb2ecbb5d0, conf=0x55eb2e6bc0e0 <conf_load>, mod=mod@entry=0x55eb2ecbb5f0, dw=0x55eb2ecbe6a0, elf=elf@entry=0x7fc0ad941010, filename=0x7ffd8905b98d ".tmp_vmlinux.btf") at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarf_loader.c:2477 #9 0x00007fc0e8b6d5c5 in cus__process_dwflmod (dwflmod=0x55eb2ecbb5f0, userdata=<optimized out>, name=<optimized out>, base=<optimized out>, arg=0x7ffd8905ab00) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarf_loader.c:2522 #10 0x00007fc0e88f9c71 in dwfl_getmodules () from /lib64/libdw.so.1 #11 0x00007fc0e8b69cdc in cus__process_file (filename=<optimized out>, fd=5, conf=0x55eb2e6bc0e0 <conf_load>, cus=0x55eb2ecbb5d0) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarf_loader.c:2575 #12 dwarf__load_file (cus=0x55eb2ecbb5d0, conf=0x55eb2e6bc0e0 <conf_load>, filename=<optimized out>) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarf_loader.c:2592 #13 0x00007fc0e8b5cb82 in cus__load_file (cus=cus@entry=0x55eb2ecbb5d0, conf=conf@entry=0x55eb2e6bc0e0 <conf_load>, filename=0x7ffd8905b98d ".tmp_vmlinux.btf") at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarves.c:1963 #14 0x00007fc0e8b5ce29 in cus__load_files (cus=0x55eb2ecbb5d0, conf=0x55eb2e6bc0e0 <conf_load>, filenames=0x7ffd8905aea8) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/dwarves.c:2324 #15 0x000055eb2e4b371e in main (argc=3, argv=0x7ffd8905ae98) at /usr/src/debug/dwarves-1.19-2.el8.x86_64/pahole.c:2760 I uploaded a gzipped core file here: http://people.redhat.com/~jolawren/coredump.gz If it's easier to get you setup on a repro system, let me know and I can do that. Thanks, -- Joe
On Tue, Jan 19, 2021 at 2:15 PM Jiri Olsa <jolsa@kernel.org> wrote: > > For very large ELF objects (with many sections), we could > get special value SHN_XINDEX (65535) for elf object's string > table index - e_shstrndx. > > In such case we need to call elf_getshdrstrndx to get the > proper string table index. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/lib/bpf/btf.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 3c3f2bc6c652..4fe987846bc0 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -863,6 +863,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > Elf_Scn *scn = NULL; > Elf *elf = NULL; > GElf_Ehdr ehdr; > + size_t shstrndx; > > if (elf_version(EV_CURRENT) == EV_NONE) { > pr_warn("failed to init libelf for %s\n", path); > @@ -887,7 +888,16 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > pr_warn("failed to get EHDR from %s\n", path); > goto done; > } > - if (!elf_rawdata(elf_getscn(elf, ehdr.e_shstrndx), NULL)) { > + > + /* > + * Get string table index from extended section index > + * table if needed. > + */ > + shstrndx = ehdr.e_shstrndx; > + if (shstrndx == SHN_XINDEX && elf_getshdrstrndx(elf, &shstrndx)) > + goto done; just use elf_getshdrstrndx() unconditionally, it works for extended and non-extended numbering (see libbpf.c). > + > + if (!elf_rawdata(elf_getscn(elf, shstrndx), NULL)) { > pr_warn("failed to get e_shstrndx from %s\n", path); > goto done; > } > @@ -902,7 +912,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > idx, path); > goto done; > } > - name = elf_strptr(elf, ehdr.e_shstrndx, sh.sh_name); > + name = elf_strptr(elf, shstrndx, sh.sh_name); > if (!name) { > pr_warn("failed to get section(%d) name from %s\n", > idx, path); > -- > 2.27.0 >
On Tue, Jan 19, 2021 at 2:16 PM Jiri Olsa <jolsa@kernel.org> wrote: > > In case the elf's header e_shstrndx contains SHN_XINDEX, > we need to call elf_getshdrstrndx to get the proper > string table index. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > dutil.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/dutil.c b/dutil.c > index 7b667647420f..321f4be6669e 100644 > --- a/dutil.c > +++ b/dutil.c > @@ -179,13 +179,17 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep, > { > Elf_Scn *sec = NULL; > size_t cnt = 1; > + size_t shstrndx = ep->e_shstrndx; > + > + if (shstrndx == SHN_XINDEX && elf_getshdrstrndx(elf, &shstrndx)) > + return NULL; > see comment for patch #3, no need for SHN_XINDEX checks, elf_getshdrstrndx() handles this transparently > while ((sec = elf_nextscn(elf, sec)) != NULL) { > char *str; > > gelf_getshdr(sec, shp); > - str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name); > - if (!strcmp(name, str)) { > + str = elf_strptr(elf, shstrndx, shp->sh_name); > + if (str && !strcmp(name, str)) { > if (index) > *index = cnt; > break; > -- > 2.27.0 >
Em Tue, Jan 19, 2021 at 05:22:08PM -0800, Andrii Nakryiko escreveu: > On Tue, Jan 19, 2021 at 2:15 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > For very large ELF objects (with many sections), we could > > get special value SHN_XINDEX (65535) for elf object's string > > table index - e_shstrndx. > > > > In such case we need to call elf_getshdrstrndx to get the > > proper string table index. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/lib/bpf/btf.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > > index 3c3f2bc6c652..4fe987846bc0 100644 > > --- a/tools/lib/bpf/btf.c > > +++ b/tools/lib/bpf/btf.c > > @@ -863,6 +863,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > > Elf_Scn *scn = NULL; > > Elf *elf = NULL; > > GElf_Ehdr ehdr; > > + size_t shstrndx; > > > > if (elf_version(EV_CURRENT) == EV_NONE) { > > pr_warn("failed to init libelf for %s\n", path); > > @@ -887,7 +888,16 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > > pr_warn("failed to get EHDR from %s\n", path); > > goto done; > > } > > - if (!elf_rawdata(elf_getscn(elf, ehdr.e_shstrndx), NULL)) { > > + > > + /* > > + * Get string table index from extended section index > > + * table if needed. > > + */ > > + shstrndx = ehdr.e_shstrndx; > > + if (shstrndx == SHN_XINDEX && elf_getshdrstrndx(elf, &shstrndx)) > > + goto done; > > just use elf_getshdrstrndx() unconditionally, it works for extended > and non-extended numbering (see libbpf.c). Yeah, we even have this in tools/perf/util/symbol-elf.c: #ifndef HAVE_ELF_GETSHDRSTRNDX_SUPPORT static int elf_getshdrstrndx(Elf *elf __maybe_unused, size_t *dst __maybe_unused) { pr_err("%s: update your libelf to > 0.140, this one lacks elf_getshdrstrndx().\n", __func__); return -1; } #endif And a feature detection for that: [acme@five perf]$ cat tools/build/feature/test-libelf-getshdrstrndx.c // SPDX-License-Identifier: GPL-2.0 #include <libelf.h> int main(void) { size_t dst; return elf_getshdrstrndx(0, &dst); } [acme@five perf]$ Your implementation would provide a good fallback tho to avoid requiring updating to 0.140 :-) - Arnaldo > > + > > + if (!elf_rawdata(elf_getscn(elf, shstrndx), NULL)) { > > pr_warn("failed to get e_shstrndx from %s\n", path); > > goto done; > > } > > @@ -902,7 +912,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > > idx, path); > > goto done; > > } > > - name = elf_strptr(elf, ehdr.e_shstrndx, sh.sh_name); > > + name = elf_strptr(elf, shstrndx, sh.sh_name); > > if (!name) { > > pr_warn("failed to get section(%d) name from %s\n", > > idx, path); > > -- > > 2.27.0 > > -- - Arnaldo
On Tue, Jan 19, 2021 at 05:23:00PM -0800, Andrii Nakryiko wrote: > On Tue, Jan 19, 2021 at 2:16 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > In case the elf's header e_shstrndx contains SHN_XINDEX, > > we need to call elf_getshdrstrndx to get the proper > > string table index. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > dutil.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/dutil.c b/dutil.c > > index 7b667647420f..321f4be6669e 100644 > > --- a/dutil.c > > +++ b/dutil.c > > @@ -179,13 +179,17 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep, > > { > > Elf_Scn *sec = NULL; > > size_t cnt = 1; > > + size_t shstrndx = ep->e_shstrndx; > > + > > + if (shstrndx == SHN_XINDEX && elf_getshdrstrndx(elf, &shstrndx)) > > + return NULL; > > > > see comment for patch #3, no need for SHN_XINDEX checks, > elf_getshdrstrndx() handles this transparently ok, will change jirka > > > while ((sec = elf_nextscn(elf, sec)) != NULL) { > > char *str; > > > > gelf_getshdr(sec, shp); > > - str = elf_strptr(elf, ep->e_shstrndx, shp->sh_name); > > - if (!strcmp(name, str)) { > > + str = elf_strptr(elf, shstrndx, shp->sh_name); > > + if (str && !strcmp(name, str)) { > > if (index) > > *index = cnt; > > break; > > -- > > 2.27.0 > > >
On Tue, Jan 19, 2021 at 05:22:08PM -0800, Andrii Nakryiko wrote: > On Tue, Jan 19, 2021 at 2:15 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > For very large ELF objects (with many sections), we could > > get special value SHN_XINDEX (65535) for elf object's string > > table index - e_shstrndx. > > > > In such case we need to call elf_getshdrstrndx to get the > > proper string table index. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/lib/bpf/btf.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > > index 3c3f2bc6c652..4fe987846bc0 100644 > > --- a/tools/lib/bpf/btf.c > > +++ b/tools/lib/bpf/btf.c > > @@ -863,6 +863,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > > Elf_Scn *scn = NULL; > > Elf *elf = NULL; > > GElf_Ehdr ehdr; > > + size_t shstrndx; > > > > if (elf_version(EV_CURRENT) == EV_NONE) { > > pr_warn("failed to init libelf for %s\n", path); > > @@ -887,7 +888,16 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > > pr_warn("failed to get EHDR from %s\n", path); > > goto done; > > } > > - if (!elf_rawdata(elf_getscn(elf, ehdr.e_shstrndx), NULL)) { > > + > > + /* > > + * Get string table index from extended section index > > + * table if needed. > > + */ > > + shstrndx = ehdr.e_shstrndx; > > + if (shstrndx == SHN_XINDEX && elf_getshdrstrndx(elf, &shstrndx)) > > + goto done; > > just use elf_getshdrstrndx() unconditionally, it works for extended > and non-extended numbering (see libbpf.c). I did not see that, ok thanks, jirka > > > + > > + if (!elf_rawdata(elf_getscn(elf, shstrndx), NULL)) { > > pr_warn("failed to get e_shstrndx from %s\n", path); > > goto done; > > } > > @@ -902,7 +912,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > > idx, path); > > goto done; > > } > > - name = elf_strptr(elf, ehdr.e_shstrndx, sh.sh_name); > > + name = elf_strptr(elf, shstrndx, sh.sh_name); > > if (!name) { > > pr_warn("failed to get section(%d) name from %s\n", > > idx, path); > > -- > > 2.27.0 > > >
hi, kpatch guys hit an issue with pahole over their vmlinux, which contains many (over 100000) sections, pahole crashes. With so many sections, ELF is using extended section index table, which is used to hold values for some of the indexes and extra code is needed to retrieve them. This patchset adds the support for pahole to properly read string table index and symbol's section index, which are used in btf_encoder. This patchset also adds support for libbpf to properly parse .BTF section on such object. This patchset based on previously posted fix [1]. thanks, jirka [1] https://lore.kernel.org/bpf/20210113102509.1338601-1-jolsa@kernel.org/ --- dwarves: Jiri Olsa (2): elf_symtab: Add support for SHN_XINDEX index to elf_section_by_name bpf_encoder: Translate SHN_XINDEX in symbol's st_shndx values btf_encoder.c | 18 ++++++++++++++++++ dutil.c | 8 ++++++-- elf_symtab.c | 31 ++++++++++++++++++++++++++++++- elf_symtab.h | 1 + 4 files changed, 55 insertions(+), 3 deletions(-) libbpf: Jiri Olsa (1): libbpf: Use string table index from index table if needed tools/lib/bpf/btf.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)