Read either DT_HASH or DT_GNU_HASH to determine the size of DT_SYMTAB

Without the section headers for the symbol table, there's no direct way
to calculate the number of entries in the table.

DT_HASH and DT_GNU_HASH are auxiliary tables that are designed to make
symbol lookup faster. DT_HASH is the original and is theoretically
mandatory. DT_GNU_HASH is the new-and-improved, but is more complex.

In practice, however, an Android build (at least vs. API 16) has only
DT_HASH, and not DT_GNU_HASH, and a Fuchsia build has only DT_GNU_HASH
but not DT_HASH. So, both are tried.

This change does not actually use the data in these tables to improve
the speed of symbol lookup, but instead only uses them to correctly
terminate the linear search.

DT_HASH contains the total number of symbols in the symbol table fairly
directly because there is an entry for each symbol table entry in the
hash table, so the number is the same.

DT_GNU_HASH regrettably does not. Instead, it's necessary to walk the
buckets and chain structure to find the largest entry.

DT_GNU_HASH doesn't appear in any "real" documentation that I'm aware
of, other than the binutils code (at least as far as I know). Some
more-and-less-useful references:
- https://flapenguin.me/2017/04/24/elf-lookup-dt-hash/
- https://flapenguin.me/2017/05/10/elf-lookup-dt-gnu-hash/
- http://deroko.phearless.org/dt_gnu_hash.txt
- https://sourceware.org/ml/binutils/2006-10/msg00377.html

Change-Id: I7cfc4372f29efc37446f0931d22a1f790e44076f
Bug: crashpad:213, crashpad:196
Reviewed-on: https://chromium-review.googlesource.com/876879
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

build/run_tests.py

@@ -379,15 +379,22 @@ def _RunOnFuchsiaTarget(binary_dir, test, device_name, extra_command_line):
 
     def netcp(local_path):
       """Uses `netcp` to copy a file or directory to the device. Files located
-      inside the build dir are stored to /pkg/bin, or /pkg/lib (if .so),
-      otherwise to /pkg/assets.
+      inside the build dir are stored to /pkg/bin, otherwise to /pkg/assets.
+      .so files are stored somewhere completely different, into /boot/lib (!).
+      This is because the loader service does not yet correctly handle the
+      namespace in which the caller is being run, and so can only load .so files
+      from a couple hardcoded locations, the only writable one of which is
+      /boot/lib, so we copy all .so files there. This bug is filed upstream as
+      ZX-1619.
       """
       in_binary_dir = local_path.startswith(binary_dir + '/')
       if in_binary_dir:
-        is_so = local_path.endswith('.so')
-        target_path = os.path.join(staging_root,
-                                   'lib' if is_so else 'bin',
-                                   local_path[len(binary_dir)+1:])
+        if local_path.endswith('.so'):
+          target_path = os.path.join(
+              '/boot/lib', local_path[len(binary_dir)+1:])
+        else:
+          target_path = os.path.join(
+              staging_root, 'bin', local_path[len(binary_dir)+1:])
       else:
         target_path = os.path.join(staging_root, 'assets', local_path)
       netcp_path = os.path.join(sdk_root, 'tools', 'netcp')

snapshot/BUILD.gn

@@ -352,6 +352,7 @@ source_set("snapshot_test") {
     ":crashpad_snapshot_test_module",
     ":crashpad_snapshot_test_module_large",
     ":crashpad_snapshot_test_module_small",
+    ":crashpad_snapshot_test_both_dt_hash_styles",
   ]
 
   if (crashpad_is_mac) {
@@ -414,6 +415,18 @@ loadable_module("crashpad_snapshot_test_module_small") {
   ]
 }
 
