[PATCH] D33485: [ELF] - Do not allow -r to eat comdats.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/InputFiles.cpp:306
     switch (Sec.sh_type) {
-    case SHT_GROUP:
-      this->Sections[I] = &InputSection::Discarded;
-      if (ComdatGroups.insert(getShtGroupSignature(ObjSections, Sec)).second)
+    case SHT_GROUP: {
+      bool New =
----------------
This needs an explanation.


================
Comment at: ELF/InputFiles.cpp:322
       }
-      break;
+    } break;
     case SHT_SYMTAB:
----------------
Format


================
Comment at: ELF/InputSection.cpp:302
+  // First entry is a flag word, we leave it unchanged.
+  auto *P = reinterpret_cast<typename ELFT::Word *>(Buf);
+  *P++ = Ents[0];
----------------
Avoid ELFT::Word` in general. You can use read32 instead. You can also remove ELFT from template parameter list if you use Config->Wordsize.


================
Comment at: ELF/InputSection.cpp:701
+    copyGroup<ELFT>(Buf + OutSecOff,
+                    this->template getDataAs<typename ELFT::Word>());
+    return;
----------------
Do not use ELFT::Word.


================
Comment at: ELF/InputSection.h:324
+  template <class ELFT, class EntTy>
+  void copyGroup(uint8_t *Buf, llvm::ArrayRef<EntTy> Ents);
 };
----------------
I prefer more specific name -- like copyShtGroup, as `Group` sounds too generic.


================
Comment at: ELF/OutputSections.cpp:129-141
+  if (Type == SHT_GROUP) {
+    // sh_link field for SHT_GROUP sections should contain the section index of
+    // the symbol table.
+    this->Link = InX::SymTab->OutSec->SectionIndex;
+
+    // sh_link then contain index of an entry in symbol table section which
+    // provides signature of the section group.
----------------
This is too much details to write in this funciton.


================
Comment at: ELF/OutputSections.cpp:276
 
 static uint64_t getOutFlags(InputSectionBase *S) {
+  if (Config->Relocatable)
----------------
This looks a bit tricky. We should turn off SHF_COMPRESSED when we decompress sections.


https://reviews.llvm.org/D33485