Fix the code that leads to the incorrect trigger of the report_fatal_error()

in MachOObjectFile::getSymbolByIndex() when a Mach-O file has
a symbol table load command but the number of symbols are zero.

The code in MachOObjectFile::symbol_begin_impl() should not be
assuming there is a symbol at index 0, in cases there is no symbol
table load command or the count of symbol is zero.  So I also fixed
that.  And needed to fix MachOObjectFile::symbol_end_impl() to
also do the same thing for no symbol table or one with zero entries.

The code in MachOObjectFile::getSymbolByIndex() should trigger
the report_fatal_error() for programmatic errors for any index when
there is no symbol table load command and not return the end iterator.
So also fixed that. Note there is no test case as this is a programmatic
error.

The test case using the file macho-invalid-bad-symbol-index has
a symbol table load command with its number of symbols (nsyms)
is zero. Which was incorrectly testing the bad triggering of the
report_fatal_error() in in MachOObjectFile::getSymbolByIndex().

This test case is an invalid Mach-O file but not for that reason.
It appears this Mach-O file use to have an nsyms value of 11,
and what makes this Mach-O file invalid is the counts and
indexes into the symbol table of the dynamic load command
are now invalid because the number of symbol table entries
(nsyms) is now zero.  Which can be seen with the existing
llvm-obdump:

% llvm-objdump -private-headers macho-invalid-bad-symbol-index
…
Load command 4
     cmd LC_SYMTAB
 cmdsize 24
  symoff 4216
   nsyms 0
  stroff 4392
 strsize 144
Load command 5
            cmd LC_DYSYMTAB
        cmdsize 80
      ilocalsym 0
      nlocalsym 8 (past the end of the symbol table)
     iextdefsym 8 (greater than the number of symbols)
     nextdefsym 2 (past the end of the symbol table)
      iundefsym 10 (greater than the number of symbols)
      nundefsym 1 (past the end of the symbol table)
...

And the native darwin tools generates an error for this file:

% nm macho-invalid-bad-symbol-index
nm: object: macho-invalid-bad-symbol-index truncated or malformed object (ilocalsym plus nlocalsym in LC_DYSYMTAB load command extends past the end of the symbol table)

I added new checks for the indexes and sizes for these in the
constructor of MachOObjectFile.  And added comments for what
would be a proper diagnostic messages.

And changed the test case using macho-invalid-bad-symbol-index
to test for the new error now produced.

Also added a test with a valid Mach-O file with a symbol table
load command where the number of symbols is zero that shows
the report_fatal_error() is not called.

llvm-svn: 258576

lib/Object/MachOObjectFile.cpp

@@ -317,6 +317,61 @@ MachOObjectFile::MachOObjectFile(MemoryBufferRef Object, bool IsLittleEndian,
       Load = LoadOrErr.get();
     }
   }
+  if (!SymtabLoadCmd) {
+    if (DysymtabLoadCmd) {
+      // Diagnostic("truncated or malformed object (contains LC_DYSYMTAB load "
+      // "command without a LC_SYMTAB load command)");
+      EC = object_error::parse_failed;
+      return;
+    }
+  } else if (DysymtabLoadCmd) {
+    MachO::symtab_command Symtab =
+      getStruct<MachO::symtab_command>(this, SymtabLoadCmd);
+    MachO::dysymtab_command Dysymtab =
+      getStruct<MachO::dysymtab_command>(this, DysymtabLoadCmd);
+    if (Dysymtab.nlocalsym != 0 && Dysymtab.ilocalsym > Symtab.nsyms) {
+      // Diagnostic("truncated or malformed object (ilocalsym in LC_DYSYMTAB "
+      // "load command extends past the end of the symbol table)"
+      EC = object_error::parse_failed;
+      return;
+    }
+    uint64_t big_size = Dysymtab.ilocalsym;
+    big_size += Dysymtab.nlocalsym;
+    if (Dysymtab.nlocalsym != 0 && big_size > Symtab.nsyms) {
+      // Diagnostic("truncated or malformed object (ilocalsym plus nlocalsym "
+      // "in LC_DYSYMTAB load command extends past the end of the symbol table)"
+      EC = object_error::parse_failed;
+      return;
+    }
+    if (Dysymtab.nextdefsym != 0 && Dysymtab.ilocalsym > Symtab.nsyms) {
+      // Diagnostic("truncated or malformed object (nextdefsym in LC_DYSYMTAB "
+      // "load command extends past the end of the symbol table)"
+      EC = object_error::parse_failed;
+      return;
+    }
+    big_size = Dysymtab.iextdefsym;
+    big_size += Dysymtab.nextdefsym;
+    if (Dysymtab.nextdefsym != 0 && big_size > Symtab.nsyms) {
+      // Diagnostic("truncated or malformed object (iextdefsym plus nextdefsym "
+      // "in LC_DYSYMTAB load command extends past the end of the symbol table)"
+      EC = object_error::parse_failed;
+      return;
+    }
+    if (Dysymtab.nundefsym != 0 && Dysymtab.iundefsym > Symtab.nsyms) {
+      // Diagnostic("truncated or malformed object (nundefsym in LC_DYSYMTAB "
+      // "load command extends past the end of the symbol table)"
+      EC = object_error::parse_failed;
+      return;
+    }
+    big_size = Dysymtab.iundefsym;
+    big_size += Dysymtab.nundefsym;
+    if (Dysymtab.nundefsym != 0 && big_size > Symtab.nsyms) {
+      // Diagnostic("truncated or malformed object (iundefsym plus nundefsym "
+      // "in LC_DYSYMTAB load command extends past the end of the symbol table)"
+      EC = object_error::parse_failed;
+      return;
+    }
+  }
   assert(LoadCommands.size() == LoadCommandCount);
 }
 
