[ELF] - Fixed behavior when amount of inputsections is too large.

[grimar]

Summary

There were 2 problems in this case. Imagine that anount of input sections
is incorrect and huge (> UINT32_MAX). Lets say it is x = UINT32_MAX + 1;

1. On 64bit systems it just may crash with std::length_error when will try to allocate

more memory than is available.

2. On 32bits situation is more interesting.

Sections.resize(Size);

will truncate Size to 0, so resize will work fine.
Next loop will never execute:

for (const Elf_Shdr &Sec : Obj.sections())

because Obj.sections() will return empty array because of implementation of:

template <class ELFT>
const typename ELFFile<ELFT>::Elf_Shdr *ELFFile<ELFT>::section_end() const {
  return section_begin() + getNumSections();
}

And finally linkage can complete fine without errors. That is what I am observing with testcase.
I suggest to limit amount of input sections to some reasonable amount. Not sure what is better
UINT16_MAX or UINT32_MAX., patch uses first for now.

[ruiu]

I got a feeling that these error checks are getting picky. It is OK to reject crazy numbers that only crash the linker, but essentially we should do what the object files tell us do. At least object files containing >65536 secitons seem plausible.

[grimar]

• Updated logic.

[grimar]

In D25272#562436, @ruiu wrote:

I got a feeling that these error checks are getting picky. It is OK to reject crazy numbers that only crash the linker, but essentially we should do what the object files tell us do. At least object files containing >65536 secitons seem plausible.

What about current diff ? This change allows to move all check logic to inside Obj.sections(). Where I am planning to check if
size of total amount of sections exceeds the file buffer size.

[ruiu]

Isn't this the same as the previous code?

[grimar]

In D25272#566393, @ruiu wrote:

Isn't this the same as the previous code?

Not exactly. It has almost the same functionality. My motivation was following. When we have code like:

uint64_t Size = this->ELFObj.getNumSections();
Sections.resize(Size);

Then we are almost unable to do something with possible crash and overflow on lld side. Ideally I think we do not want to insert checks in getNumSections() probably as some utils like llvm-objdump for example may want to show the value even if it is incorrect. So issue checking should happen only if we try to access the section content.
So plan was to do some checking in Obj.sections().

At the same time current Object/ELF.h code already has some code that has checks for amount of sections. And it do that on object loading. Code does not work well though, and I posted first patch for it: D25432. Though I am not sure that doing a check during loading is the best way.

Lets put this on hold until some changes in ELF.h be landed.

[grimar]

Depricated.

ELF/InputFiles.cpp

if (Flags & SHF_STRINGS)
     return true;
 
   return Sec.sh_addralign <= EntSize;
 }
 
 template <class ELFT>
 void elf::ObjectFile<ELFT>::initializeSections(
     DenseSet<StringRef> &ComdatGroups) {
-  uint64_t Size = this->ELFObj.getNumSections();
-  Sections.resize(Size);
-  unsigned I = -1;
   const ELFFile<ELFT> &Obj = this->ELFObj;
-  for (const Elf_Shdr &Sec : Obj.sections()) {
+  typename ELFT::ShdrRange R = Obj.sections();
+
+  Sections.resize(R.size());
+  unsigned I = -1;
+  for (const Elf_Shdr &Sec : R) {
     ++I;
     if (Sections[I] == &InputSection<ELFT>::Discarded)
       continue;
 
     // SHF_EXCLUDE'ed sections are discarded by the linker. However,
     // if -r is given, we'll let the final link discard such sections.
     // This is compatible with GNU.
     if ((Sec.sh_flags & SHF_EXCLUDE) && !Config->Relocatable) {
       Sections[I] = &InputSection<ELFT>::Discarded;
       continue;
     }
 
     switch (Sec.sh_type) {
     case SHT_GROUP:
       Sections[I] = &InputSection<ELFT>::Discarded;
       if (ComdatGroups.insert(getShtGroupSignature(Sec)).second)
         continue;
       for (uint32_t SecIndex : getShtGroupEntries(Sec)) {
-        if (SecIndex >= Size)
+        if (SecIndex >= R.size())
           fatal(getFilename(this) + ": invalid section index in group: " +
                 Twine(SecIndex));
         Sections[SecIndex] = &InputSection<ELFT>::Discarded;
       }
       break;
     case SHT_SYMTAB:
       this->Symtab = &Sec;
       break;