Cleanup Linux debug link file handling code.

- Handle the case when the debug link points back to the object file.
- Move some checks into a separate SanitizeDebugFile() function.

BUG=636
Review URL: https://breakpad.appspot.com/3784002/


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

1 files changed, 74 insertions, 44 deletions

diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc
index eb7f1820..0bcc18ab 100644
--- a/src/common/linux/dump_symbols.cc
+++ b/src/common/linux/dump_symbols.cc
@@ -413,6 +413,15 @@ bool ElfEndianness(const typename ElfClass::Ehdr* elf_header,
   return false;
 }
 
+// Given |left_abspath|, find the absolute path for |right_path| and see if the
+// two absolute paths are the same.
+bool IsSameFile(const char* left_abspath, const string& right_path) {
+  char right_abspath[PATH_MAX];
+  if (!realpath(right_path.c_str(), right_abspath))
+    return false;
+  return strcmp(left_abspath, right_abspath) == 0;
+}
+
 // Read the .gnu_debuglink and get the debug file name. If anything goes
 // wrong, return an empty string.
 string ReadDebugLink(const char* debuglink,
@@ -430,14 +439,27 @@ string ReadDebugLink(const char* debuglink,
     return string();
   }
 
-  bool found = false;
-  int debuglink_fd = -1;
+  char obj_file_abspath[PATH_MAX];
+  if (!realpath(obj_file.c_str(), obj_file_abspath)) {
+    fprintf(stderr, "Cannot resolve absolute path for %s\n", obj_file.c_str());
+    return string();
+  }
+
+  std::vector<string> searched_paths;
   string debuglink_path;
   std::vector<string>::const_iterator it;
   for (it = debug_dirs.begin(); it < debug_dirs.end(); ++it) {
     const string& debug_dir = *it;
     debuglink_path = debug_dir + "/" + debuglink;
-    debuglink_fd = open(debuglink_path.c_str(), O_RDONLY);
+
+    // There is the annoying case of /path/to/foo.so having foo.so as the
+    // debug link file name. Thus this may end up opening /path/to/foo.so again,
+    // and there is a small chance of the two files having the same CRC.
+    if (IsSameFile(obj_file_abspath, debuglink_path))
+      continue;
+
+    searched_paths.push_back(debug_dir);
+    int debuglink_fd = open(debuglink_path.c_str(), O_RDONLY);
     if (debuglink_fd < 0)
       continue;
 
@@ -469,21 +491,19 @@ string ReadDebugLink(const char* debuglink,
               debuglink_path.c_str());
       continue;
     }
-    found = true;
-    break;
-  }
 
-  if (!found) {
-    fprintf(stderr, "Failed to find debug ELF file for '%s' after trying:\n",
-            obj_file.c_str());
-    for (it = debug_dirs.begin(); it < debug_dirs.end(); ++it) {
-      const string debug_dir = *it;
-      fprintf(stderr, "  %s/%s\n", debug_dir.c_str(), debuglink);
-    }
-    return string();
+    // Found debug file.
+    return debuglink_path;
   }
 
-  return debuglink_path;
+  // Not found case.
+  fprintf(stderr, "Failed to find debug ELF file for '%s' after trying:\n",
+          obj_file.c_str());
+  for (it = searched_paths.begin(); it < searched_paths.end(); ++it) {
+    const string& debug_dir = *it;
+    fprintf(stderr, "  %s/%s\n", debug_dir.c_str(), debuglink);
+  }
+  return string();
 }
 
 //
@@ -801,11 +821,42 @@ string BaseFileName(const string &filename) {
 }
 
 template<typename ElfClass>
+bool SanitizeDebugFile(const typename ElfClass::Ehdr* debug_elf_header,
+                       const string& debuglink_file,
+                       const string& obj_filename,
+                       const char* obj_file_architecture,
+                       const bool obj_file_is_big_endian) {
+  const char* debug_architecture =
+      ElfArchitecture<ElfClass>(debug_elf_header);
+  if (!debug_architecture) {
+    fprintf(stderr, "%s: unrecognized ELF machine architecture: %d\n",
+            debuglink_file.c_str(), debug_elf_header->e_machine);
+    return false;
+  }
+  if (strcmp(obj_file_architecture, debug_architecture)) {
+    fprintf(stderr, "%s with ELF machine architecture %s does not match "
+            "%s with ELF architecture %s\n",
+            debuglink_file.c_str(), debug_architecture,
+            obj_filename.c_str(), obj_file_architecture);
+    return false;
+  }
+  bool debug_big_endian;
+  if (!ElfEndianness<ElfClass>(debug_elf_header, &debug_big_endian))
+    return false;
+  if (debug_big_endian != obj_file_is_big_endian) {
+    fprintf(stderr, "%s and %s does not match in endianness\n",
+            obj_filename.c_str(), debuglink_file.c_str());
+    return false;
+  }
+  return true;
+}
+
+template<typename ElfClass>
 bool ReadSymbolDataElfClass(const typename ElfClass::Ehdr* elf_header,
-                             const string& obj_filename,
-                             const std::vector<string>& debug_dirs,
-                             const DumpOptions& options,
-                             Module** out_module) {
+                            const string& obj_filename,
+                            const std::vector<string>& debug_dirs,
+                            const DumpOptions& options,
+                            Module** out_module) {
   typedef typename ElfClass::Ehdr Ehdr;
   typedef typename ElfClass::Shdr Shdr;
 
@@ -849,34 +900,13 @@ bool ReadSymbolDataElfClass(const typename ElfClass::Ehdr* elf_header,
     MmapWrapper debug_map_wrapper;
     Ehdr* debug_elf_header = NULL;
     if (!LoadELF(debuglink_file, &debug_map_wrapper,
-                 reinterpret_cast<void**>(&debug_elf_header)))
-      return false;
-    // Sanity checks to make sure everything matches up.
-    const char *debug_architecture =
-        ElfArchitecture<ElfClass>(debug_elf_header);
-    if (!debug_architecture) {
-      fprintf(stderr, "%s: unrecognized ELF machine architecture: %d\n",
-              debuglink_file.c_str(), debug_elf_header->e_machine);
-      return false;
-    }
-    if (strcmp(architecture, debug_architecture)) {
-      fprintf(stderr, "%s with ELF machine architecture %s does not match "
-              "%s with ELF architecture %s\n",
-              debuglink_file.c_str(), debug_architecture,
-              obj_filename.c_str(), architecture);
-      return false;
-    }
-
-    bool debug_big_endian;
-    if (!ElfEndianness<ElfClass>(debug_elf_header, &debug_big_endian))
-      return false;
-    if (debug_big_endian != big_endian) {
-      fprintf(stderr, "%s and %s does not match in endianness\n",
-              obj_filename.c_str(), debuglink_file.c_str());
+                 reinterpret_cast<void**>(&debug_elf_header)) ||
+        !SanitizeDebugFile<ElfClass>(debug_elf_header, debuglink_file,
+                                     obj_filename, architecture, big_endian)) {
       return false;
     }
 
-    if (!LoadSymbols<ElfClass>(debuglink_file, debug_big_endian,
+    if (!LoadSymbols<ElfClass>(debuglink_file, big_endian,
                                debug_elf_header, false, &info,
                                options, module.get())) {
       return false;