| Message ID | 20210121202203.9346-3-jolsa@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | dwarves,libbpf: Add support to use optional extended section index table | expand |
On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: > On Thu, Jan 21, 2021 at 12:25 PM Jiri Olsa <jolsa@kernel.org> wrote: > > > > For very large ELF objects (with many sections), we could > > get special value SHN_XINDEX (65535) for symbol's st_shndx. > > > > This patch is adding code to detect the optional extended > > section index table and use it to resolve symbol's section > > index. > > > > Adding elf_symtab__for_each_symbol_index macro that returns > > symbol's section index and usign it in collect_symbols function. > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > You missed fixing up collect_function() as well, which is using > elf_sym__section(), which doesn't know about extended numbering. ah right, it's for modules, I guess it's why it did not show up thanks, jirka
On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: SNIP > > @@ -598,9 +599,36 @@ static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl) > > fl->mcount_stop = sym->st_value; > > } > > > > +static bool elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, > > + int id, GElf_Sym *sym, Elf32_Word *sym_sec_idx) > > +{ > > + if (!gelf_getsym(syms, id, sym)) > > + return false; > > + > > + *sym_sec_idx = sym->st_shndx; > > + > > + if (sym->st_shndx == SHN_XINDEX) { > > + if (!syms_sec_idx_table) > > + return false; > > + if (!gelf_getsymshndx(syms, syms_sec_idx_table, > > + id, sym, sym_sec_idx)) > > > gelf_getsymshndx() is supposed to work even for cases that don't use > extended numbering, so this should work, right? > > if (!gelf_getsymshndx(syms, syms_sec_idx_table, id, sym, sym_sec_idx)) > return false; > it seems you're right, gelf_getsymshndx seem to work for both cases, I'll check > if (sym->st_shndx == SHN_XINDEX) > *sym_sec_idx = sym->st_shndx; I don't understand this.. gelf_getsymshndx will return both symbol and proper index, no? also sym_sec_idx is already assigned from previou call > > return true; > > ? > > > + return false; > > + } > > + > > + return true; > > +} > > + > > +#define elf_symtab__for_each_symbol_index(symtab, id, sym, sym_sec_idx) \ > > + for (id = 0, elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, \ > > + id, &sym, &sym_sec_idx); \ > > + id < symtab->nr_syms; \ > > + id++, elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, \ > > + id, &sym, &sym_sec_idx)) > > what do we want to do if elf_sym__get() returns error (false)? We can > either stop or ignore that symbol, right? But currently you are > returning invalid symbol data. > > so either > > for (id = 0; id < symtab->nr_syms && elf_sym__get(symtab->syms, > symtab->syms_sec_idx_table, d, &sym, &sym_sec_idx); id++) > > or > > for (id = 0; id < symtab->nr_syms; id++) > if (elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, d, &sym, > &sym_sec_idx)) if we go ahead with skipping symbols, this one seems good > > > But the current variant looks broken. Oh, and > elf_symtab__for_each_symbol() is similarly broken, can you please fix > that as well? > > And this new macro should probably be in elf_symtab.h, along the > elf_symtab__for_each_symbol. thanks, jirka
On Fri, Jan 22, 2021 at 12:47 PM Jiri Olsa <jolsa@redhat.com> wrote: > > On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: > > SNIP > > > > @@ -598,9 +599,36 @@ static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl) > > > fl->mcount_stop = sym->st_value; > > > } > > > > > > +static bool elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, > > > + int id, GElf_Sym *sym, Elf32_Word *sym_sec_idx) > > > +{ > > > + if (!gelf_getsym(syms, id, sym)) > > > + return false; > > > + > > > + *sym_sec_idx = sym->st_shndx; > > > + > > > + if (sym->st_shndx == SHN_XINDEX) { > > > + if (!syms_sec_idx_table) > > > + return false; > > > + if (!gelf_getsymshndx(syms, syms_sec_idx_table, > > > + id, sym, sym_sec_idx)) > > > > > > gelf_getsymshndx() is supposed to work even for cases that don't use > > extended numbering, so this should work, right? > > > > if (!gelf_getsymshndx(syms, syms_sec_idx_table, id, sym, sym_sec_idx)) > > return false; > > > > it seems you're right, gelf_getsymshndx seem to work for > both cases, I'll check > > > > if (sym->st_shndx == SHN_XINDEX) > > *sym_sec_idx = sym->st_shndx; > > I don't understand this.. gelf_getsymshndx will return both > symbol and proper index, no? also sym_sec_idx is already > assigned from previou call Reading (some) implementation of gelf_getsymshndx() that I found online, it won't set sym_sec_idx, if the symbol *doesn't* use extended numbering. But it will still return symbol data. So to return the section index in all cases, we need to check again *after* we got symbol, and if it's not extended, then set index manually. > > > > > return true; > > > > ? > > > > > + return false; > > > + } > > > + > > > + return true; > > > +} > > > + > > > +#define elf_symtab__for_each_symbol_index(symtab, id, sym, sym_sec_idx) \ > > > + for (id = 0, elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, \ > > > + id, &sym, &sym_sec_idx); \ > > > + id < symtab->nr_syms; \ > > > + id++, elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, \ > > > + id, &sym, &sym_sec_idx)) > > > > what do we want to do if elf_sym__get() returns error (false)? We can > > either stop or ignore that symbol, right? But currently you are > > returning invalid symbol data. > > > > so either > > > > for (id = 0; id < symtab->nr_syms && elf_sym__get(symtab->syms, > > symtab->syms_sec_idx_table, d, &sym, &sym_sec_idx); id++) > > > > or > > > > for (id = 0; id < symtab->nr_syms; id++) > > if (elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, d, &sym, > > &sym_sec_idx)) > > if we go ahead with skipping symbols, this one seems good I think skipping symbols is nicer. If ELF is totally broken, then all symbols are going to be ignored anyway. If it's some one-off issue for a specific symbol, we'll just ignore it (unfortunately, silently). > > > > > > > But the current variant looks broken. Oh, and > > elf_symtab__for_each_symbol() is similarly broken, can you please fix > > that as well? > > > > And this new macro should probably be in elf_symtab.h, along the > > elf_symtab__for_each_symbol. > > thanks, > jirka >
On Fri, Jan 22, 2021 at 02:55:51PM -0800, Andrii Nakryiko wrote: > On Fri, Jan 22, 2021 at 12:47 PM Jiri Olsa <jolsa@redhat.com> wrote: > > > > On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: > > > > SNIP > > > > > > @@ -598,9 +599,36 @@ static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl) > > > > fl->mcount_stop = sym->st_value; > > > > } > > > > > > > > +static bool elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, > > > > + int id, GElf_Sym *sym, Elf32_Word *sym_sec_idx) > > > > +{ > > > > + if (!gelf_getsym(syms, id, sym)) > > > > + return false; > > > > + > > > > + *sym_sec_idx = sym->st_shndx; > > > > + > > > > + if (sym->st_shndx == SHN_XINDEX) { > > > > + if (!syms_sec_idx_table) > > > > + return false; > > > > + if (!gelf_getsymshndx(syms, syms_sec_idx_table, > > > > + id, sym, sym_sec_idx)) > > > > > > > > > gelf_getsymshndx() is supposed to work even for cases that don't use > > > extended numbering, so this should work, right? > > > > > > if (!gelf_getsymshndx(syms, syms_sec_idx_table, id, sym, sym_sec_idx)) > > > return false; > > > > > > > it seems you're right, gelf_getsymshndx seem to work for > > both cases, I'll check > > > > > > > if (sym->st_shndx == SHN_XINDEX) > > > *sym_sec_idx = sym->st_shndx; > > > > I don't understand this.. gelf_getsymshndx will return both > > symbol and proper index, no? also sym_sec_idx is already > > assigned from previou call > > Reading (some) implementation of gelf_getsymshndx() that I found > online, it won't set sym_sec_idx, if the symbol *doesn't* use extended > numbering. But it will still return symbol data. So to return the the latest upstream code seems to set it always, but I agree we should be careful Mark, any insight in here? thanks > section index in all cases, we need to check again *after* we got > symbol, and if it's not extended, then set index manually. hum, then we should use '!=', right? if (sym->st_shndx != SHN_XINDEX) *sym_sec_idx = sym->st_shndx; SNIP > > > so either > > > > > > for (id = 0; id < symtab->nr_syms && elf_sym__get(symtab->syms, > > > symtab->syms_sec_idx_table, d, &sym, &sym_sec_idx); id++) > > > > > > or > > > > > > for (id = 0; id < symtab->nr_syms; id++) > > > if (elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, d, &sym, > > > &sym_sec_idx)) > > > > if we go ahead with skipping symbols, this one seems good > > I think skipping symbols is nicer. If ELF is totally broken, then all > symbols are going to be ignored anyway. If it's some one-off issue for > a specific symbol, we'll just ignore it (unfortunately, silently). agreed, I'll use this thanks, jirka
On Sat, Jan 23, 2021 at 10:51 AM Jiri Olsa <jolsa@redhat.com> wrote: > > On Fri, Jan 22, 2021 at 02:55:51PM -0800, Andrii Nakryiko wrote: > > On Fri, Jan 22, 2021 at 12:47 PM Jiri Olsa <jolsa@redhat.com> wrote: > > > > > > On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: > > > > > > SNIP > > > > > > > > @@ -598,9 +599,36 @@ static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl) > > > > > fl->mcount_stop = sym->st_value; > > > > > } > > > > > > > > > > +static bool elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, > > > > > + int id, GElf_Sym *sym, Elf32_Word *sym_sec_idx) > > > > > +{ > > > > > + if (!gelf_getsym(syms, id, sym)) > > > > > + return false; > > > > > + > > > > > + *sym_sec_idx = sym->st_shndx; > > > > > + > > > > > + if (sym->st_shndx == SHN_XINDEX) { > > > > > + if (!syms_sec_idx_table) > > > > > + return false; > > > > > + if (!gelf_getsymshndx(syms, syms_sec_idx_table, > > > > > + id, sym, sym_sec_idx)) > > > > > > > > > > > > gelf_getsymshndx() is supposed to work even for cases that don't use > > > > extended numbering, so this should work, right? > > > > > > > > if (!gelf_getsymshndx(syms, syms_sec_idx_table, id, sym, sym_sec_idx)) > > > > return false; > > > > > > > > > > it seems you're right, gelf_getsymshndx seem to work for > > > both cases, I'll check > > > > > > > > > > if (sym->st_shndx == SHN_XINDEX) > > > > *sym_sec_idx = sym->st_shndx; > > > > > > I don't understand this.. gelf_getsymshndx will return both > > > symbol and proper index, no? also sym_sec_idx is already > > > assigned from previou call > > > > Reading (some) implementation of gelf_getsymshndx() that I found > > online, it won't set sym_sec_idx, if the symbol *doesn't* use extended > > numbering. But it will still return symbol data. So to return the > > the latest upstream code seems to set it always, > but I agree we should be careful oh, then maybe it's not necessary. I honestly don't even know where the authoritative source code of libelf is, so I just found some random source code with Google. > > Mark, any insight in here? thanks > > > section index in all cases, we need to check again *after* we got > > symbol, and if it's not extended, then set index manually. > > hum, then we should use '!=', right? > > if (sym->st_shndx != SHN_XINDEX) > *sym_sec_idx = sym->st_shndx; yeah, sorry, that was a typo > > SNIP > > > > > so either > > > > > > > > for (id = 0; id < symtab->nr_syms && elf_sym__get(symtab->syms, > > > > symtab->syms_sec_idx_table, d, &sym, &sym_sec_idx); id++) > > > > > > > > or > > > > > > > > for (id = 0; id < symtab->nr_syms; id++) > > > > if (elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, d, &sym, > > > > &sym_sec_idx)) > > > > > > if we go ahead with skipping symbols, this one seems good > > > > I think skipping symbols is nicer. If ELF is totally broken, then all > > symbols are going to be ignored anyway. If it's some one-off issue for > > a specific symbol, we'll just ignore it (unfortunately, silently). > > agreed, I'll use this sounds good > > thanks, > jirka >
Hi Jiri, On Sat, 2021-01-23 at 19:51 +0100, Jiri Olsa wrote: > On Fri, Jan 22, 2021 at 02:55:51PM -0800, Andrii Nakryiko wrote: > > > > > I don't understand this.. gelf_getsymshndx will return both > > > symbol and proper index, no? also sym_sec_idx is already > > > assigned from previou call > > > > Reading (some) implementation of gelf_getsymshndx() that I found > > online, it won't set sym_sec_idx, if the symbol *doesn't* use > > extended > > numbering. But it will still return symbol data. So to return the > > the latest upstream code seems to set it always, > but I agree we should be careful > > Mark, any insight in here? thanks GElf_Sym * gelf_getsymshndx (Elf_Data *symdata, Elf_Data *shndxdata, int ndx, GElf_Sym *dst, Elf32_Word *dstshndx) Will always set *dst, but only set *dstshndx if both it and shndxdata are not NULL and no error occurred (the function returns NULL and set libelf_error in case of error). So as long as shndxdata != NULL you can rely on *dstshndx being set. Otherwise you get the section index from dst->st_shndx. Cheers, Mark
On Sat, Jan 23, 2021 at 09:08:15PM +0100, Mark Wielaard wrote: > Hi Jiri, > > On Sat, 2021-01-23 at 19:51 +0100, Jiri Olsa wrote: > > On Fri, Jan 22, 2021 at 02:55:51PM -0800, Andrii Nakryiko wrote: > > > > > > > I don't understand this.. gelf_getsymshndx will return both > > > > symbol and proper index, no? also sym_sec_idx is already > > > > assigned from previou call > > > > > > Reading (some) implementation of gelf_getsymshndx() that I found > > > online, it won't set sym_sec_idx, if the symbol *doesn't* use > > > extended > > > numbering. But it will still return symbol data. So to return the > > > > the latest upstream code seems to set it always, > > but I agree we should be careful > > > > Mark, any insight in here? thanks > > GElf_Sym * > gelf_getsymshndx (Elf_Data *symdata, Elf_Data *shndxdata, int ndx, > GElf_Sym *dst, Elf32_Word *dstshndx) > > Will always set *dst, but only set *dstshndx if both it and shndxdata > are not NULL and no error occurred (the function returns NULL and set > libelf_error in case of error). > > So as long as shndxdata != NULL you can rely on *dstshndx being set. > Otherwise you get the section index from dst->st_shndx. ok, so it's as Andrii said, I'll make the extra check then thanks, jirka
Hi, On Sat, 2021-01-23 at 12:07 -0800, Andrii Nakryiko wrote: > > the latest upstream code seems to set it always, > > but I agree we should be careful > > oh, then maybe it's not necessary. I honestly don't even know where > the authoritative source code of libelf is, so I just found some > random source code with Google. The elfutils.org libelf implementation can be found here: https://sourceware.org/git/?p=elfutils.git;a=tree;f=libelf;hb=HEAD There are some other implementations, but some aren't maintained and others aren't packaged for any distro (anymore). libelf is a semi- standard "SVR4 Unix" library, so you might also find it for some none GNU/Linux OSes like Solaris. The ELF specification itself is contained in the System V Application Binary Interface (gABI). The libelf library itself isn't actually officially part of the specification. But we still do try to keep the implementations (source) compatible through the generic-abi mailinglist. Cheers, Mark
On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: SNIP > But the current variant looks broken. Oh, and > elf_symtab__for_each_symbol() is similarly broken, can you please fix > that as well? we'll have to change its callers a bit, because of hanging 'else' I'll send this separately if that's ok, when I figure out how to test ctf code jirka --- diff --git a/elf_symtab.h b/elf_symtab.h index 489e2b1a3505..6823a8c37ecf 100644 --- a/elf_symtab.h +++ b/elf_symtab.h @@ -99,10 +99,9 @@ elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, * @index: uint32_t index * @sym: GElf_Sym iterator */ -#define elf_symtab__for_each_symbol(symtab, index, sym) \ - for (index = 0, gelf_getsym(symtab->syms, index, &sym);\ - index < symtab->nr_syms; \ - index++, gelf_getsym(symtab->syms, index, &sym)) +#define elf_symtab__for_each_symbol(symtab, index, sym) \ + for (index = 0; index < symtab->nr_syms; index++) \ + if (gelf_getsym(symtab->syms, index, &sym)) /** * elf_symtab__for_each_symbol_index - iterate through all the symbols, diff --git a/libctf.h b/libctf.h index 749be8955c52..ee5412bec77e 100644 --- a/libctf.h +++ b/libctf.h @@ -90,11 +90,9 @@ char *ctf__string(struct ctf *ctf, uint32_t ref); */ #define ctf__for_each_symtab_function(ctf, index, sym) \ elf_symtab__for_each_symbol(ctf->symtab, index, sym) \ - if (ctf__ignore_symtab_function(&sym, \ + if (!ctf__ignore_symtab_function(&sym, \ elf_sym__name(&sym, \ ctf->symtab))) \ - continue; \ - else /** * ctf__for_each_symtab_object - iterate thru all the symtab objects @@ -105,11 +103,9 @@ char *ctf__string(struct ctf *ctf, uint32_t ref); */ #define ctf__for_each_symtab_object(ctf, index, sym) \ elf_symtab__for_each_symbol(ctf->symtab, index, sym) \ - if (ctf__ignore_symtab_object(&sym, \ + if (!ctf__ignore_symtab_object(&sym, \ elf_sym__name(&sym, \ ctf->symtab))) \ - continue; \ - else #endif /* _LIBCTF_H */
On Sat, Jan 23, 2021 at 1:23 PM Jiri Olsa <jolsa@redhat.com> wrote: > > On Thu, Jan 21, 2021 at 03:32:40PM -0800, Andrii Nakryiko wrote: > > SNIP > > > But the current variant looks broken. Oh, and > > elf_symtab__for_each_symbol() is similarly broken, can you please fix > > that as well? > > we'll have to change its callers a bit, because of hanging 'else' > I'll send this separately if that's ok, when I figure out how to > test ctf code > oh, else sucks. Sure, no problem doing it separately. > jirka > > > --- > diff --git a/elf_symtab.h b/elf_symtab.h > index 489e2b1a3505..6823a8c37ecf 100644 > --- a/elf_symtab.h > +++ b/elf_symtab.h > @@ -99,10 +99,9 @@ elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, > * @index: uint32_t index > * @sym: GElf_Sym iterator > */ > -#define elf_symtab__for_each_symbol(symtab, index, sym) \ > - for (index = 0, gelf_getsym(symtab->syms, index, &sym);\ > - index < symtab->nr_syms; \ > - index++, gelf_getsym(symtab->syms, index, &sym)) > +#define elf_symtab__for_each_symbol(symtab, index, sym) \ > + for (index = 0; index < symtab->nr_syms; index++) \ > + if (gelf_getsym(symtab->syms, index, &sym)) > > /** > * elf_symtab__for_each_symbol_index - iterate through all the symbols, > diff --git a/libctf.h b/libctf.h > index 749be8955c52..ee5412bec77e 100644 > --- a/libctf.h > +++ b/libctf.h > @@ -90,11 +90,9 @@ char *ctf__string(struct ctf *ctf, uint32_t ref); > */ > #define ctf__for_each_symtab_function(ctf, index, sym) \ > elf_symtab__for_each_symbol(ctf->symtab, index, sym) \ > - if (ctf__ignore_symtab_function(&sym, \ > + if (!ctf__ignore_symtab_function(&sym, \ > elf_sym__name(&sym, \ > ctf->symtab))) \ > - continue; \ > - else > > /** > * ctf__for_each_symtab_object - iterate thru all the symtab objects > @@ -105,11 +103,9 @@ char *ctf__string(struct ctf *ctf, uint32_t ref); > */ > #define ctf__for_each_symtab_object(ctf, index, sym) \ > elf_symtab__for_each_symbol(ctf->symtab, index, sym) \ > - if (ctf__ignore_symtab_object(&sym, \ > + if (!ctf__ignore_symtab_object(&sym, \ > elf_sym__name(&sym, \ > ctf->symtab))) \ > - continue; \ > - else > > > #endif /* _LIBCTF_H */ >
diff --git a/btf_encoder.c b/btf_encoder.c index 5557c9efd365..6e6f22c438ce 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -585,12 +585,13 @@ static int collect_percpu_var(struct btf_elf *btfe, GElf_Sym *sym) return 0; } -static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl) +static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl, + Elf32_Word sym_sec_idx) { if (!fl->mcount_start && !strcmp("__start_mcount_loc", elf_sym__name(sym, btfe->symtab))) { fl->mcount_start = sym->st_value; - fl->mcount_sec_idx = sym->st_shndx; + fl->mcount_sec_idx = sym_sec_idx; } if (!fl->mcount_stop && @@ -598,9 +599,36 @@ static void collect_symbol(GElf_Sym *sym, struct funcs_layout *fl) fl->mcount_stop = sym->st_value; } +static bool elf_sym__get(Elf_Data *syms, Elf_Data *syms_sec_idx_table, + int id, GElf_Sym *sym, Elf32_Word *sym_sec_idx) +{ + if (!gelf_getsym(syms, id, sym)) + return false; + + *sym_sec_idx = sym->st_shndx; + + if (sym->st_shndx == SHN_XINDEX) { + if (!syms_sec_idx_table) + return false; + if (!gelf_getsymshndx(syms, syms_sec_idx_table, + id, sym, sym_sec_idx)) + return false; + } + + return true; +} + +#define elf_symtab__for_each_symbol_index(symtab, id, sym, sym_sec_idx) \ + for (id = 0, elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, \ + id, &sym, &sym_sec_idx); \ + id < symtab->nr_syms; \ + id++, elf_sym__get(symtab->syms, symtab->syms_sec_idx_table, \ + id, &sym, &sym_sec_idx)) + static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars) { struct funcs_layout fl = { }; + Elf32_Word sym_sec_idx; uint32_t core_id; GElf_Sym sym; @@ -608,12 +636,12 @@ static int collect_symbols(struct btf_elf *btfe, bool collect_percpu_vars) percpu_var_cnt = 0; /* search within symtab for percpu variables */ - elf_symtab__for_each_symbol(btfe->symtab, core_id, sym) { + elf_symtab__for_each_symbol_index(btfe->symtab, core_id, sym, sym_sec_idx) { if (collect_percpu_vars && collect_percpu_var(btfe, &sym)) return -1; if (collect_function(btfe, &sym)) return -1; - collect_symbol(&sym, &fl); + collect_symbol(&sym, &fl, sym_sec_idx); } if (collect_percpu_vars) { diff --git a/elf_symtab.c b/elf_symtab.c index 741990ea3ed9..fad5e0c0ba3c 100644 --- a/elf_symtab.c +++ b/elf_symtab.c @@ -17,11 +17,13 @@ struct elf_symtab *elf_symtab__new(const char *name, Elf *elf, GElf_Ehdr *ehdr) { + size_t symtab_index; + if (name == NULL) name = ".symtab"; GElf_Shdr shdr; - Elf_Scn *sec = elf_section_by_name(elf, ehdr, &shdr, name, NULL); + Elf_Scn *sec = elf_section_by_name(elf, ehdr, &shdr, name, &symtab_index); if (sec == NULL) return NULL; @@ -41,6 +43,12 @@ struct elf_symtab *elf_symtab__new(const char *name, Elf *elf, GElf_Ehdr *ehdr) if (symtab->syms == NULL) goto out_free_name; + /* + * This returns extended section index table's + * section index, if it exists. + */ + int symtab_xindex = elf_scnshndx(sec); + sec = elf_getscn(elf, shdr.sh_link); if (sec == NULL) goto out_free_name; @@ -49,6 +57,35 @@ struct elf_symtab *elf_symtab__new(const char *name, Elf *elf, GElf_Ehdr *ehdr) if (symtab->symstrs == NULL) goto out_free_name; + /* + * The .symtab section has optional extended section index + * table, load its data so it can be used to resolve symbol's + * section index. + **/ + if (symtab_xindex > 0) { + GElf_Shdr shdr_xindex; + Elf_Scn *sec_xindex; + + sec_xindex = elf_getscn(elf, symtab_xindex); + if (sec_xindex == NULL) + goto out_free_name; + + if (gelf_getshdr(sec_xindex, &shdr_xindex) == NULL) + goto out_free_name; + + /* Extra check to verify it's correct type */ + if (shdr_xindex.sh_type != SHT_SYMTAB_SHNDX) + goto out_free_name; + + /* Extra check to verify it belongs to the .symtab */ + if (symtab_index != shdr_xindex.sh_link) + goto out_free_name; + + symtab->syms_sec_idx_table = elf_getdata(elf_getscn(elf, symtab_xindex), NULL); + if (symtab->syms_sec_idx_table == NULL) + goto out_free_name; + } + symtab->nr_syms = shdr.sh_size / shdr.sh_entsize; return symtab; diff --git a/elf_symtab.h b/elf_symtab.h index 359add69c8ab..2e05ca98158b 100644 --- a/elf_symtab.h +++ b/elf_symtab.h @@ -16,6 +16,8 @@ struct elf_symtab { uint32_t nr_syms; Elf_Data *syms; Elf_Data *symstrs; + /* Data of SHT_SYMTAB_SHNDX section. */ + Elf_Data *syms_sec_idx_table; char *name; };
For very large ELF objects (with many sections), we could get special value SHN_XINDEX (65535) for symbol's st_shndx. This patch is adding code to detect the optional extended section index table and use it to resolve symbol's section index. Adding elf_symtab__for_each_symbol_index macro that returns symbol's section index and usign it in collect_symbols function. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- btf_encoder.c | 36 ++++++++++++++++++++++++++++++++---- elf_symtab.c | 39 ++++++++++++++++++++++++++++++++++++++- elf_symtab.h | 2 ++ 3 files changed, 72 insertions(+), 5 deletions(-)