@@ -941,15 +996,20 @@ MachOObjectFile::getRelocationRelocatedSection(relocation_iterator Rel) const {
 }
 
 basic_symbol_iterator MachOObjectFile::symbol_begin_impl() const {
+  DataRefImpl DRI;
+  MachO::symtab_command Symtab = getSymtabLoadCommand();
+  if (!SymtabLoadCmd || Symtab.nsyms == 0)
+    return basic_symbol_iterator(SymbolRef(DRI, this));
+
   return getSymbolByIndex(0);
 }
 
 basic_symbol_iterator MachOObjectFile::symbol_end_impl() const {
   DataRefImpl DRI;
-  if (!SymtabLoadCmd)
+  MachO::symtab_command Symtab = getSymtabLoadCommand();
+  if (!SymtabLoadCmd || Symtab.nsyms == 0)
     return basic_symbol_iterator(SymbolRef(DRI, this));
 
-  MachO::symtab_command Symtab = getSymtabLoadCommand();
   unsigned SymbolTableEntrySize = is64Bit() ?
     sizeof(MachO::nlist_64) :
     sizeof(MachO::nlist);
@@ -960,15 +1020,12 @@ basic_symbol_iterator MachOObjectFile::symbol_end_impl() const {
 }
 
 basic_symbol_iterator MachOObjectFile::getSymbolByIndex(unsigned Index) const {
-  DataRefImpl DRI;
-  if (!SymtabLoadCmd)
-    return basic_symbol_iterator(SymbolRef(DRI, this));
-
   MachO::symtab_command Symtab = getSymtabLoadCommand();
-  if (Index >= Symtab.nsyms)
+  if (!SymtabLoadCmd || Index >= Symtab.nsyms)
     report_fatal_error("Requested symbol index is out of range.");
   unsigned SymbolTableEntrySize =
     is64Bit() ? sizeof(MachO::nlist_64) : sizeof(MachO::nlist);
+  DataRefImpl DRI;
   DRI.p = reinterpret_cast<uintptr_t>(getPtr(this, Symtab.symoff));
   DRI.p += Index * SymbolTableEntrySize;
   return basic_symbol_iterator(SymbolRef(DRI, this));

test/Object/macho-invalid.test

@@ -25,7 +25,11 @@ TOO-MANY-SECTS: Mach-O segment load command contains too many sections
 
 RUN: not llvm-objdump -t %p/Inputs/macho-invalid-bad-symbol-index 2>&1 \
 RUN:      | FileCheck -check-prefix BAD-SYMBOL %s
-BAD-SYMBOL: Requested symbol index is out of range
+BAD-SYMBOL: Invalid data was encountered while parsing the file.
+RUN: llvm-objdump -t %p/Inputs/macho-valid-0-nsyms 2>&1 \
+RUN:      | FileCheck -check-prefix ZERO-NSYMS %s
+ZERO-NSYMS: SYMBOL TABLE
+ZERO-NSYMS-NOT: Requested symbol index is out of range
 
 RUN: not llvm-objdump -t %p/Inputs/macho-invalid-symbol-name-past-eof 2>&1 \
 RUN:      | FileCheck -check-prefix NAME-PAST-EOF %s