[PATCH] D25016: [ELF] - Fixed assert fail when symbol table has invalid sh_info value.

[George Rimar via llvm-commits]

>rafael added inline comments.
>
>================
>Comment at: ELF/InputFiles.cpp:84
>@@ -83,3 +83,3 @@
>   uint32_t FirstNonLocal = Symtab->sh_info;
>-  if (FirstNonLocal > NumSymbols)
>+  if (!FirstNonLocal || (FirstNonLocal > NumSymbols))
>     fatal(getFilename(this) + ": invalid sh_info in symbol table");
>----------------
>davide wrote:
>> ruiu wrote:
>> > Probably `FirstNonLocal == 0` is better as we all know that 0 is not a valid symbol index.
>> Agree.
>Do you need this check in here to avoid the crash? How was it crashing before getting to the other check you added in Writer?
>

Current invalid-symtab-sh_info3.elf input seems does not show the crash that is fixed by this change (it just reveals zero sh_info),
I'll update it as soon as phabricator gets alive to demonstrate issue.

Testcase I am talking about crashes on next lines:
    for (SymbolBody *B : F->getLocalSymbols()) {
      if (!B->IsLocal) // HERE

That happens because of getLocalSymbols() implementation:

ArrayRef<SymbolBody *> elf::ObjectFile<ELFT>::getLocalSymbols() {
.....
  uint32_t FirstNonLocal = this->Symtab->sh_info;
  return makeArrayRef(this->SymbolBodies).slice(1, FirstNonLocal - 1);
}

When FirstNonLocal is 0, (FirstNonLocal - 1) == 0xFFFFFFFF,
slice() itself does not have an valid assert to check that, it do:
assert(N+M <= size() && "Invalid specifier");

But 1 + 0xFFFFFFFF <= 2 in my case, so returned array has invalid size and
when accessing B->IsLocal second time it access invalid memory and crashes.

George.