[PATCH] D54697: [llvm-objdump] Add `Version References` dumper
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 25 00:47:34 PST 2019
grimar added inline comments.
================
Comment at: test/tools/llvm-objdump/verneed-wrong-info.test:5
+# We have a SHT_GNU_verneed section with a broken sh_info field
+# that says the section contains more entries that it actually have
+
----------------
that->than, have->has (sorry, did these mistakes/mistypes when suggested it).
I hope @jhenderson will re-check it though :)
================
Comment at: tools/llvm-objdump/ELFDump.cpp:288
+ }
+ }
+
----------------
Higuoxing wrote:
> grimar wrote:
> > If you move this loop to `printSymbolVersionInfo` and change the `printSymbolVersionDependency` signature
> > to `printSymbolVersionDependency(const typename ELFT::Shdr &Shdr, ArrayRef<uint8_t> Contents)`
> > that probably will be better, because for `printSymbolVersionDefinition` implementation you would also
> > need to iterate over all sections and take the content. That can be done in a single place.
> >
> >
> > I mean the code in `printSymbolVersionInfo` can be like:
> >
> > ```
> > template <class ELFT>
> > static void printSymbolVersionInfo(const ELFFile<ELFT> *Elf) {
> > typedef typename ELFT::Shdr Elf_Shdr;
> >
> > auto SecOrErr = Elf->sections();
> > if (!SecOrErr)
> > report_fatal_error(errorToErrorCode(SecOrErr.takeError()).message());
> >
> > for (const Elf_Shdr &Shdr : *SecOrErr) {
> > if (Shdr.sh_type != ELF::SHT_GNU_verdef &&
> > Shdr.sh_type != ELF::SHT_GNU_verneed)
> > continue;
> >
> > auto ContentsOrErr = Elf->getSectionContents(&Shdr);
> > if (!ContentsOrErr)
> > report_fatal_error(errorToErrorCode(ContentsOrErr.takeError()).message());
> >
> > if (Shdr.sh_type == ELF::SHT_GNU_verdef)
> > printSymbolVersionDefinition<ELFT>(Shdr, *ContentsOrErr);
> > else if (Shdr.sh_type == ELF::SHT_GNU_verneed)
> > printSymbolVersionDependency<ELFT>(Shdr, *ContentsOrErr);
> > }
> > }
> > ```
> >
> >
> Oh, thank you @grimar , I am wandering that whether the dumping order of verneed and verdef sections is important? Sometimes, the verneed section is before verdef and sometimes verneed is after verdef.
I did not notice that, but if so then I guess GNU objdump just print them in the same order as they are in a section table probably.
i.e. the same is done in this patch.
================
Comment at: tools/llvm-objdump/ELFDump.cpp:303
+template <class ELFT>
+void printSymbolVersionInfo(const ELFFile<ELFT> *Elf, StringRef FileName) {
+ typedef typename ELFT::Shdr Elf_Shdr;
----------------
Shouldn't you be able to get `Filename` from `Elf`?
================
Comment at: tools/llvm-objdump/ELFDump.cpp:313
+ Shdr.sh_type != ELF::SHT_GNU_verneed)
+ continue;
+
----------------
If you're not going to implement `SHT_GNU_verneed` in this patch then I think you should
get rid of `Shdr.sh_type != ELF::SHT_GNU_verneed`.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54697/new/
https://reviews.llvm.org/D54697
More information about the llvm-commits
mailing list