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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/InputFiles.cpp:306-309
+    // We discard comdat sections usually. When -r we should not do that. We
+    // still do deduplication in this case to simplify implementation, because
+    // otherwise merging group sections together would requre additional
+    // regeneration of its contents.
----------------
ruiu wrote:
> Move it after `case SHT_GROUP`
Done.


================
Comment at: ELF/InputSection.cpp:304
+  // First entry is a flag word, we leave it unchanged.
+  uint32_t Flag = read32(&From[0], Config->Endianness);
+  write32(To++, Flag, Config->Endianness);
----------------
ruiu wrote:
> You can remove this temporary variable.
Right. Done.


================
Comment at: ELF/OutputSections.cpp:122-123
+  // provides signature of the section group.
+  InputSection *First = Sec->Sections[0];
+  elf::ObjectFile<ELFT> *Obj = First->getFile<ELFT>();
+  ArrayRef<SymbolBody *> Symbols = Obj->getSymbols();
----------------
ruiu wrote:
> It took for a while to understand this code. What was confusing is that the first section is not special. It is just that when you pass -r, we create an output section for each input section, so there's always only one section in an output file.
> 
> Please add
> 
>   assert(Config->Relocatable && Sec->Sections.size == 1);
> 
> remove `First` variable and use Sec->Sections[0] directly. This should improve readability.
Done.


https://reviews.llvm.org/D33485