Chrome on Android now supports loading the shared library directly from the APK file.

This patch makes two changes to breakpad to enable crash reporting to work correctly when the library is inside another file (an archive):

- Do not filter mappings which map an executable at a non-zero offset.
- If such an executable is mapped look in the ELF information for the
shared object name and use that name in the minidump.

Note this change doesn't care about the archive format and isn't Android
specific (though loading the shared library this way is currently only done on Android).

BUG=390618
R=thestig@chromium.org

Review URL: https://breakpad.appspot.com/7684002

Patch from Anton Carver <anton@chromium.org>.

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

src/client/linux/minidump_writer/linux_core_dumper.cc

@@ -124,7 +124,7 @@ bool LinuxCoreDumper::ThreadsResume() {
 }
 
 bool LinuxCoreDumper::EnumerateThreads() {
-  if (!mapped_core_file_.Map(core_path_)) {
+  if (!mapped_core_file_.Map(core_path_, 0)) {
     fprintf(stderr, "Could not map core dump file into memory\n");
     return false;
   }

src/client/linux/minidump_writer/linux_dumper.cc

@@ -38,12 +38,14 @@
 #include "client/linux/minidump_writer/linux_dumper.h"
 
 #include <assert.h>
+#include <elf.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <stddef.h>
 #include <string.h>
 
 #include "client/linux/minidump_writer/line_reader.h"
+#include "common/linux/elfutils.h"
 #include "common/linux/file_id.h"
 #include "common/linux/linux_libc_support.h"
 #include "common/linux/memory_mapped_file.h"
@@ -115,15 +117,16 @@ LinuxDumper::ElfFileIdentifierForMapping(const MappingInfo& mapping,
 
   char filename[NAME_MAX];
   size_t filename_len = my_strlen(mapping.name);
-  assert(filename_len < NAME_MAX);
-  if (filename_len >= NAME_MAX)
+  if (filename_len >= NAME_MAX) {
+    assert(false);
     return false;
+  }
   my_memcpy(filename, mapping.name, filename_len);
   filename[filename_len] = '\0';
   bool filename_modified = HandleDeletedFileInMapping(filename);
 
-  MemoryMappedFile mapped_file(filename);
-  if (!mapped_file.data())  // Should probably check if size >= ElfW(Ehdr)?
+  MemoryMappedFile mapped_file(filename, mapping.offset);
+  if (!mapped_file.data() || mapped_file.size() < SELFMAG)
     return false;
 
   bool success =
@@ -136,6 +139,80 @@ LinuxDumper::ElfFileIdentifierForMapping(const MappingInfo& mapping,
   return success;
 }
 
+namespace {
+bool ElfFileSoNameFromMappedFile(
+    const void* elf_base, char* soname, size_t soname_size) {
+  if (!IsValidElf(elf_base)) {
+    // Not ELF
+    return false;
+  }
+
+  const void* segment_start;
+  size_t segment_size;
+  int elf_class;
+  if (!FindElfSection(elf_base, ".dynamic", SHT_DYNAMIC,
+                      &segment_start, &segment_size, &elf_class)) {
+    // No dynamic section
+    return false;
+  }
+
+  const void* dynstr_start;
+  size_t dynstr_size;
+  if (!FindElfSection(elf_base, ".dynstr", SHT_STRTAB,
+                      &dynstr_start, &dynstr_size, &elf_class)) {
+    // No dynstr section
+    return false;
+  }
+
+  const ElfW(Dyn)* dynamic = static_cast<const ElfW(Dyn)*>(segment_start);
+  size_t dcount = segment_size / sizeof(ElfW(Dyn));
+  for (const ElfW(Dyn)* dyn = dynamic; dyn < dynamic + dcount; ++dyn) {
+    if (dyn->d_tag == DT_SONAME) {
+      const char* dynstr = static_cast<const char*>(dynstr_start);
+      if (dyn->d_un.d_val >= dynstr_size) {
+        // Beyond the end of the dynstr section
+        return false;
+      }
+      const char* str = dynstr + dyn->d_un.d_val;
+      const size_t maxsize = dynstr_size - dyn->d_un.d_val;
+      my_strlcpy(soname, str, maxsize < soname_size ? maxsize : soname_size);
+      return true;
+    }
+  }
+
+  // Did not find SONAME
+  return false;
+}
+}  // namespace
+
+// static
+bool LinuxDumper::ElfFileSoName(
+    const MappingInfo& mapping, char* soname, size_t soname_size) {
+  if (IsMappedFileOpenUnsafe(mapping)) {
+    // Not safe
+    return false;
+  }
+
+  char filename[NAME_MAX];
+  size_t filename_len = my_strlen(mapping.name);
+  if (filename_len >= NAME_MAX) {
+    assert(false);
+    // name too long
+    return false;
+  }
+
+  my_memcpy(filename, mapping.name, filename_len);
+  filename[filename_len] = '\0';
+
+  MemoryMappedFile mapped_file(filename, mapping.offset);
+  if (!mapped_file.data() || mapped_file.size() < SELFMAG) {
+    // mmap failed
+    return false;
+  }
+
+  return ElfFileSoNameFromMappedFile(mapped_file.data(), soname, soname_size);
+}
+
 bool LinuxDumper::ReadAuxv() {
   char auxv_path[NAME_MAX];
   if (!BuildProcPath(auxv_path, pid_, "auxv")) {
@@ -195,6 +272,7 @@ bool LinuxDumper::EnumerateMappings() {
     if (*i1 == '-') {
       const char* i2 = my_read_hex_ptr(&end_addr, i1 + 1);
       if (*i2 == ' ') {
+        bool exec = (*(i2 + 3) == 'x');
         const char* i3 = my_read_hex_ptr(&offset, i2 + 6 /* skip ' rwxp ' */);
         if (*i3 == ' ') {
           const char* name = NULL;
@@ -223,6 +301,7 @@ bool LinuxDumper::EnumerateMappings() {
           module->start_addr = start_addr;
           module->size = end_addr - start_addr;
           module->offset = offset;
+          module->exec = exec;
           if (name != NULL) {
             const unsigned l = my_strlen(name);
             if (l < sizeof(module->name))

src/client/linux/minidump_writer/linux_dumper.h

@@ -107,6 +107,7 @@ struct MappingInfo {
   uintptr_t start_addr;
   size_t size;
   size_t offset;  // offset into the backed file.
+  bool exec;  // true if the mapping has the execute bit set.
   char name[NAME_MAX];
 };
 
@@ -162,6 +163,13 @@ class LinuxDumper {
                                    unsigned int mapping_id,
                                    uint8_t identifier[sizeof(MDGUID)]);
 
+  // Find the shared object name (SONAME) by examining the ELF information
+  // for |mapping|. If the SONAME is found copy it into the passed buffer
+  // |soname| and return true. The size of the buffer is |soname_size|.
+  // The SONAME will be truncated if it is too long to fit in the buffer.
+  static bool ElfFileSoName(
+      const MappingInfo& mapping, char* soname, size_t soname_size);
+
   uintptr_t crash_address() const { return crash_address_; }
   void set_crash_address(uintptr_t crash_address) {
     crash_address_ = crash_address;

src/client/linux/minidump_writer/minidump_writer.cc

@@ -938,7 +938,9 @@ class MinidumpWriter {
 
   static bool ShouldIncludeMapping(const MappingInfo& mapping) {
     if (mapping.name[0] == 0 ||  // only want modules with filenames.
-        mapping.offset ||  // only want to include one mapping per shared lib.
+        // Only want to include one mapping per shared lib.
+        // Avoid filtering executable mappings.
+        (mapping.offset != 0 && !mapping.exec) ||
         mapping.size < 4096) {  // too small to get a signature for.
       return false;
     }
@@ -1029,7 +1031,8 @@ class MinidumpWriter {
 
     mod.base_of_image = mapping.start_addr;
     mod.size_of_image = mapping.size;
-    const size_t filepath_len = my_strlen(mapping.name);
+    const char* filepath_ptr = mapping.name;
+    size_t filepath_len = my_strlen(mapping.name);
 
     // Figure out file name from path
     const char* filename_ptr = mapping.name + filepath_len - 1;
@@ -1040,7 +1043,31 @@ class MinidumpWriter {
     }
     filename_ptr++;
 
-    const size_t filename_len = mapping.name + filepath_len - filename_ptr;
+    size_t filename_len = mapping.name + filepath_len - filename_ptr;
+
+    // If an executable is mapped from a non-zero offset, this is likely
+    // because the executable was loaded directly from inside an archive
+    // file. We try to find the name of the shared object (SONAME) by
+    // looking in the file for ELF sections.
+
+    char soname[NAME_MAX];
+    char pathname[NAME_MAX];
+    if (mapping.exec && mapping.offset != 0 &&
+        LinuxDumper::ElfFileSoName(mapping, soname, sizeof(soname))) {
+      filename_ptr = soname;
+      filename_len = my_strlen(soname);
+
+      if (filepath_len + filename_len + 1 < NAME_MAX) {
+        // It doesn't have a real pathname, but tools such as stackwalk
+        // extract the basename, so simulating a pathname is helpful.
+        my_memcpy(pathname, filepath_ptr, filepath_len);
+        pathname[filepath_len] = '/';
+        my_memcpy(pathname + filepath_len + 1, filename_ptr, filename_len);
+        pathname[filepath_len + filename_len + 1] = '\0';
+        filepath_ptr = pathname;
+        filepath_len = filepath_len + filename_len + 1;
+      }
+    }
 
     uint8_t cv_buf[MDCVInfoPDB70_minsize + NAME_MAX];
     uint8_t* cv_ptr = cv_buf;
@@ -1070,7 +1097,7 @@ class MinidumpWriter {
     mod.cv_record = cv.location();
 
     MDLocationDescriptor ld;
-    if (!minidump_writer_.WriteString(mapping.name, filepath_len, &ld))
+    if (!minidump_writer_.WriteString(filepath_ptr, filepath_len, &ld))
       return false;
     mod.module_name_rva = ld.rva;
     return true;

src/common/linux/elf_core_dump_unittest.cc

@@ -70,7 +70,7 @@ TEST(ElfCoreDumpTest, TestElfHeader) {
   ElfCoreDump core;
 
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header) - 1));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
   EXPECT_EQ(NULL, core.GetHeader());
@@ -80,49 +80,49 @@ TEST(ElfCoreDumpTest, TestElfHeader) {
   EXPECT_FALSE(core.GetFirstNote().IsValid());
 
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_ident[0] = ELFMAG0;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_ident[1] = ELFMAG1;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_ident[2] = ELFMAG2;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_ident[3] = ELFMAG3;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_ident[4] = ElfCoreDump::kClass;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_version = EV_CURRENT;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_FALSE(core.IsValid());
 
   header.e_type = ET_CORE;
   ASSERT_TRUE(WriteFile(core_file, &header, sizeof(header)));
-  ASSERT_TRUE(mapped_core_file.Map(core_file));
+  ASSERT_TRUE(mapped_core_file.Map(core_file, 0));
   core.SetContent(mapped_core_file.content());
   EXPECT_TRUE(core.IsValid());
 }
@@ -156,7 +156,8 @@ TEST(ElfCoreDumpTest, ValidCoreFile) {
 #endif
 
   MemoryMappedFile mapped_core_file;
-  ASSERT_TRUE(mapped_core_file.Map(crash_generator.GetCoreFilePath().c_str()));
+  ASSERT_TRUE(
+      mapped_core_file.Map(crash_generator.GetCoreFilePath().c_str(), 0));
 
   ElfCoreDump core;
   core.SetContent(mapped_core_file.content());

src/common/linux/file_id.cc

@@ -149,7 +149,7 @@ bool FileID::ElfFileIdentifierFromMappedFile(const void* base,
 }
 
 bool FileID::ElfFileIdentifier(uint8_t identifier[kMDGUIDSize]) {
-  MemoryMappedFile mapped_file(path_.c_str());
+  MemoryMappedFile mapped_file(path_.c_str(), 0);
   if (!mapped_file.data())  // Should probably check if size >= ElfW(Ehdr)?
     return false;
 

src/common/linux/memory_mapped_file.cc

@@ -46,15 +46,15 @@ namespace google_breakpad {
 
 MemoryMappedFile::MemoryMappedFile() {}
 
-MemoryMappedFile::MemoryMappedFile(const char* path) {
-  Map(path);
+MemoryMappedFile::MemoryMappedFile(const char* path, size_t offset) {
+  Map(path, offset);
 }
 
 MemoryMappedFile::~MemoryMappedFile() {
   Unmap();
 }
 
-bool MemoryMappedFile::Map(const char* path) {
+bool MemoryMappedFile::Map(const char* path, size_t offset) {
   Unmap();
 
   int fd = sys_open(path, O_RDONLY, 0);
@@ -73,25 +73,33 @@ bool MemoryMappedFile::Map(const char* path) {
     return false;
   }
 
-  // If the file size is zero, simply use an empty MemoryRange and return
-  // true. Don't bother to call mmap() even though mmap() can handle an
-  // empty file on some platforms.
-  if (st.st_size == 0) {
+  // Strangely file size can be negative, but we check above that it is not.
+  size_t file_len = static_cast<size_t>(st.st_size);
+  // If the file does not extend beyond the offset, simply use an empty
+  // MemoryRange and return true. Don't bother to call mmap()
+  // even though mmap() can handle an empty file on some platforms.
+  if (offset >= file_len) {
     sys_close(fd);
     return true;
   }
 
 #if defined(__x86_64__) || defined(__aarch64__)
-  void* data = sys_mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+  void* data = sys_mmap(NULL, file_len, PROT_READ, MAP_PRIVATE, fd, offset);
 #else
-  void* data = sys_mmap2(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+  if ((offset & 4095) != 0) {
+    // Not page aligned.
+    sys_close(fd);
+    return false;
+  }
+  void* data = sys_mmap2(
+      NULL, file_len, PROT_READ, MAP_PRIVATE, fd, offset >> 12);
 #endif
   sys_close(fd);
   if (data == MAP_FAILED) {
     return false;
   }
 
-  content_.Set(data, st.st_size);
+  content_.Set(data, file_len - offset);
   return true;
 }
 

src/common/linux/memory_mapped_file.h

@@ -33,6 +33,7 @@
 #ifndef COMMON_LINUX_MEMORY_MAPPED_FILE_H_
 #define COMMON_LINUX_MEMORY_MAPPED_FILE_H_
 
+#include <stddef.h>
 #include "common/basictypes.h"
 #include "common/memory_range.h"
 
@@ -47,7 +48,7 @@ class MemoryMappedFile {
 
   // Constructor that calls Map() to map a file at |path| into memory.
   // If Map() fails, the object behaves as if it is default constructed.
-  explicit MemoryMappedFile(const char* path);
+  MemoryMappedFile(const char* path, size_t offset);
 
   ~MemoryMappedFile();
 
@@ -56,7 +57,7 @@ class MemoryMappedFile {
   // success. Mapping an empty file will succeed but with data() and size()
   // returning NULL and 0, respectively. An existing mapping is unmapped
   // before a new mapping is created.
-  bool Map(const char* path);
+  bool Map(const char* path, size_t offset);
 
   // Unmaps the memory for the mapped file. It's a no-op if no file is
   // mapped.

src/common/linux/memory_mapped_file_unittest.cc

@@ -71,12 +71,12 @@ TEST_F(MemoryMappedFileTest, UnmapWithoutMap) {
 
 TEST_F(MemoryMappedFileTest, MapNonexistentFile) {
   {
-    MemoryMappedFile mapped_file("nonexistent-file");
+    MemoryMappedFile mapped_file("nonexistent-file", 0);
     ExpectNoMappedData(mapped_file);
   }
   {
     MemoryMappedFile mapped_file;
-    EXPECT_FALSE(mapped_file.Map("nonexistent-file"));
+    EXPECT_FALSE(mapped_file.Map("nonexistent-file", 0));
     ExpectNoMappedData(mapped_file);
   }
 }
@@ -87,12 +87,12 @@ TEST_F(MemoryMappedFileTest, MapEmptyFile) {
   ASSERT_TRUE(WriteFile(test_file.c_str(), NULL, 0));
 
   {
-    MemoryMappedFile mapped_file(test_file.c_str());
+    MemoryMappedFile mapped_file(test_file.c_str(), 0);
     ExpectNoMappedData(mapped_file);
   }
   {
     MemoryMappedFile mapped_file;
-    EXPECT_TRUE(mapped_file.Map(test_file.c_str()));
+    EXPECT_TRUE(mapped_file.Map(test_file.c_str(), 0));
     ExpectNoMappedData(mapped_file);
   }
 }
@@ -109,7 +109,7 @@ TEST_F(MemoryMappedFileTest, MapNonEmptyFile) {
   ASSERT_TRUE(WriteFile(test_file.c_str(), data, data_size));
 
   {
-    MemoryMappedFile mapped_file(test_file.c_str());
+    MemoryMappedFile mapped_file(test_file.c_str(), 0);
     EXPECT_FALSE(mapped_file.content().IsEmpty());
     EXPECT_TRUE(mapped_file.data() != NULL);
     EXPECT_EQ(data_size, mapped_file.size());
@@ -117,7 +117,7 @@ TEST_F(MemoryMappedFileTest, MapNonEmptyFile) {
   }
   {
     MemoryMappedFile mapped_file;
-    EXPECT_TRUE(mapped_file.Map(test_file.c_str()));
+    EXPECT_TRUE(mapped_file.Map(test_file.c_str(), 0));
     EXPECT_FALSE(mapped_file.content().IsEmpty());
     EXPECT_TRUE(mapped_file.data() != NULL);
     EXPECT_EQ(data_size, mapped_file.size());
@@ -145,13 +145,13 @@ TEST_F(MemoryMappedFileTest, RemapAfterMap) {
   ASSERT_TRUE(WriteFile(test_file2.c_str(), data2, data2_size));
 
   {
-    MemoryMappedFile mapped_file(test_file1.c_str());
+    MemoryMappedFile mapped_file(test_file1.c_str(), 0);
     EXPECT_FALSE(mapped_file.content().IsEmpty());
     EXPECT_TRUE(mapped_file.data() != NULL);
     EXPECT_EQ(data1_size, mapped_file.size());
     EXPECT_EQ(0, memcmp(data1, mapped_file.data(), data1_size));
 
-    mapped_file.Map(test_file2.c_str());
+    mapped_file.Map(test_file2.c_str(), 0);
     EXPECT_FALSE(mapped_file.content().IsEmpty());
     EXPECT_TRUE(mapped_file.data() != NULL);
     EXPECT_EQ(data2_size, mapped_file.size());
@@ -159,16 +159,50 @@ TEST_F(MemoryMappedFileTest, RemapAfterMap) {
   }
   {
     MemoryMappedFile mapped_file;
-    EXPECT_TRUE(mapped_file.Map(test_file1.c_str()));
+    EXPECT_TRUE(mapped_file.Map(test_file1.c_str(), 0));
     EXPECT_FALSE(mapped_file.content().IsEmpty());
     EXPECT_TRUE(mapped_file.data() != NULL);
     EXPECT_EQ(data1_size, mapped_file.size());
     EXPECT_EQ(0, memcmp(data1, mapped_file.data(), data1_size));
 
-    mapped_file.Map(test_file2.c_str());
+    mapped_file.Map(test_file2.c_str(), 0);
     EXPECT_FALSE(mapped_file.content().IsEmpty());
     EXPECT_TRUE(mapped_file.data() != NULL);
     EXPECT_EQ(data2_size, mapped_file.size());
     EXPECT_EQ(0, memcmp(data2, mapped_file.data(), data2_size));
   }
 }
+
+TEST_F(MemoryMappedFileTest, MapWithOffset) {
+  // Put more data in the test file this time. Offsets can only be
+  // done on page boundaries, so we need a two page file to test this.
+  const int page_size = 4096;
+  char data1[2 * page_size];
+  size_t data1_size = sizeof(data1);
+  for (size_t i = 0; i < data1_size; ++i) {
+    data1[i] = i & 0x7f;
+  }
+
+  AutoTempDir temp_dir;
+  string test_file1 = temp_dir.path() + "/test_file1";
+  ASSERT_TRUE(WriteFile(test_file1.c_str(), data1, data1_size));
+  {
+    MemoryMappedFile mapped_file(test_file1.c_str(), page_size);
+    EXPECT_FALSE(mapped_file.content().IsEmpty());
+    EXPECT_TRUE(mapped_file.data() != NULL);
+    EXPECT_EQ(data1_size - page_size, mapped_file.size());
+    EXPECT_EQ(
+        0,
+        memcmp(data1 + page_size, mapped_file.data(), data1_size - page_size));
+  }
+  {
+    MemoryMappedFile mapped_file;
+    mapped_file.Map(test_file1.c_str(), page_size);
+    EXPECT_FALSE(mapped_file.content().IsEmpty());
+    EXPECT_TRUE(mapped_file.data() != NULL);
+    EXPECT_EQ(data1_size - page_size, mapped_file.size());
+    EXPECT_EQ(
+        0,
+        memcmp(data1 + page_size, mapped_file.data(), data1_size - page_size));
+  }
+}

src/tools/linux/md2core/minidump-2-core.cc

@@ -997,7 +997,7 @@ main(int argc, char** argv) {
   if (argc != argi + 1)
     return usage(argv[0]);
 
-  MemoryMappedFile mapped_file(argv[argi]);
+  MemoryMappedFile mapped_file(argv[argi], 0);
   if (!mapped_file.data()) {
     fprintf(stderr, "Failed to mmap dump file\n");
     return 1;