| Message ID | 20210121202203.9346-1-jolsa@kernel.org |
|---|---|
| Headers | show |
| Series | dwarves,libbpf: Add support to use optional extended section index table | expand |
On Thu, Jan 21, 2021 at 12:24 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..9e0fdca3ae04 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 str_idx; > + > + if (elf_getshdrstrndx(elf, &str_idx)) > + return NULL; > > 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, str_idx, shp->sh_name); > + if (str && !strcmp(name, str)) { if (!str) would be an error? should we bail out here? > if (index) > *index = cnt; > break; > -- > 2.27.0 >
On Thu, Jan 21, 2021 at 03:10:25PM -0800, Andrii Nakryiko wrote: > On Thu, Jan 21, 2021 at 12:24 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..9e0fdca3ae04 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 str_idx; > > + > > + if (elf_getshdrstrndx(elf, &str_idx)) > > + return NULL; > > > > 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, str_idx, shp->sh_name); > > + if (str && !strcmp(name, str)) { > > if (!str) would be an error? should we bail out here? seems like if elf_nextscn returns NULL, it will be the case for all the sections in here.. but bailing out on (!str) is more direct and safer I'll send an update thanks, jirka > > > if (index) > > *index = cnt; > > break; > > -- > > 2.27.0 > > >
On Thu, Jan 21, 2021 at 12:26 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. > > Call elf_getshdrstrndx to get the proper string table index, > instead of reading it directly from ELF header. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- I've applied this patch to bpf-next, you don't need to re-send it in the next version of this patch set. > tools/lib/bpf/btf.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 9970a288dda5..d9c10830d749 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -858,6 +858,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); > @@ -882,7 +883,14 @@ 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)) { > + > + if (elf_getshdrstrndx(elf, &shstrndx)) { > + pr_warn("failed to get section names section index for %s\n", > + path); > + goto done; > + } > + > + if (!elf_rawdata(elf_getscn(elf, shstrndx), NULL)) { > pr_warn("failed to get e_shstrndx from %s\n", path); > goto done; > } > @@ -897,7 +905,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 is based on previously posted fix [1]. v2 changes: - many variables renames [Andrii] - use elf_getshdrstrndx() unconditionally [Andrii] - add elf_symtab__for_each_symbol_index macro [Andrii] - add more comments [Andrii] - verify that extended symtab section type is SHT_SYMTAB_SHNDX [Andrii] - fix Joe's crash in dwarves build, wrong sym.st_shndx assignment 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 | 36 ++++++++++++++++++++++++++++++++---- dutil.c | 8 ++++++-- elf_symtab.c | 39 ++++++++++++++++++++++++++++++++++++++- elf_symtab.h | 2 ++ 4 files changed, 78 insertions(+), 7 deletions(-) libbpf: Jiri Olsa (1): libbpf: Use string table index from index table if needed tools/lib/bpf/btf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)