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

[grimar]

Summary

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.

[davide]

We should really move away from a model where we check in a binary for the dumper output.
Can you try to write a YAML file to generate the same object? (I think mc won't do it).

[grimar]

• Used yaml2obj for producing testcase instead of using precompiled binary.

Note: there is patch for printGroupSections() refactoring on review, it is D37621,
it may make sence to land it first probably and rebase this one after that.

[grimar]

• Rebased after r313236.

[grimar]

• Print introduced error to stderr instead of stdout. This is consistent with GNU readelf behavior.

llvm/trunk/test/tools/llvm-readobj/broken-group.test

+# RUN: yaml2obj %s -o %t.o
+
+# RUN: llvm-readobj --elf-section-groups -elf-output-style=GNU \
+# RUN:   %t.o >%tout.log 2>%terr.log
+# RUN: FileCheck %s -check-prefix=GNU-STDERR < %terr.log
+# GNU-STDERR: Error: section [    3] in group section [    4] already in group section [    2]
+# RUN: FileCheck %s -check-prefix=GNU-STDOUT < %tout.log
+# GNU-STDOUT:      COMDAT group section [    2] `.group' [bar] contains 1 sections:
+# GNU-STDOUT-NEXT:   [Index]    Name
+# GNU-STDOUT-NEXT:   [    3]   .foo
+# GNU-STDOUT:      COMDAT group section [    4] `.group1' [zed] contains 1 sections:
+# GNU-STDOUT-NEXT:   [Index]    Name
+
+# RUN: llvm-readobj --elf-section-groups \
+# RUN:   %t.o >%t2out.log 2>%t2err.log %t.o
+# RUN: FileCheck %s -check-prefix=LLVM-STDERR < %t2err.log
+# LLVM-STDERR: Error: .foo (3) in a group .group1 (4) is already in a group .group (2)
+# RUN: FileCheck %s -check-prefix=LLVM-STDOUT < %t2out.log
+# LLVM-STDOUT:      Groups {
+# LLVM-STDOUT-NEXT:   Group {
+# LLVM-STDOUT-NEXT:     Name: .group
+# LLVM-STDOUT-NEXT:     Index: 2
+# LLVM-STDOUT-NEXT:     Type: COMDAT
+# LLVM-STDOUT-NEXT:     Signature: bar
+# LLVM-STDOUT-NEXT:     Section(s) in group [
+# LLVM-STDOUT-NEXT:       .foo (3)
+# LLVM-STDOUT-NEXT:     ]
+# LLVM-STDOUT-NEXT:   }
+# LLVM-STDOUT-NEXT:   Group {
+# LLVM-STDOUT-NEXT:     Name: .group1
+# LLVM-STDOUT-NEXT:     Index: 4
+# LLVM-STDOUT-NEXT:     Type: COMDAT
+# LLVM-STDOUT-NEXT:     Signature: zed
+# LLVM-STDOUT-NEXT:     Section(s) in group [
+# LLVM-STDOUT-NEXT:     ]
+# LLVM-STDOUT-NEXT:   }
+# LLVM-STDOUT-NEXT: }
+
+!ELF
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         ''
+  - Name:            .group
+    Type:            SHT_GROUP
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            bar
+    Members:         
+      - SectionOrType:   GRP_COMDAT
+      - SectionOrType:   .foo
+  - Name:            .foo
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .group1
+    Type:            SHT_GROUP
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            zed
+    Members:         
+      - SectionOrType:   GRP_COMDAT
+      - SectionOrType:   .foo
+Symbols:         
+  Local:           
+    - Name:            bar
+      Section:         .group
+    - Name:            zed
+      Section:         .group1

llvm/trunk/tools/llvm-readobj/ELFDumper.cpp

 //===----------------------------------------------------------------------===//
 
 #include "ARMEHABIPrinter.h"
 #include "Error.h"
 #include "ObjDumper.h"
 #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"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
for (const Elf_Shdr &Sec : unwrapOrError(Obj->sections())) {
     for (uint32_t Ndx : Data.slice(1)) {
       auto Sec = unwrapOrError(Obj->getSection(Ndx));
       const StringRef Name = unwrapOrError(Obj->getSectionName(Sec));
       GM.push_back({Name, Ndx});
     }
   }
   return Ret;
 }
