[PATCH] [Object] Split the ELF interface into 3 parts.

[Shankar Kalpathi Easwaran]

================
Comment at: include/llvm/Object/ELF.h:106-107
@@ -546,4 +105,4 @@
     difference_type operator -(const ELFEntityIterator &Other) const {
       assert(EntitySize == Other.EntitySize &&
              "Subtracting iterators of different EntitiySize!");
       return (Current - Other.Current) / EntitySize;
----------------
EntitiySize->EntitySize.

================
Comment at: include/llvm/Object/ELF.h:206-208
@@ +205,5 @@
+
+    difference_type operator-(const Elf_Sym_Iter &Other) const {
+      assert(EntitySize == Other.EntitySize &&
+             "Subtracting iterators of different EntitiySize!");
+      return (Current.getPointer() - Other.Current.getPointer()) / EntitySize;
----------------
Same here. EntitySize

================
Comment at: include/llvm/Object/ELF.h:225
@@ -582,1 +224,3 @@
 private:
+  typedef SmallVector<const Elf_Shdr *, 2> Sections_t;
+  typedef DenseMap<unsigned, unsigned> IndexMap_t;
----------------
Why only 2 entries ? Should this be a vector instead ?

================
Comment at: include/llvm/Object/ELF.h:510
@@ -981,3 +509,3 @@
   if (symb->st_shndx >= ELF::SHN_LORESERVE)
     return 0;
   return getSection(symb->st_shndx);
----------------
return NULL.

================
Comment at: include/llvm/Object/ELF.h:1227-1230
@@ +1226,6 @@
+  assert(Header && "Header not initialized!");
+  if (Header->e_shnum == ELF::SHN_UNDEF) {
+    assert(SectionHeaderTable && "SectionHeaderTable not initialized!");
+    return SectionHeaderTable->sh_size;
+  }
+  return Header->e_shnum;
----------------
Can this happen ? An ELF file with no sections ?

================
Comment at: include/llvm/Object/ELF.h:1374-1385
@@ +1373,14 @@
+
+  // TODO: Scan dynamic table for dynamic data in case section headers have been
+  //       stripped.
+#if 0
+  for (Elf_Dyn_Iter DynI = begin_dynamic_table(), DynE = end_dynamic_table();
+       DynI != DynE; ++DynI) {
+    switch (DynI->d_tag) {
+    case ELF::DT_SYMTAB:
+      ;
+      // Convert virtual addresses to file offsets...
+    }
+  }
+#endif
+
----------------
code to be removed ?

================
Comment at: include/llvm/Object/ELF.h:1441-1458
@@ -2616,16 +1440,20 @@
+ELFFile<ELFT>::end_dynamic_table(bool NULLEnd) const {
+  if (DynamicRegion.Addr) {
+    Elf_Dyn_Iter Ret(DynamicRegion.EntSize,
+                     (const char *)DynamicRegion.Addr + DynamicRegion.Size);
 
     if (NULLEnd) {
-      Elf_Dyn_iterator Start = begin_dynamic_table();
+      Elf_Dyn_Iter Start = begin_dynamic_table();
       while (Start != Ret && Start->getTag() != ELF::DT_NULL)
         ++Start;
 
       // Include the DT_NULL.
       if (Start != Ret)
         ++Start;
       Ret = Start;
     }
     return Ret;
   }
-  return Elf_Dyn_iterator(0, 0);
+  return Elf_Dyn_Iter(0, 0);
 }
 
----------------
This could be converted to 
 
if (!DynamicRegion.Addr)
  return Elf_Dyn_Iter(0,0);


================
Comment at: include/llvm/Object/ELF.h:1462-1474
@@ -2634,17 +1461,15 @@
+StringRef ELFFile<ELFT>::getLoadName() const {
   if (!dt_soname) {
     // Find the DT_SONAME entry
-    Elf_Dyn_iterator it = begin_dynamic_table();
-    Elf_Dyn_iterator ie = end_dynamic_table();
+    Elf_Dyn_Iter it = begin_dynamic_table();
+    Elf_Dyn_Iter ie = end_dynamic_table();
     while (it != ie && it->getTag() != ELF::DT_SONAME)
       ++it;
 
     if (it != ie) {
-      if (dot_dynstr_sec == NULL)
-        report_fatal_error("Dynamic string table is missing");
-      dt_soname = getString(dot_dynstr_sec, it->getVal());
     } else {
       dt_soname = "";
     }
   }
   return dt_soname;
 }
----------------
looks like the code for reading the dt_soname got missed.

================
Comment at: include/llvm/Object/ELF.h:1495
@@ -2855,3 +1494,3 @@
   if (index == 0)
     return 0;
   if (!SectionHeaderTable || index >= getNumSections())
----------------
return NULL.

================
Comment at: include/llvm/Object/ELFObjectFile.h:273-289
@@ +272,19 @@
+
+template <class ELFT>
+error_code ELFObjectFile<ELFT>::getSymbolAddress(DataRefImpl Symb,
+                                                 uint64_t &Result) const {
+  const Elf_Sym *ESym = getSymbol(Symb);
+  const Elf_Shdr *ESec;
+  switch (EF.getSymbolTableIndex(ESym)) {
+  case ELF::SHN_COMMON:
+  case ELF::SHN_UNDEF:
+    Result = UnknownAddressOrSize;
+    return object_error::success;
+  case ELF::SHN_ABS:
+    Result = ESym->st_value;
+    return object_error::success;
+  default:
+    ESec = EF.getSection(ESym);
+  }
+
+  switch (ESym->getType()) {
----------------
this doesnot handle target specific types though. For example target specific commons. They would be treated as a regular section index.

================
Comment at: include/llvm/Object/ELFObjectFile.h:1381-1384
@@ +1380,6 @@
+    report_fatal_error("Invalid section type in Rel!");
+  case ELF::SHT_REL: {
+    Result = 0;
+    return object_error::success;
+  }
+  case ELF::SHT_RELA: {
----------------
addend is part of the contents.

================
Comment at: include/llvm/Object/ELFObjectFile.h:1412
@@ +1411,3 @@
+    symbol_index = getRela(Rel)->getSymbol(EF.isMips64EL());
+    addend = getRela(Rel)->r_addend;
+    break;
----------------
Should this call getRelocationAddend ?


http://llvm-reviews.chandlerc.com/D1033