+if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) {
+  loadable_module("crashpad_snapshot_test_both_dt_hash_styles") {
+    testonly = true
+    sources = [
+      "hash_types_test.cc",
+    ]
+
+    # This makes `ld` emit both .hash and .gnu.hash sections.
+    ldflags = [ "-Wl,--hash-style=both" ]
+  }
+}
+
 if (crashpad_is_mac) {
   loadable_module("crashpad_snapshot_test_module_crashy_initializer") {
     testonly = true

snapshot/elf/elf_dynamic_array_reader.h

@@ -50,13 +50,15 @@ class ElfDynamicArrayReader {
   //!
   //! \param[in] tag Specifies which value should be retrieved. The possible
   //!     values for this parameter are the `DT_*` values from `<elf.h>`.
+  //! \param[in] log Specifies whether an error should be logged if \a tag is
+  //!     not found.
   //! \param[out] value The value, casted to an appropriate type, if found.
   //! \return `true` if the value is found.
   template <typename V>
-  bool GetValue(uint64_t tag, V* value) {
+  bool GetValue(uint64_t tag, bool log, V* value) {
     auto iter = values_.find(tag);
     if (iter == values_.end()) {
-      LOG(ERROR) << "tag not found";
+      LOG_IF(ERROR, log) << "tag not found";
       return false;
     }
     return ReinterpretBytes(iter->second, value);

snapshot/elf/elf_image_reader.cc

@@ -519,8 +519,8 @@ bool ElfImageReader::ReadDynamicStringTableAtOffset(VMSize offset,
 
   VMAddress string_table_address;
   VMSize string_table_size;
-  if (!GetAddressFromDynamicArray(DT_STRTAB, &string_table_address) ||
-      !dynamic_array_->GetValue(DT_STRSZ, &string_table_size)) {
+  if (!GetAddressFromDynamicArray(DT_STRTAB, true, &string_table_address) ||
+      !dynamic_array_->GetValue(DT_STRSZ, true, &string_table_size)) {
     LOG(ERROR) << "missing string table info";
     return false;
   }
@@ -542,7 +542,7 @@ bool ElfImageReader::GetDebugAddress(VMAddress* debug) {
   if (!InitializeDynamicArray()) {
     return false;
   }
-  return GetAddressFromDynamicArray(DT_DEBUG, debug);
+  return GetAddressFromDynamicArray(DT_DEBUG, true, debug);
 }
 
 bool ElfImageReader::InitializeProgramHeaders() {
@@ -609,20 +609,35 @@ bool ElfImageReader::InitializeDynamicSymbolTable() {
   }
 
   VMAddress symbol_table_address;
-  if (!GetAddressFromDynamicArray(DT_SYMTAB, &symbol_table_address)) {
+  if (!GetAddressFromDynamicArray(DT_SYMTAB, true, &symbol_table_address)) {
     LOG(ERROR) << "no symbol table";
     return false;
   }
 
-  symbol_table_.reset(
-      new ElfSymbolTableReader(&memory_, this, symbol_table_address));
+  // Try both DT_HASH and DT_GNU_HASH. They're completely different, but both
+  // circuitously offer a way to find the number of entries in the symbol table.
+  // DT_HASH is specifically checked first, because depending on the linker, the
+  // count maybe be incorrect for zero-export cases. In practice, it is believed
+  // that the zero-export case is probably not particularly useful, so this
+  // incorrect count will only occur in constructed test cases (see
+  // ElfImageReader.DtHashAndDtGnuHashMatch).
+  VMSize number_of_symbol_table_entries;
+  if (!GetNumberOfSymbolEntriesFromDtHash(&number_of_symbol_table_entries) &&
+      !GetNumberOfSymbolEntriesFromDtGnuHash(&number_of_symbol_table_entries)) {
+    LOG(ERROR) << "could not retrieve number of symbol table entries";
+    return false;
+  }
+
+  symbol_table_.reset(new ElfSymbolTableReader(
+      &memory_, this, symbol_table_address, number_of_symbol_table_entries));
   symbol_table_initialized_.set_valid();
   return true;
 }
 
 bool ElfImageReader::GetAddressFromDynamicArray(uint64_t tag,
+                                                bool log,
                                                 VMAddress* address) {
-  if (!dynamic_array_->GetValue(tag, address)) {
+  if (!dynamic_array_->GetValue(tag, log, address)) {
     return false;
   }
 #if defined(OS_ANDROID) || defined(OS_FUCHSIA)
@@ -635,6 +650,100 @@ bool ElfImageReader::GetAddressFromDynamicArray(uint64_t tag,
   return true;
 }
 
+bool ElfImageReader::GetNumberOfSymbolEntriesFromDtHash(
+    VMSize* number_of_symbol_table_entries) {
+  if (!InitializeDynamicArray()) {
+    return false;
+  }
+
+  VMAddress dt_hash_address;
+  if (!GetAddressFromDynamicArray(DT_HASH, false, &dt_hash_address)) {
+    return false;
+  }
+
+  struct {
+    uint32_t nbucket;
+    uint32_t nchain;
+  } header;
+
+  if (!memory_.Read(dt_hash_address, sizeof(header), &header)) {
+    LOG(ERROR) << "failed to read DT_HASH header";
+    return false;
+  }
+
+  *number_of_symbol_table_entries = header.nchain;
+  return true;
+}
+
+bool ElfImageReader::GetNumberOfSymbolEntriesFromDtGnuHash(
+    VMSize* number_of_symbol_table_entries) {
+  if (!InitializeDynamicArray()) {
+    return false;
+  }
+
+  VMAddress dt_gnu_hash_address;
+  if (!GetAddressFromDynamicArray(DT_GNU_HASH, false, &dt_gnu_hash_address)) {
+    return false;
+  }
+
+  // See https://flapenguin.me/2017/05/10/elf-lookup-dt-gnu-hash/ and
+  // https://sourceware.org/ml/binutils/2006-10/msg00377.html.
+  struct {
+    uint32_t nbuckets;
+    uint32_t symoffset;
+    uint32_t bloom_size;
+    uint32_t bloom_shift;
+  } header;
+  if (!memory_.Read(dt_gnu_hash_address, sizeof(header), &header)) {
+    LOG(ERROR) << "failed to read DT_GNU_HASH header";
+    return false;
+  }
+
+  std::vector<uint32_t> buckets(header.nbuckets);
+  const size_t kNumBytesForBuckets = sizeof(buckets[0]) * buckets.size();
+  const size_t kWordSize =
+      memory_.Is64Bit() ? sizeof(uint64_t) : sizeof(uint32_t);
+  const VMAddress buckets_address =
+      dt_gnu_hash_address + sizeof(header) + (kWordSize * header.bloom_size);
+  if (!memory_.Read(buckets_address, kNumBytesForBuckets, buckets.data())) {
+    LOG(ERROR) << "read buckets";
+    return false;
+  }
+
+  // Locate the chain that handles the largest index bucket.
+  uint32_t last_symbol = 0;
+  for (uint32_t i = 0; i < header.nbuckets; ++i) {
+    last_symbol = std::max(buckets[i], last_symbol);
+  }
+
+  if (last_symbol < header.symoffset) {
+    *number_of_symbol_table_entries = header.symoffset;
+    return true;
+  }
+
+  // Walk the bucket's chain to add the chain length to the total.
+  const VMAddress chains_base_address = buckets_address + kNumBytesForBuckets;
+  for (;;) {
+    uint32_t chain_entry;
+    if (!memory_.Read(chains_base_address + (last_symbol - header.symoffset) *
+                                                sizeof(chain_entry),
+                      sizeof(chain_entry),
+                      &chain_entry)) {
+      LOG(ERROR) << "read chain entry";
+      return false;
+    }
+
+    ++last_symbol;
+
+    // If the low bit is set, this entry is the end of the chain.
+    if (chain_entry & 1)
+      break;
+  }
+
+  *number_of_symbol_table_entries = last_symbol;
+  return true;
+}
+
 std::unique_ptr<ElfImageReader::NoteReader> ElfImageReader::Notes(
     ssize_t max_note_size) {
   return std::make_unique<NoteReader>(

snapshot/elf/elf_image_reader.h

@@ -207,6 +207,33 @@ class ElfImageReader {
   //! The caller does not take ownership of the returned object.
   const ProcessMemoryRange* Memory() const;
 
+  //! \brief Retrieves the number of symbol table entries in `DT_SYMTAB`
+  //!     according to the data in the `DT_HASH` section.
+  //!
+  //! \note Exposed for testing, not normally otherwise useful.
+  //!
+  //! \param[out] number_of_symbol_table_entries The number of entries expected
+  //!     in `DT_SYMTAB`.
+  //! \return `true` if a `DT_HASH` section was found, and was read
+  //!     successfully, otherwise `false` with an error logged.
+  bool GetNumberOfSymbolEntriesFromDtHash(
+      VMSize* number_of_symbol_table_entries);
+
+  //! \brief Retrieves the number of symbol table entries in `DT_SYMTAB`
+  //!     according to the data in the `DT_GNU_HASH` section.
+  //!
+  //! \note Exposed for testing, not normally otherwise useful.
+  //!
+  //! \note Depending on the linker that generated the `DT_GNU_HASH` section,
+  //!     this value may not be as expected if there are zero exported symbols.
+  //!
+  //! \param[out] number_of_symbol_table_entries The number of entries expected
+  //!     in `DT_SYMTAB`.
+  //! \return `true` if a `DT_GNU_HASH` section was found, and was read
+  //!     successfully, otherwise `false` with an error logged.
+  bool GetNumberOfSymbolEntriesFromDtGnuHash(
+      VMSize* number_of_symbol_table_entries);
+
  private:
   template <typename PhdrType>
   class ProgramHeaderTableSpecific;
@@ -214,7 +241,7 @@ class ElfImageReader {
   bool InitializeProgramHeaders();
   bool InitializeDynamicArray();
   bool InitializeDynamicSymbolTable();
-  bool GetAddressFromDynamicArray(uint64_t tag, VMAddress* address);
+  bool GetAddressFromDynamicArray(uint64_t tag, bool log, VMAddress* address);
 
   union {
     Elf32_Ehdr header_32_;

snapshot/elf/elf_image_reader_test.cc

@@ -15,6 +15,7 @@
 #include "snapshot/elf/elf_image_reader.h"
 
 #include <dlfcn.h>
+#include <link.h>
 #include <unistd.h>
 
 #include "base/logging.h"
@@ -22,6 +23,8 @@
 #include "gtest/gtest.h"
 #include "test/multiprocess_exec.h"
 #include "test/process_type.h"
+#include "test/scoped_module_handle.h"
+#include "test/test_paths.h"
 #include "util/file/file_io.h"
 #include "util/misc/address_types.h"
 #include "util/misc/from_pointer_cast.h"
@@ -29,7 +32,6 @@
 
 #if defined(OS_FUCHSIA)
 
-#include <link.h>
 #include <zircon/syscalls.h>
 
 #include "base/fuchsia/fuchsia_logging.h"
@@ -288,6 +290,63 @@ TEST(ElfImageReader, OneModuleChild) {
   test.Run();
 }
 
+#if defined(OS_FUCHSIA)
+
+// crashpad_snapshot_test_both_dt_hash_styles is specially built and forced to
+// include both .hash and .gnu.hash sections. Linux, Android, and Fuchsia have
+// different defaults for which of these sections should be included; this test
+// confirms that we get the same count from both sections.
+//
+// TODO(scottmg): Investigation in https://crrev.com/c/876879 resulted in
+// realizing that ld.bfd does not emit a .gnu.hash that is very useful for this
+// purpose when there's 0 exported entries in the module. This is not likely to
+// be too important, as there's little need to look up non-exported symbols.
+// However, it makes this test not work on Linux, where the default build uses
+// ld.bfd. On Fuchsia, the only linker in use is lld, and it generates the
+// expected .gnu.hash. So, for now, this test is only run on Fuchsia, not Linux.
+//
+// TODO(scottmg): Separately, the location of the ELF on Android needs some
+// work, and then the test could also be enabled there.
+TEST(ElfImageReader, DtHashAndDtGnuHashMatch) {
+  base::FilePath module_path =
+      TestPaths::BuildArtifact(FILE_PATH_LITERAL("snapshot"),
+                               FILE_PATH_LITERAL("both_dt_hash_styles"),
+                               TestPaths::FileType::kLoadableModule);
+  // TODO(scottmg): Remove this when upstream Fuchsia bug ZX-1619 is resolved.
+  // See also explanation in build/run_tests.py for Fuchsia .so files.
+  module_path = module_path.BaseName();
+  ScopedModuleHandle module(
+      dlopen(module_path.value().c_str(), RTLD_LAZY | RTLD_LOCAL));
+  ASSERT_TRUE(module.valid()) << "dlopen " << module_path.value() << ": "
+                              << dlerror();
+
+#if defined(ARCH_CPU_64_BITS)
+  constexpr bool am_64_bit = true;
+#else
+  constexpr bool am_64_bit = false;
+#endif  // ARCH_CPU_64_BITS
+
+  ProcessMemoryNative memory;
+  ASSERT_TRUE(memory.Initialize(GetSelfProcess()));
+  ProcessMemoryRange range;
+  ASSERT_TRUE(range.Initialize(&memory, am_64_bit));
+
+  struct link_map* lm = reinterpret_cast<struct link_map*>(module.get());
+
+  ElfImageReader reader;
+  ASSERT_TRUE(reader.Initialize(range, lm->l_addr));
+
+  VMSize from_dt_hash;
+  ASSERT_TRUE(reader.GetNumberOfSymbolEntriesFromDtHash(&from_dt_hash));
+
+  VMSize from_dt_gnu_hash;
+  ASSERT_TRUE(reader.GetNumberOfSymbolEntriesFromDtGnuHash(&from_dt_gnu_hash));
+
+  EXPECT_EQ(from_dt_hash, from_dt_gnu_hash);
+}
+
+#endif  // OS_FUCHSIA
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

snapshot/elf/elf_symbol_table_reader.cc

@@ -51,8 +51,12 @@ uint8_t GetVisibility(const Elf64_Sym& sym) {
 
 ElfSymbolTableReader::ElfSymbolTableReader(const ProcessMemoryRange* memory,
                                            ElfImageReader* elf_reader,
-                                           VMAddress address)
-    : memory_(memory), elf_reader_(elf_reader), base_address_(address) {}
+                                           VMAddress address,
+                                           VMSize num_entries)
+    : memory_(memory),
+      elf_reader_(elf_reader),
+      base_address_(address),
+      num_entries_(num_entries) {}
 
 ElfSymbolTableReader::~ElfSymbolTableReader() {}
 
@@ -68,9 +72,10 @@ bool ElfSymbolTableReader::ScanSymbolTable(const std::string& name,
   VMAddress address = base_address_;
   SymEnt entry;
   std::string string;
-  while (memory_->Read(address, sizeof(entry), &entry) &&
-         elf_reader_->ReadDynamicStringTableAtOffset(entry.st_name, &string)) {
-    if (string == name) {
+  size_t i = 0;
+  while (i < num_entries_ && memory_->Read(address, sizeof(entry), &entry)) {
+    if (elf_reader_->ReadDynamicStringTableAtOffset(entry.st_name, &string) &&
+        string == name) {
       info_out->address = entry.st_value;
       info_out->size = entry.st_size;
       info_out->shndx = entry.st_shndx;
@@ -79,7 +84,9 @@ bool ElfSymbolTableReader::ScanSymbolTable(const std::string& name,
       info_out->visibility = GetVisibility(entry);
       return true;
     }
+    // TODO(scottmg): This should respect DT_SYMENT if present.
     address += sizeof(entry);
+    ++i;
   }
   return false;
 }

snapshot/elf/elf_symbol_table_reader.h

@@ -61,7 +61,8 @@ class ElfSymbolTableReader {
   // lookup.
   ElfSymbolTableReader(const ProcessMemoryRange* memory,
                        ElfImageReader* elf_reader,
-                       VMAddress address);
+                       VMAddress address,
+                       VMSize num_entries);
   ~ElfSymbolTableReader();
 
   //! \brief Lookup information about a symbol.
@@ -78,6 +79,7 @@ class ElfSymbolTableReader {
   const ProcessMemoryRange* const memory_;  // weak
   ElfImageReader* const elf_reader_;  // weak
   const VMAddress base_address_;
+  const VMSize num_entries_;
 
   DISALLOW_COPY_AND_ASSIGN(ElfSymbolTableReader);
 };

snapshot/hash_types_test.cc

@@ -0,0 +1,17 @@
+// Copyright 2018 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+int main() {
+  return 0;
+}

snapshot/snapshot_test.gyp

@@ -52,6 +52,7 @@
       'target_name': 'crashpad_snapshot_test',
       'type': 'executable',
       'dependencies': [
+        'crashpad_snapshot_test_both_dt_hash_styles',
         'crashpad_snapshot_test_module',
         'crashpad_snapshot_test_module_large',
         'crashpad_snapshot_test_module_small',
@@ -193,6 +194,18 @@
         'crashpad_info_size_test_module.cc',
       ],
     },
+    {
+      'target_name': 'crashpad_snapshot_test_both_dt_hash_styles',
+      'type': 'executable',
+      'sources': [
+        'hash_types_test.cc',
+      ],
+
+      'ldflags': [
+        # This makes `ld` emit both .hash and .gnu.hash sections.
+        '-Wl,--hash-style=both',
+      ],
+    },
   ],
   'conditions': [
     ['OS=="mac"', {