+
+DenseMap<uint64_t, const GroupSection *>
+mapSectionsToGroups(ArrayRef<GroupSection> Groups) {
+  DenseMap<uint64_t, const GroupSection *> Ret;
+  for (const GroupSection &G : Groups)
+    for (const GroupMember &GM : G.Members)
+      Ret.insert({GM.Index, &G});
+  return Ret;
+}
+
 } // namespace
 
 template <class ELFT> void GNUStyle<ELFT>::printGroupSections(const ELFO *Obj) {
   std::vector<GroupSection> V = getGroups<ELFT>(Obj);
+  DenseMap<uint64_t, const GroupSection *> Map = mapSectionsToGroups(V);
   for (const GroupSection &G : V) {
     OS << "\n"
        << getGroupType(G.Type) << " group section ["
        << format_decimal(G.Index, 5) << "] `" << G.Name << "' [" << G.Signature
        << "] contains " << G.Members.size() << " sections:\n"
        << "   [Index]    Name\n";
-    for (const GroupMember &GM : G.Members)
+    for (const GroupMember &GM : G.Members) {
+      const GroupSection *MainGroup = Map[GM.Index];
+      if (MainGroup != &G) {
+        OS.flush();
+        errs() << "Error: section [" << format_decimal(GM.Index, 5)
+               << "] in group section [" << format_decimal(G.Index, 5)
+               << "] already in group section ["
+               << format_decimal(MainGroup->Index, 5) << "]";
+        errs().flush();
+        continue;
+      }
       OS << "   [" << format_decimal(GM.Index, 5) << "]   " << GM.Name << "\n";
     }
+  }
 
   if (V.empty())
     OS << "There are no section groups in this file.\n";
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printRelocation(const ELFO *Obj, const Elf_Shdr *SymTab,
                                      const Elf_Rela &R, bool IsRela) {
const Elf_Ehdr *e = Obj->getHeader();
     W.printNumber("StringTableSectionIndex", e->e_shstrndx);
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printGroupSections(const ELFO *Obj) {
   DictScope Lists(W, "Groups");
   std::vector<GroupSection> V = getGroups<ELFT>(Obj);
+  DenseMap<uint64_t, const GroupSection *> Map = mapSectionsToGroups(V);
   for (const GroupSection &G : V) {
     DictScope D(W, "Group");
     W.printNumber("Name", G.Name, G.ShName);
     W.printNumber("Index", G.Index);
     W.printHex("Type", getGroupType(G.Type), G.Type);
     W.startLine() << "Signature: " << G.Signature << "\n";
 
     ListScope L(W, "Section(s) in group");
-    for (const GroupMember &GM : G.Members)
+    for (const GroupMember &GM : G.Members) {
+      const GroupSection *MainGroup = Map[GM.Index];
+      if (MainGroup != &G) {
+        W.flush();
+        errs() << "Error: " << GM.Name << " (" << GM.Index
+               << ") in a group " + G.Name + " (" << G.Index
+               << ") is already in a group " + MainGroup->Name + " ("
+               << MainGroup->Index << ")\n";
+        errs().flush();
+        continue;
+      }
       W.startLine() << GM.Name << " (" << GM.Index << ")\n";
     }
+  }
 
   if (V.empty())
     W.startLine() << "There are no group sections in the file.\n";
 }
+
 template <class ELFT> void LLVMStyle<ELFT>::printRelocations(const ELFO *Obj) {
   ListScope D(W, "Relocations");
 
   int SectionNumber = -1;
   for (const Elf_Shdr &Sec : unwrapOrError(Obj->sections())) {
     ++SectionNumber;
 
     if (Sec.sh_type != ELF::SHT_REL && Sec.sh_type != ELF::SHT_RELA)