[ELF] - Move the addition of synthetics from addPredefinedSections()

[grimar]

Summary

These were 3 last synthetics that were added in addPredefinedSections() instead
of createSyntheticSections(). Now it is possible to move addition to correct common place.

Also patch fixes testcase which discards .shstrtab, by restricting doing that.

[ruiu]

LGTM

ELF/Writer.cpp

1600–1601

I'd think this is slightly more readable.

if (In<ELFT>::ShStrTab->OutSec)
  EHdr->e_shstrndx = In<ELFT>::ShStrTab->OutSec->SectionIndex;

[grimar]

ELF/Writer.cpp

1600–1601

May be. At least it shorter. Though this form does not explicitly assign SHN_UNDEF, it is a luck that SHN_UNDEF == 0.
I usually prefer not to assume values for named constants, even for such cases.

I'll use your suggestion to land this patch after resolving D28560 dependency as its seems a matter of taste in this place.

[grimar]

• Discarding .shstrtab is restricted.

ELF/Writer.cpp

StringRef elf::getOutputSectionName(StringRef Name) {
   // ".zdebug_" is a prefix for ZLIB-compressed sections.
   // Because we decompressed input sections, we want to remove 'z'.
   if (Name.startswith(".zdebug_"))
     return Saver.save(Twine(".") + Name.substr(2));
   return Name;
 }
 
 template <class ELFT> void elf::reportDiscarded(InputSectionBase<ELFT> *IS) {
+  if (IS == In<ELFT>::ShStrTab)
+    error("discarding .shstrtab section is not allowed");
   if (!Config->PrintGcSections)
     return;
   errs() << "removing unused section from '" << IS->Name << "' in file '"
          << IS->getFile()->getName() << "'\n";
 }
 
 template <class ELFT> static bool needsInterpSection() {
   return !Symtab<ELFT>::X->getSharedFiles().empty() &&
template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
   Symtab<ELFT>::X->Sections.push_back(In<ELFT>::Plt);
   In<ELFT>::Iplt = make<IpltSection<ELFT>>();
   Symtab<ELFT>::X->Sections.push_back(In<ELFT>::Iplt);
 
   if (Config->EhFrameHdr) {
     In<ELFT>::EhFrameHdr = make<EhFrameHeader<ELFT>>();
     Symtab<ELFT>::X->Sections.push_back(In<ELFT>::EhFrameHdr);
   }
+
+  if (In<ELFT>::SymTab)
+    Symtab<ELFT>::X->Sections.push_back(In<ELFT>::SymTab);
+  Symtab<ELFT>::X->Sections.push_back(In<ELFT>::ShStrTab);
+  if (In<ELFT>::StrTab)
+    Symtab<ELFT>::X->Sections.push_back(In<ELFT>::StrTab);
 }
 
 template <class ELFT>
 static bool shouldKeepInSymtab(InputSectionBase<ELFT> *Sec, StringRef SymName,
                                const SymbolBody &B) {
   if (B.isFile())
     return false;
 
template <class ELFT> void Writer<ELFT>::addPredefinedSections() {
   if (Out<ELFT>::Bss->Size > 0)
     OutputSections.push_back(Out<ELFT>::Bss);
   if (Out<ELFT>::BssRelRo->Size > 0)
     OutputSections.push_back(Out<ELFT>::BssRelRo);
 
   auto OS = dyn_cast_or_null<OutputSection<ELFT>>(findSection(".ARM.exidx"));
   if (OS && !OS->Sections.empty() && !Config->Relocatable)
     OS->addSection(make<ARMExidxSentinelSection<ELFT>>());
-
-  addInputSec(In<ELFT>::SymTab);
-  addInputSec(In<ELFT>::ShStrTab);
-  addInputSec(In<ELFT>::StrTab);
 }
 
 // The linker is expected to define SECNAME_start and SECNAME_end
 // symbols for a few sections. This function defines them.
 template <class ELFT> void Writer<ELFT>::addStartEndSymbols() {
   auto Define = [&](StringRef Start, StringRef End, OutputSectionBase *OS) {
     // These symbols resolve to the image base if the section does not exist.
     // A special value -1 indicates end of the section.
template <class ELFT> void Writer<ELFT>::writeHeader() {
   EHdr->e_machine = Config->EMachine;
   EHdr->e_version = EV_CURRENT;
   EHdr->e_entry = getEntryAddr();
   EHdr->e_shoff = SectionHeaderOff;
   EHdr->e_ehsize = sizeof(Elf_Ehdr);
   EHdr->e_phnum = Phdrs.size();
   EHdr->e_shentsize = sizeof(Elf_Shdr);
   EHdr->e_shnum = OutputSections.size() + 1;
   EHdr->e_shstrndx = In<ELFT>::ShStrTab->OutSec->SectionIndex;
 
   if (Config->EMachine == EM_ARM)
     // We don't currently use any features incompatible with EF_ARM_EABI_VER5,
     // but we don't have any firm guarantees of conformance. Linux AArch64
     // kernels (as of 2016) require an EABI version to be set.
     EHdr->e_flags = EF_ARM_EABI_VER5;
   else if (Config->EMachine == EM_MIPS)
     EHdr->e_flags = getMipsEFlags<ELFT>();
 

test/ELF/linkerscript/sections.s

 # SEC-SWAP-NAMES: 2 .text         00000020 {{[0-9a-f]*}} DATA
 # SEC-SWAP-NAMES: 3 other         00000003 {{[0-9a-f]*}} DATA
 # SEC-SWAP-NAMES: 4 .bss          00000002 {{[0-9a-f]*}} BSS
 # SEC-SWAP-NAMES: 5 .comment      00000008 {{[0-9a-f]*}}
 # SEC-SWAP-NAMES: 6 .symtab       00000030 {{[0-9a-f]*}}
 # SEC-SWAP-NAMES: 7 .shstrtab     0000003b {{[0-9a-f]*}}
 # SEC-SWAP-NAMES: 8 .strtab       00000008 {{[0-9a-f]*}}
 
-# .shstrtab from the input object file is discarded.
-# RUN: echo "SECTIONS { \
-# RUN:          /DISCARD/ : { *(.shstrtab) } }" > %t.script
-# RUN: ld.lld -o %t5 --script %t.script %t
-# RUN: llvm-objdump -section-headers %t5 | \
+# Attemp to discard .shstrtab section.
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.shstrtab) } }" > %t.script
+# RUN: not ld.lld -o %t5 --script %t.script %t 2>&1 | \
 # RUN:   FileCheck -check-prefix=SEC-DISCARD %s
-
-#             Idx Name          Size
-# SEC-DISCARD: 1 .text         0000000e {{[0-9a-f]*}} TEXT DATA
-# SEC-DISCARD: 2 .data         00000020 {{[0-9a-f]*}} DATA
-# SEC-DISCARD: 3 other         00000003 {{[0-9a-f]*}} DATA
-# SEC-DISCARD: 4 .bss          00000002 {{[0-9a-f]*}} BSS
-# SEC-DISCARD: 5 .comment      00000008 {{[0-9a-f]*}}
-# SEC-DISCARD: 6 .symtab       00000030 {{[0-9a-f]*}}
-# SEC-DISCARD: 7 .shstrtab     0000003b {{[0-9a-f]*}}
-# SEC-DISCARD: 8 .strtab       00000008 {{[0-9a-f]*}}
+# SEC-DISCARD: discarding .shstrtab section is not allowed
 
 # Multiple SECTIONS command specifying additional input section descriptions
 # for the same output section description - input sections are merged into
 # one output section.
 # RUN: echo "SECTIONS { \
 # RUN:          .text : { *(.text) } \
 # RUN:          .data : { *(.data) } } \
 # RUN:       SECTIONS { \