Dump PUBLIC + CFI records from libraries without debug info on Linux, use .dynsym for symbol names if there are no usable debug symbols.

R=jimb at http://breakpad.appspot.com/275001/show

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@793 4c0a9323-5329-0410-9bdc-e9ce6186880e

4 files changed, 710 insertions, 5 deletions

diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc
index 887880d2..77455a57 100644
--- a/src/common/linux/dump_symbols.cc
+++ b/src/common/linux/dump_symbols.cc
@@ -56,6 +56,7 @@
 #include "common/dwarf_cfi_to_module.h"
 #include "common/dwarf_cu_to_module.h"
 #include "common/dwarf_line_to_module.h"
+#include "common/linux/elf_symbols_to_module.h"
 #include "common/linux/file_id.h"
 #include "common/module.h"
 #include "common/stabs_reader.h"
@@ -531,6 +532,7 @@ static bool LoadSymbols(const std::string &obj_file,
       reinterpret_cast<ElfW(Shdr) *>(elf_header->e_shoff);
   const ElfW(Shdr) *section_names = sections + elf_header->e_shstrndx;
   bool found_debug_info_section = false;
+  bool found_usable_info = false;
 
   // Look for STABS debugging information, and load it if present.
   const ElfW(Shdr) *stab_section
@@ -540,6 +542,7 @@ static bool LoadSymbols(const std::string &obj_file,
     const ElfW(Shdr) *stabstr_section = stab_section->sh_link + sections;
     if (stabstr_section) {
       found_debug_info_section = true;
+      found_usable_info = true;
       info->LoadedSection(".stab");
       if (!LoadStabs(elf_header, stab_section, stabstr_section, big_endian,
                      module)) {
@@ -555,6 +558,7 @@ static bool LoadSymbols(const std::string &obj_file,
                           elf_header->e_shnum);
   if (dwarf_section) {
     found_debug_info_section = true;
+    found_usable_info = true;
     info->LoadedSection(".debug_info");
     if (!LoadDwarf(obj_file, elf_header, big_endian, module))
       fprintf(stderr, "%s: \".debug_info\" section found, but failed to load "
@@ -571,8 +575,10 @@ static bool LoadSymbols(const std::string &obj_file,
     // information, the other debugging information could be perfectly
     // useful.
     info->LoadedSection(".debug_frame");
-    LoadDwarfCFI(obj_file, elf_header, ".debug_frame",
-                 dwarf_cfi_section, false, 0, 0, big_endian, module);
+    bool result =
+      LoadDwarfCFI(obj_file, elf_header, ".debug_frame",
+                   dwarf_cfi_section, false, 0, 0, big_endian, module);
+    found_usable_info = found_usable_info || result;
   }
 
   // Linux C++ exception handling information can also provide
@@ -590,8 +596,10 @@ static bool LoadSymbols(const std::string &obj_file,
                         elf_header->e_shnum);
     info->LoadedSection(".eh_frame");
     // As above, ignore the return value of this function.
-    LoadDwarfCFI(obj_file, elf_header, ".eh_frame", eh_frame_section, true,
-                 got_section, text_section, big_endian, module);
+    bool result =
+      LoadDwarfCFI(obj_file, elf_header, ".eh_frame", eh_frame_section, true,
+                   got_section, text_section, big_endian, module);
+    found_usable_info = found_usable_info || result;
   }
 
   if (!found_debug_info_section) {
@@ -617,7 +625,44 @@ static bool LoadSymbols(const std::string &obj_file,
         fprintf(stderr, "%s does not contain a .gnu_debuglink section.\n",
                 obj_file.c_str());
       }
+    } else {
+      // The caller doesn't want to consult .gnu_debuglink.
+      // See if there are export symbols available.
+      const ElfW(Shdr) *dynsym_section =
+        FindSectionByName(".dynsym", sections, section_names,
+                          elf_header->e_shnum);
+      const ElfW(Shdr) *dynstr_section =
+        FindSectionByName(".dynstr", sections, section_names,
+                          elf_header->e_shnum);
+      if (dynsym_section && dynstr_section) {
+        info->LoadedSection(".dynsym");
+        fprintf(stderr, "Have .dynsym + .dynstr\n");
+
+        uint8_t* dynsyms =
+          reinterpret_cast<uint8_t*>(dynsym_section->sh_offset);
+        uint8_t* dynstrs =
+          reinterpret_cast<uint8_t*>(dynstr_section->sh_offset);
+        bool result =
+          ELFSymbolsToModule(dynsyms,
+                             dynsym_section->sh_size,
+                             dynstrs,
+                             dynstr_section->sh_size,
+                             big_endian,
+                             // This could change to something more useful
+                             // when support for dumping cross-architecture
+                             // symbols is finished.
+                             sizeof(ElfW(Addr)),
+                             module);
+        found_usable_info = found_usable_info || result;
+      }
+
+      // Return true if some usable information was found, since
+      // the caller doesn't want to use .gnu_debuglink.
+      return found_usable_info;
     }
+
+    // No debug info was found, let the user try again with .gnu_debuglink
+    // if present.
     return false;
   }
 
@@ -709,7 +754,8 @@ bool WriteSymbolFile(const std::string &obj_file,
 
   LoadSymbolsInfo info(debug_dir);
   Module module(name, os, architecture, id);
-  if (!LoadSymbols(obj_file, big_endian, elf_header, true, &info, &module)) {
+  if (!LoadSymbols(obj_file, big_endian, elf_header, !debug_dir.empty(),
+                   &info, &module)) {
     const std::string debuglink_file = info.debuglink_file();
     if (debuglink_file.empty())
       return false;
diff --git a/src/common/linux/elf_symbols_to_module.cc b/src/common/linux/elf_symbols_to_module.cc
new file mode 100644
index 00000000..82d53dd1
--- /dev/null
+++ b/src/common/linux/elf_symbols_to_module.cc
@@ -0,0 +1,168 @@
+// -*- mode: c++ -*-
+
+// Copyright (c) 2011 Google Inc. All Rights Reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Original author: Ted Mielczarek <ted.mielczarek@gmail.com>
+
+#include "common/linux/elf_symbols_to_module.h"
+
+#include <elf.h>
+#include <string.h>
+
+#include "common/byte_cursor.h"
+#include "common/module.h"
+
+namespace google_breakpad {
+
+class ELFSymbolIterator {
+public:
+  // The contents of an ELF symbol, adjusted for the host's endianness,
+  // word size, and so on. Corresponds to the data in Elf32_Sym / Elf64_Sym.
+  struct Symbol {
+    // True if this iterator has reached the end of the symbol array. When
+    // this is set, the other members of this structure are not valid.
+    bool at_end;
+
+    // The number of this symbol within the list.
+    size_t index;
+
+    // The current symbol's name offset. This is the offset within the
+    // string table.
+    size_t name_offset;
+
+    // The current symbol's value, size, info and shndx fields.
+    uint64_t value;
+    uint64_t size;
+    unsigned char info;
+    uint16_t shndx;
+  };
+
+  // Create an ELFSymbolIterator walking the symbols in BUFFER. Treat the
+  // symbols as big-endian if BIG_ENDIAN is true, as little-endian
+  // otherwise. Assume each symbol has a 'value' field whose size is
+  // VALUE_SIZE.
+  //
+  ELFSymbolIterator(const ByteBuffer *buffer, bool big_endian,
+                    size_t value_size)
+    : value_size_(value_size), cursor_(buffer, big_endian) {
+    // Actually, weird sizes could be handled just fine, but they're
+    // probably mistakes --- expressed in bits, say.
+    assert(value_size == 4 || value_size == 8);
+    symbol_.index = 0;
+    Fetch();
+  }
+
+  // Move to the next symbol. This function's behavior is undefined if
+  // at_end() is true when it is called.
+  ELFSymbolIterator &operator++() { Fetch(); symbol_.index++; return *this; }
+
+  // Dereferencing this iterator produces a reference to an Symbol structure
+  // that holds the current symbol's values. The symbol is owned by this
+  // SymbolIterator, and will be invalidated at the next call to operator++.
+  const Symbol &operator*() const { return symbol_; }
+  const Symbol *operator->() const { return &symbol_; }
+
+private:
+  // Read the symbol at cursor_, and set symbol_ appropriately.
+  void Fetch() {
+    // Elf32_Sym and Elf64_Sym have different layouts.
+    unsigned char other;
+    if (value_size_ == 4) {
+      // Elf32_Sym
+      cursor_
+        .Read(4, false, &symbol_.name_offset)
+        .Read(4, false, &symbol_.value)
+        .Read(4, false, &symbol_.size)
+        .Read(1, false, &symbol_.info)
+        .Read(1, false, &other)
+        .Read(2, false, &symbol_.shndx);
+    } else {
+      // Elf64_Sym
+      cursor_
+        .Read(4, false, &symbol_.name_offset)
+        .Read(1, false, &symbol_.info)
+        .Read(1, false, &other)
+        .Read(2, false, &symbol_.shndx)
+        .Read(8, false, &symbol_.value)
+        .Read(8, false, &symbol_.size);
+    }
+    symbol_.at_end = !cursor_;
+  }
+
+  // The size of symbols' value field, in bytes.
+  size_t value_size_;
+
+  // A byte cursor traversing buffer_.
+  ByteCursor cursor_;
+
+  // Values for the symbol this iterator refers to.
+  Symbol symbol_;
+};
+
+const char *SymbolString(ptrdiff_t offset, ByteBuffer& strings) {
+  if (offset < 0 || (size_t) offset >= strings.Size()) {
+    // Return the null string.
+    offset = 0;
+  }
+  return reinterpret_cast<const char *>(strings.start + offset);
+}
+
+bool ELFSymbolsToModule(const uint8_t *symtab_section,
+                        size_t symtab_size,
+                        const uint8_t *string_section,
+                        size_t string_size,
+                        const bool big_endian,
+                        size_t value_size,
+                        Module *module) {
+  ByteBuffer symbols(symtab_section, symtab_size);
+  // Ensure that the string section is null-terminated.
+  if (string_section[string_size - 1] != '\0') {
+    const void* null_terminator = memrchr(string_section, '\0', string_size);
+    string_size = reinterpret_cast<const uint8_t*>(null_terminator)
+      - string_section;
+  }
+  ByteBuffer strings(string_section, string_size);
+
+  // The iterator walking the symbol table.
+  ELFSymbolIterator iterator(&symbols, big_endian, value_size);
+
+  while(!iterator->at_end) {
+    if (ELF32_ST_TYPE(iterator->info) == STT_FUNC &&
+        iterator->shndx != SHN_UNDEF) {
+      Module::Extern *ext = new Module::Extern;
+      ext->name = SymbolString(iterator->name_offset, strings);
+      ext->address = iterator->value;
+      module->AddExtern(ext);
+    }
+    ++iterator;
+  }
+  return true;
+}
+
+}  // namespace google_breakpad
diff --git a/src/common/linux/elf_symbols_to_module.h b/src/common/linux/elf_symbols_to_module.h
new file mode 100644
index 00000000..2e7c0971
--- /dev/null
+++ b/src/common/linux/elf_symbols_to_module.h
@@ -0,0 +1,58 @@
+// -*- mode: c++ -*-
+
+// Copyright (c) 2011 Google Inc. All Rights Reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Original author: Ted Mielczarek <ted.mielczarek@gmail.com>
+
+// elf_symbols_to_module.h: Exposes ELFSymbolsToModule, a function
+// for reading ELF symbol tables and inserting exported symbol names
+// into a google_breakpad::Module as Extern definitions.
+
+#ifndef BREAKPAD_COMMON_LINUX_ELF_SYMBOLS_TO_MODULE_H_
+#define BREAKPAD_COMMON_LINUX_ELF_SYMBOLS_TO_MODULE_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+namespace google_breakpad {
+
+class Module;
+
+bool ELFSymbolsToModule(const uint8_t *symtab_section,
+                        size_t symtab_size,
+                        const uint8_t *string_section,
+                        size_t string_size,
+                        const bool big_endian,
+                        size_t value_size,
+                        Module *module);
+
+}  // namespace google_breakpad
+
+
+#endif  // BREAKPAD_COMMON_LINUX_ELF_SYMBOLS_TO_MODULE_H_
diff --git a/src/common/linux/elf_symbols_to_module_unittest.cc b/src/common/linux/elf_symbols_to_module_unittest.cc
new file mode 100644
index 00000000..f7ab49c0
--- /dev/null
+++ b/src/common/linux/elf_symbols_to_module_unittest.cc
@@ -0,0 +1,433 @@
+// Copyright (c) 2011 Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Original author: Ted Mielczarek <ted.mielczarek@gmail.com>
+
+// elf_symbols_to_module_unittest.cc:
+// Unittests for google_breakpad::ELFSymbolsToModule
+
+#include <elf.h>
+
+#include <string>
+#include <vector>
+
+#include "breakpad_googletest_includes.h"
+#include "common/linux/elf_symbols_to_module.h"
+#include "common/module.h"
+#include "common/test_assembler.h"
+
+using google_breakpad::Module;
+using google_breakpad::test_assembler::Endianness;
+using google_breakpad::test_assembler::kBigEndian;
+using google_breakpad::test_assembler::kLittleEndian;
+using google_breakpad::test_assembler::kUnsetEndian;
+using google_breakpad::test_assembler::Label;
+using google_breakpad::test_assembler::Section;
+using ::testing::Test;
+using ::testing::TestWithParam;
+using std::string;
+using std::vector;
+
+// String Tables are used in ELF headers, add a class
+// for convenience.
+class StringTable : public Section {
+public:
+  StringTable(Endianness endianness = kUnsetEndian)
+  : Section(endianness) {
+    start() = 0;
+    empty_string = Add("");
+  }
+
+  // Add the string s to the string table, and return
+  // a label containing the offset into the string table
+  // at which it was added.
+  Label Add(const string& s) {
+    Label string_label(Here());
+    AppendCString(s);
+    return string_label;
+  }
+
+  // All StringTables contain an empty string as their first
+  // entry.
+  Label empty_string;
+};
+
+class StringTableTest : public Test {
+public:
+  StringTableTest() : table(kLittleEndian) {}
+
+  StringTable table;
+};
+
+TEST_F(StringTableTest, Empty) {
+  string contents;
+  ASSERT_TRUE(table.GetContents(&contents));
+  const string kExpectedContents = "\0";
+  EXPECT_EQ(0,
+            memcmp(kExpectedContents.c_str(), contents.c_str(), table.Size()));
+  ASSERT_TRUE(table.empty_string.IsKnownConstant());
+  EXPECT_EQ(0, table.empty_string.Value());
+}
+
+TEST_F(StringTableTest, Basic) {
+  const string s1("table fills with strings");
+  const string s2("offsets preserved as labels");
+  const string s3("verified with tests");
+  const string kExpectedContents = 
+    "\0table fills with strings\0"
+    "offsets preserved as labels\0"
+    "verified with tests\0";
+  Label l1(table.Add(s1));
+  Label l2(table.Add(s2));
+  Label l3(table.Add(s3));
+  string contents;
+  ASSERT_TRUE(table.GetContents(&contents));
+  EXPECT_EQ(0,
+            memcmp(kExpectedContents.c_str(), contents.c_str(), table.Size()));
+  // empty_string is at zero, other strings start at 1.
+  ASSERT_TRUE(l1.IsKnownConstant());
+  EXPECT_EQ(1, l1.Value());
+  // Each string has an extra byte for a trailing null.
+  EXPECT_EQ(1 + s1.length() + 1, l2.Value());
+  EXPECT_EQ(1 + s1.length() + 1 + s2.length() + 1, l3.Value());
+}
+
+class ELFSymbolsToModuleTestFixture {
+public:
+  ELFSymbolsToModuleTestFixture(Endianness endianness,
+                                size_t value_size) : module("a", "b", "c", "d"),
+                                                     section(endianness),
+                                                     table(endianness),
+                                                     value_size(value_size) {}
+
+  bool ProcessSection() {
+    string section_contents, table_contents;
+    section.GetContents(&section_contents);
+    table.GetContents(&table_contents);
+
+    bool ret = ELFSymbolsToModule(reinterpret_cast<const uint8_t*>(section_contents.data()),
+                                  section_contents.size(),
+                                  reinterpret_cast<const uint8_t*>(table_contents.data()),
+                                  table_contents.size(),
+                                  section.endianness() == kBigEndian,
+                                  value_size,
+                                  &module);
+    module.GetExterns(&externs, externs.end());
+    return ret;
+  }
+
+  Module module;
+  Section section;
+  StringTable table;
+  string section_contents;
+  // 4 or 8 (bytes)
+  size_t value_size;
+
+  vector<Module::Extern *> externs;
+};
+
+class ELFSymbolsToModuleTest32 : public ELFSymbolsToModuleTestFixture,
+                                   public TestWithParam<Endianness>  {
+public:
+  ELFSymbolsToModuleTest32() : ELFSymbolsToModuleTestFixture(GetParam(), 4) {}
+
+  void AddElf32Sym(const string& name, uint32_t value,
+                   uint32_t size, unsigned info, uint16_t shndx) {
+    section
+      .D32(table.Add(name))
+      .D32(value)
+      .D32(size)
+      .D8(info)
+      .D8(0) // other
+      .D16(shndx);
+  }
+};
+
+TEST_P(ELFSymbolsToModuleTest32, NoFuncs) {
+  ProcessSection();
+
+  ASSERT_EQ((size_t)0, externs.size());
+}
+
+TEST_P(ELFSymbolsToModuleTest32, OneFunc) {
+  const string kFuncName = "superfunc";
+  const uint32_t kFuncAddr = 0x1000;
+  const uint32_t kFuncSize = 0x10;
+
+  AddElf32Sym(kFuncName, kFuncAddr, kFuncSize,
+              ELF32_ST_INFO(STB_GLOBAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 1);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)1, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr, extern1->address);
+}
+
+TEST_P(ELFSymbolsToModuleTest32, NameOutOfBounds) {
+  const string kFuncName = "";
+  const uint32_t kFuncAddr = 0x1000;
+  const uint32_t kFuncSize = 0x10;
+
+  table.Add("Foo");
+  table.Add("Bar");
+  // Can't use AddElf32Sym because it puts in a valid string offset.
+  section
+    .D32((uint32_t)table.Here().Value() + 1)
+    .D32(kFuncAddr)
+    .D32(kFuncSize)
+    .D8(ELF32_ST_INFO(STB_GLOBAL, STT_FUNC))
+    .D8(0) // other
+    .D16(SHN_UNDEF + 1);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)1, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr, extern1->address);
+}
+
+TEST_P(ELFSymbolsToModuleTest32, NonTerminatedStringTable) {
+  const string kFuncName = "";
+  const uint32_t kFuncAddr = 0x1000;
+  const uint32_t kFuncSize = 0x10;
+
+  table.Add("Foo");
+  table.Add("Bar");
+  // Add a non-null-terminated string to the end of the string table
+  Label l;
+  table
+    .Mark(&l)
+    .Append("Unterminated");
+  // Can't use AddElf32Sym because it puts in a valid string offset.
+  section
+    .D32((uint32_t)l.Value())
+    .D32(kFuncAddr)
+    .D32(kFuncSize)
+    .D8(ELF32_ST_INFO(STB_GLOBAL, STT_FUNC))
+    .D8(0) // other
+    .D16(SHN_UNDEF + 1);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)1, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr, extern1->address);
+}
+
+TEST_P(ELFSymbolsToModuleTest32, MultipleFuncs) {
+  const string kFuncName1 = "superfunc";
+  const uint32_t kFuncAddr1 = 0x10001000;
+  const uint32_t kFuncSize1 = 0x10;
+  const string kFuncName2 = "awesomefunc";
+  const uint32_t kFuncAddr2 = 0x20002000;
+  const uint32_t kFuncSize2 = 0x2f;
+  const string kFuncName3 = "megafunc";
+  const uint32_t kFuncAddr3 = 0x30003000;
+  const uint32_t kFuncSize3 = 0x3c;
+
+  AddElf32Sym(kFuncName1, kFuncAddr1, kFuncSize1,
+              ELF32_ST_INFO(STB_GLOBAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 1);
+  AddElf32Sym(kFuncName2, kFuncAddr2, kFuncSize2,
+              ELF32_ST_INFO(STB_LOCAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 2);
+  AddElf32Sym(kFuncName3, kFuncAddr3, kFuncSize3,
+              ELF32_ST_INFO(STB_LOCAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 3);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)3, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName1, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr1, extern1->address);
+  Module::Extern *extern2 = externs[1];
+  EXPECT_EQ(kFuncName2, extern2->name);
+  EXPECT_EQ((Module::Address)kFuncAddr2, extern2->address);
+  Module::Extern *extern3 = externs[2];
+  EXPECT_EQ(kFuncName3, extern3->name);
+  EXPECT_EQ((Module::Address)kFuncAddr3, extern3->address);
+}
+
+TEST_P(ELFSymbolsToModuleTest32, SkipStuff) {
+  const string kFuncName = "superfunc";
+  const uint32_t kFuncAddr = 0x1000;
+  const uint32_t kFuncSize = 0x10;
+
+  // Should skip functions in SHN_UNDEF
+  AddElf32Sym("skipme", 0xFFFF, 0x10,
+              ELF32_ST_INFO(STB_GLOBAL, STT_FUNC),
+              SHN_UNDEF);
+  AddElf32Sym(kFuncName, kFuncAddr, kFuncSize,
+              ELF32_ST_INFO(STB_GLOBAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 1);
+  // Should skip non-STT_FUNC entries.
+  AddElf32Sym("skipmetoo", 0xAAAA, 0x10,
+              ELF32_ST_INFO(STB_GLOBAL, STT_FILE),
+              SHN_UNDEF + 1);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)1, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr, extern1->address);
+}
+
+// Run all the 32-bit tests with both endianness
+INSTANTIATE_TEST_CASE_P(Endian,
+                        ELFSymbolsToModuleTest32,
+                        ::testing::Values(kLittleEndian, kBigEndian));
+
+// Similar tests, but with 64-bit values. Ostensibly this could be
+// shoehorned into the parameterization by using ::testing::Combine,
+// but that would make it difficult to get the types right since these
+// actual test cases aren't parameterized. This could also be written
+// as a type-parameterized test, but combining that with a value-parameterized
+// test seemed really ugly, and also makes it harder to test 64-bit
+// values.
+class ELFSymbolsToModuleTest64 : public ELFSymbolsToModuleTestFixture,
+                                 public TestWithParam<Endianness>  {
+public:
+  ELFSymbolsToModuleTest64() : ELFSymbolsToModuleTestFixture(GetParam(), 8) {}
+
+  void AddElf64Sym(const string& name, uint64_t value,
+                   uint64_t size, unsigned info, uint16_t shndx) {
+    section
+      .D32(table.Add(name))
+      .D8(info)
+      .D8(0) // other
+      .D16(shndx)
+      .D64(value)
+      .D64(size);
+  }
+};
+
+TEST_P(ELFSymbolsToModuleTest64, NoFuncs) {
+  ProcessSection();
+
+  ASSERT_EQ((size_t)0, externs.size());
+}
+
+TEST_P(ELFSymbolsToModuleTest64, OneFunc) {
+  const string kFuncName = "superfunc";
+  const uint64_t kFuncAddr = 0x1000200030004000ULL;
+  const uint64_t kFuncSize = 0x1000;
+
+  AddElf64Sym(kFuncName, kFuncAddr, kFuncSize,
+              ELF64_ST_INFO(STB_GLOBAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 1);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)1, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr, extern1->address);
+}
+
+TEST_P(ELFSymbolsToModuleTest64, MultipleFuncs) {
+  const string kFuncName1 = "superfunc";
+  const uint64_t kFuncAddr1 = 0x1000100010001000ULL;
+  const uint64_t kFuncSize1 = 0x1000;
+  const string kFuncName2 = "awesomefunc";
+  const uint64_t kFuncAddr2 = 0x2000200020002000ULL;
+  const uint64_t kFuncSize2 = 0x2f00;
+  const string kFuncName3 = "megafunc";
+  const uint64_t kFuncAddr3 = 0x3000300030003000ULL;
+  const uint64_t kFuncSize3 = 0x3c00;
+
+  AddElf64Sym(kFuncName1, kFuncAddr1, kFuncSize1,
+              ELF64_ST_INFO(STB_GLOBAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 1);
+  AddElf64Sym(kFuncName2, kFuncAddr2, kFuncSize2,
+              ELF64_ST_INFO(STB_LOCAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 2);
+  AddElf64Sym(kFuncName3, kFuncAddr3, kFuncSize3,
+              ELF64_ST_INFO(STB_LOCAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 3);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)3, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName1, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr1, extern1->address);
+  Module::Extern *extern2 = externs[1];
+  EXPECT_EQ(kFuncName2, extern2->name);
+  EXPECT_EQ((Module::Address)kFuncAddr2, extern2->address);
+  Module::Extern *extern3 = externs[2];
+  EXPECT_EQ(kFuncName3, extern3->name);
+  EXPECT_EQ((Module::Address)kFuncAddr3, extern3->address);
+}
+
+TEST_P(ELFSymbolsToModuleTest64, SkipStuff) {
+  const string kFuncName = "superfunc";
+  const uint64_t kFuncAddr = 0x1000100010001000ULL;
+  const uint64_t kFuncSize = 0x1000;
+
+  // Should skip functions in SHN_UNDEF
+  AddElf64Sym("skipme", 0xFFFF, 0x10,
+              ELF64_ST_INFO(STB_GLOBAL, STT_FUNC),
+              SHN_UNDEF);
+  AddElf64Sym(kFuncName, kFuncAddr, kFuncSize,
+              ELF64_ST_INFO(STB_GLOBAL, STT_FUNC),
+              // Doesn't really matter, just can't be SHN_UNDEF.
+              SHN_UNDEF + 1);
+  // Should skip non-STT_FUNC entries.
+  AddElf64Sym("skipmetoo", 0xAAAA, 0x10,
+              ELF64_ST_INFO(STB_GLOBAL, STT_FILE),
+              SHN_UNDEF + 1);
+
+  ProcessSection();
+
+  ASSERT_EQ((size_t)1, externs.size());
+  Module::Extern *extern1 = externs[0];
+  EXPECT_EQ(kFuncName, extern1->name);
+  EXPECT_EQ((Module::Address)kFuncAddr, extern1->address);
+}
+
+// Run all the 64-bit tests with both endianness
+INSTANTIATE_TEST_CASE_P(Endian,
+                        ELFSymbolsToModuleTest64,
+                        ::testing::Values(kLittleEndian, kBigEndian));