[PATCH] D62349: Change ELF tools to allow multiple dynsym, versym, verdef and verneed sections per file.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D62349#1515398 <https://reviews.llvm.org/D62349#1515398>, @grimar wrote:

> I think you can add a test using yaml2obj probably. Can you?
>  Also, does it really pass existent test cases? (I would expect to see some of them testing "Multiple SHT_*" message)
>
> And ideal behavior will be to dump all the sections, right?
>  Perhaps it worth to report a bug then. It is probably possible to work on that with use of yaml2obj tool
>  for creating the tests.


You might want to enhance yaml2obj to allow multiple sections with the same name, although I don't think it's strictly needed. On taking the first one, do GNU tools do this? Or do they take the last one/emit an error/print all? I see no reason to emit an error here, so the change in behaviour is good, but if it picks the last, we should probably do the same. As @grimar mentions, if GNU dumps all, we should file bugs.

On the LLVM-specific sections, I think it would make more sense to just do what we do with the normal sections.



================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:948-949
     case ELF::SHT_SYMTAB: {
       if (DotSymtabSec)
         return createError("More than one static symbol table!");
       DotSymtabSec = &Sec;
----------------
What about this case?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1429-1430
     case ELF::SHT_LLVM_CALL_GRAPH_PROFILE:
       if (DotCGProfileSec != nullptr)
         reportError("Multiple .llvm.call-graph-profile");
       DotCGProfileSec = &Sec;
----------------
Ditto.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1434-1435
     case ELF::SHT_LLVM_ADDRSIG:
       if (DotAddrsigSec != nullptr)
         reportError("Multiple .llvm_addrsig");
       DotAddrsigSec = &Sec;
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62349