[PATCH] D37567: [llvm-readobj] - Teach tool to report error if some section is in multiple COMDAT groups at once.

[Rafael Avila de Espindola via llvm-commits]

Can the testcase be created with yaml2obj?

Can we have a common code path for printing the error in both cases?

Should the error go to stderr?

Should we exit with 1 when there is an error?

Cheers,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
>
> This is relative to PR34506,
> current LLD head produces broken output. It places the same section into 2 COMDAT groups.
> gnu readelf tool reports an error in this case.
>
> Patch teaches llvm-readobj to do the same.
>
>
> https://reviews.llvm.org/D37567
>
> Files:
>   test/tools/llvm-readobj/Inputs/broken-group.obj.elf-x86_64
>   test/tools/llvm-readobj/broken-group.test
>   tools/llvm-readobj/ELFDumper.cpp
>
> Index: tools/llvm-readobj/ELFDumper.cpp
> ===================================================================
> --- tools/llvm-readobj/ELFDumper.cpp
> +++ tools/llvm-readobj/ELFDumper.cpp
> @@ -18,6 +18,7 @@
>  #include "StackMapPrinter.h"
>  #include "llvm-readobj.h"
>  #include "llvm/ADT/ArrayRef.h"
> +#include "llvm/ADT/DenseMap.h"
>  #include "llvm/ADT/Optional.h"
>  #include "llvm/ADT/PointerIntPair.h"
>  #include "llvm/ADT/SmallString.h"
> @@ -2444,6 +2445,7 @@
>  template <class ELFT> void GNUStyle<ELFT>::printGroupSections(const ELFO *Obj) {
>    uint32_t SectionIndex = 0;
>    bool HasGroups = false;
> +  DenseMap<uint32_t, uint32_t> SectionToGroup;
>    for (const Elf_Shdr &Sec : unwrapOrError(Obj->sections())) {
>      if (Sec.sh_type == ELF::SHT_GROUP) {
>        HasGroups = true;
> @@ -2459,7 +2461,15 @@
>           << StrTable.data() + Signature->st_name << "] contains "
>           << (Data.size() - 1) << " sections:\n"
>           << "   [Index]    Name\n";
> -      for (auto &Ndx : Data.slice(1)) {
> +      for (Elf_Word Ndx : Data.slice(1)) {
> +        auto It = SectionToGroup.insert({Ndx, SectionIndex});
> +        if (!It.second) {
> +          OS << "Error: section [" << format_decimal(Ndx, 5)
> +             << "] in group section [" << format_decimal(SectionIndex, 5)
> +             << "] already in group section ["
> +             << format_decimal(It.first->second, 5) << "]";
> +          continue;
> +        }
>          auto Sec = unwrapOrError(Obj->getSection(Ndx));
>          const StringRef Name = unwrapOrError(Obj->getSectionName(Sec));
>          OS << "   [" << format_decimal(Ndx, 5) << "]   " << Name
> @@ -3491,6 +3501,7 @@
>    DictScope Lists(W, "Groups");
>    uint32_t SectionIndex = 0;
>    bool HasGroups = false;
> +  DenseMap<uint32_t, std::pair<StringRef, uint32_t>> SectionToGroup;
>    for (const Elf_Shdr &Sec : unwrapOrError(Obj->sections())) {
>      if (Sec.sh_type == ELF::SHT_GROUP) {
>        HasGroups = true;
> @@ -3501,18 +3512,28 @@
>        auto Data = unwrapOrError(
>            Obj->template getSectionContentsAsArray<Elf_Word>(&Sec));
>        DictScope D(W, "Group");
> -      StringRef Name = unwrapOrError(Obj->getSectionName(&Sec));
> -      W.printNumber("Name", Name, Sec.sh_name);
> +      StringRef GroupName = unwrapOrError(Obj->getSectionName(&Sec));
> +      W.printNumber("Name", GroupName, Sec.sh_name);
>        W.printNumber("Index", SectionIndex);
>        W.printHex("Type", getGroupType(Data[0]), Data[0]);
>        W.startLine() << "Signature: " << StrTable.data() + Sym->st_name << "\n";
>        {
>          ListScope L(W, "Section(s) in group");
>          size_t Member = 1;
>          while (Member < Data.size()) {
>            auto Sec = unwrapOrError(Obj->getSection(Data[Member]));
> -          const StringRef Name = unwrapOrError(Obj->getSectionName(Sec));
> -          W.startLine() << Name << " (" << Data[Member++] << ")\n";
> +          Elf_Word Ndx = Data[Member++];
> +          const StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
> +          auto It = SectionToGroup.insert({Ndx, {GroupName, SectionIndex}});
> +          if (!It.second) {
> +            StringRef GroupName = It.first->second.first;
> +            uint32_t GroupIndex = It.first->second.second;
> +            W.startLine() << "Error: " << SecName << " (" << Ndx
> +                          << ") already in a group " + GroupName + " ("
> +                          << GroupIndex << ")\n";
> +            continue;
> +          }
> +          W.startLine() << SecName << " (" << Ndx << ")\n";
>          }
>        }
>      }
> Index: test/tools/llvm-readobj/broken-group.test
> ===================================================================
> --- test/tools/llvm-readobj/broken-group.test
> +++ test/tools/llvm-readobj/broken-group.test
> @@ -0,0 +1,41 @@
> +broken-group.obj.elf-x86_64 contans 2 COMDAT groups and each one contains
> +the same section foo. This is a result of bug in linker.
> +Testcase shows that llvm-readobj reports error in that case.
> +Linker used: LLD 6.0.0 (trunk 310596) (compatible with GNU linkers)
> +Code:
> +.section        .foo,"axG", at progbits,bar,comdat
> +.section        .foo,"axG", at progbits,zed,comdat
> +Invocation:
> +ld.lld -r test.o -o out.lld
> +
> +RUN: llvm-readobj --elf-section-groups -elf-output-style=GNU \
> +RUN:   %p/Inputs/broken-group.obj.elf-x86_64 | FileCheck %s -check-prefix=GNU
> +GNU:      COMDAT group section [    2] `.group' [bar] contains 1 sections:
> +GNU-NEXT:   [Index]    Name
> +GNU-NEXT:   [    3]   .foo
> +GNU:      COMDAT group section [    4] `.group' [zed] contains 1 sections:
> +GNU-NEXT:   [Index]    Name
> +GNU-NEXT:   Error: section [    3] in group section [    4] already in group section [    2]
> +
> +RUN: llvm-readobj --elf-section-groups \
> +RUN:   %p/Inputs/broken-group.obj.elf-x86_64 | FileCheck %s -check-prefix=LLVM
> +LLVM:      Groups {
> +LLVM-NEXT:   Group {
> +LLVM-NEXT:     Name: .group
> +LLVM-NEXT:     Index: 2
> +LLVM-NEXT:     Type: COMDAT
> +LLVM-NEXT:     Signature: bar
> +LLVM-NEXT:     Section(s) in group [
> +LLVM-NEXT:       .foo (3)
> +LLVM-NEXT:     ]
> +LLVM-NEXT:   }
> +LLVM-NEXT:   Group {
> +LLVM-NEXT:     Name: .group
> +LLVM-NEXT:     Index: 4
> +LLVM-NEXT:     Type: COMDAT
> +LLVM-NEXT:     Signature: zed
> +LLVM-NEXT:     Section(s) in group [
> +LLVM-NEXT:       Error: .foo (3) already in a group .group (2)
> +LLVM-NEXT:     ]
> +LLVM-NEXT:   }
> +LLVM-NEXT: }