[PATCH] D84231: [llvm-readobj] - Don't get the name of the symbol table in ELFDumper<ELFT>::printSymbolsHelper.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 22 05:41:56 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/Object/invalid.test:292
+# INVALID-SECTION-SIZE2-NEXT:  Other: 0
+# INVALID-SECTION-SIZE2-NEXT: warning: '[[FILE]]': section [index 1] has a sh_offset (0xffffffff) + sh_size (0x1b) that cannot be represented
+# INVALID-SECTION-SIZE2-NEXT:  Section: <?> (0x1)
----------------
jhenderson wrote:
> This seems to be missing context that was useful before? In other words, what went wrong here?
The llvm-readobj doesn't try to read a name for a symbol table in `ELFDumper<ELFT>::printSymbolsHelper`
for no reason anymore.

The original YAML produces no warning with this patch. I had to change it slightly
to force the llvm-readobj code to try to read the `.shstrtab` section name again.
So the different message is shown, but the reason (an invalid sh_offset, which is tested here) remains the same,




================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3982
+      this->reportUniqueWarning(createError(
+          "unable to get the name of the symbol table section with index " +
+          Twine(SecNdx) + ": " + toString(NameOrErr.takeError())));
----------------
jhenderson wrote:
> MaskRay wrote:
> > The original messages carry more information as a symbol table can represent either  SHT_DYNSYM or SHT_SYMTAB... I am fine combing them, though
> Could you use the new `describe` function from D84240 to resolve that?
> Could you use the new describe function from D84240 to resolve that?

Yes. Done.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84231/new/

https://reviews.llvm.org/D84231





More information about the llvm-commits mailing list