[Libunwind-devel] [PATCH V2] elf: Follow .gnu_debuglink when resolving function names

[Dave Watson]

Centralize gnu_debuglink logic in elfxx.  Remove previous duplicated logic
from Gfind_proc_info and os-linux.

Logic is roughly the same as previous load_debug_frame, but uses VLAs
instead of malloc.
---
 src/dwarf/Gfind_proc_info-lsb.c | 149 ++++++----------------------------------
 src/elfxx.c                     |  96 +++++++++++++++++++++++++-
 src/elfxx.h                     |   1 +
 src/os-linux.c                  |   8 +--
 4 files changed, 117 insertions(+), 137 deletions(-)

diff --git a/src/dwarf/Gfind_proc_info-lsb.c b/src/dwarf/Gfind_proc_info-lsb.c
index af0d75c..4559495 100644
--- a/src/dwarf/Gfind_proc_info-lsb.c
+++ b/src/dwarf/Gfind_proc_info-lsb.c
@@ -86,151 +86,42 @@ linear_search (unw_addr_space_t as, unw_word_t ip,
 /* Load .debug_frame section from FILE.  Allocates and returns space
    in *BUF, and sets *BUFSIZE to its size.  IS_LOCAL is 1 if using the
    local process, in which case we can search the system debug file
-   directory; 0 for other address spaces, in which case we do not; or
-   -1 for recursive calls following .gnu_debuglink.  Returns 0 on
-   success, 1 on error.  Succeeds even if the file contains no
-   .debug_frame.  */
+   directory; 0 for other address spaces, in which case we do
+   not. Returns 0 on success, 1 on error.  Succeeds even if the file
+   contains no .debug_frame.  */
 /* XXX: Could use mmap; but elf_map_image keeps tons mapped in.  */
 
 static int
 load_debug_frame (const char *file, char **buf, size_t *bufsize, int is_local)
 {
-  FILE *f;
-  Elf_W (Ehdr) ehdr;
-  Elf_W (Half) shstrndx;
-  Elf_W (Shdr) *sec_hdrs = NULL;
-  char *stringtab = NULL;
-  unsigned int i;
-  size_t linksize = 0;
-  char *linkbuf = NULL;
-  
-  *buf = NULL;
-  *bufsize = 0;
-  
-  f = fopen (file, "r");
-  
-  if (!f)
-    return 1;
-  
-  if (fread (&ehdr, sizeof (Elf_W (Ehdr)), 1, f) != 1)
-    goto file_error;
-  
-  shstrndx = ehdr.e_shstrndx;
-  
-  Debug (4, "opened file '%s'. Section header at offset %d\n",
-         file, (int) ehdr.e_shoff);
-
-  fseek (f, ehdr.e_shoff, SEEK_SET);
-  sec_hdrs = calloc (ehdr.e_shnum, sizeof (Elf_W (Shdr)));
-  if (fread (sec_hdrs, sizeof (Elf_W (Shdr)), ehdr.e_shnum, f) != ehdr.e_shnum)
-    goto file_error;
-  
-  Debug (4, "loading string table of size %zd\n",
-           sec_hdrs[shstrndx].sh_size);
-  stringtab = malloc (sec_hdrs[shstrndx].sh_size);
-  fseek (f, sec_hdrs[shstrndx].sh_offset, SEEK_SET);
-  if (fread (stringtab, 1, sec_hdrs[shstrndx].sh_size, f) != 
sec_hdrs[shstrndx].sh_size)
-    goto file_error;
-  
-  for (i = 1; i < ehdr.e_shnum && *buf == NULL; i++)
-    {
-      char *secname = &stringtab[sec_hdrs[i].sh_name];
-
-      if (strcmp (secname, ".debug_frame") == 0)
-        {
-          *bufsize = sec_hdrs[i].sh_size;
-          *buf = malloc (*bufsize);
-
-          fseek (f, sec_hdrs[i].sh_offset, SEEK_SET);
-          if (fread (*buf, 1, *bufsize, f) != *bufsize)
-            goto file_error;
-
-          Debug (4, "read %zd bytes of .debug_frame from offset %zd\n",
-                 *bufsize, sec_hdrs[i].sh_offset);
-        }
-      else if (strcmp (secname, ".gnu_debuglink") == 0)
-        {
-          linksize = sec_hdrs[i].sh_size;
-          linkbuf = malloc (linksize);
-
-          fseek (f, sec_hdrs[i].sh_offset, SEEK_SET);
-          if (fread (linkbuf, 1, linksize, f) != linksize)
-            goto file_error;
-
-          Debug (4, "read %zd bytes of .gnu_debuglink from offset %zd\n",
-                 linksize, sec_hdrs[i].sh_offset);
-        }
-    }
+  struct elf_image ei;
+  Elf_W (Shdr) *shdr;
+  int ret;
 
-  free (stringtab);
-  free (sec_hdrs);
+  ei.image = NULL;
 
-  fclose (f);
+  ret = elf_w (load_debuglink) (file, &ei, is_local);
+  if (ret != 0)
+    return ret;
 
-  /* Ignore separate debug files which contain a .gnu_debuglink section. */
-  if (linkbuf && is_local == -1)
+  shdr = elf_w (find_section) (&ei, ".debug_frame");
+  if (!shdr ||
+      (shdr->sh_offset + shdr->sh_size > ei.size))
     {
-      free (linkbuf);
+      munmap(ei.image, ei.size);
       return 1;
     }
 
-  if (*buf == NULL && linkbuf != NULL && memchr (linkbuf, 0, linksize) != NULL)
-    {
-      char *newname, *basedir, *p;
-      static const char *debugdir = "/usr/lib/debug";
-      int ret;
-
-      /* XXX: Don't bother with the checksum; just search for the file.  */
-      basedir = malloc (strlen (file) + 1);
-      newname = malloc (strlen (linkbuf) + strlen (debugdir)
-                        + strlen (file) + 9);
-
-      p = strrchr (file, '/');
-      if (p != NULL)
-        {
-          memcpy (basedir, file, p - file);
-          basedir[p - file] = '\0';
-        }
-      else
-        basedir[0] = 0;
-
-      strcpy (newname, basedir);
-      strcat (newname, "/");
-      strcat (newname, linkbuf);
-      ret = load_debug_frame (newname, buf, bufsize, -1);
+  *bufsize = shdr->sh_size;
+  *buf = malloc (*bufsize);
 
-      if (ret == 1)
-        {
-          strcpy (newname, basedir);
-          strcat (newname, "/.debug/");
-          strcat (newname, linkbuf);
-          ret = load_debug_frame (newname, buf, bufsize, -1);
-        }
-
-      if (ret == 1 && is_local == 1)
-        {
-          strcpy (newname, debugdir);
-          strcat (newname, basedir);
-          strcat (newname, "/");
-          strcat (newname, linkbuf);
-          ret = load_debug_frame (newname, buf, bufsize, -1);
-        }
+  memcpy(*buf, shdr->sh_offset + ei.image, *bufsize);
 
-      free (basedir);
-      free (newname);
-    }
-  free (linkbuf);
+  Debug (4, "read %zd bytes of .debug_frame from offset %zd\n",
+        *bufsize, shdr->sh_offset);
 
+  munmap(ei.image, ei.size);
   return 0;
-
-/* An error reading image file. Release resources and return error code */
-file_error:
-  free(stringtab);
-  free(sec_hdrs);
-  free(linkbuf);
-  fclose(f);
-
-  return 1;
 }
 
 /* Locate the binary which originated the contents of address ADDR. Return
diff --git a/src/elfxx.c b/src/elfxx.c
index 25bd5a2..685cf2f 100644
--- a/src/elfxx.c
+++ b/src/elfxx.c
@@ -312,8 +312,13 @@ elf_w (get_proc_name) (unw_addr_space_t as, pid_t pid, 
unw_word_t ip,
   unsigned long segbase, mapoff;
   struct elf_image ei;
   int ret;
+  char file[PATH_MAX];
 
-  ret = tdep_get_elf_image (&ei, pid, ip, &segbase, &mapoff, NULL, 0);
+  ret = tdep_get_elf_image (&ei, pid, ip, &segbase, &mapoff, file, PATH_MAX);
+  if (ret < 0)
+    return ret;
+
+  ret = elf_w (load_debuglink) (file, &ei, 1);
   if (ret < 0)
     return ret;
 
@@ -368,3 +373,92 @@ elf_w (find_section) (struct elf_image *ei, const char* 
secname)
   /* section not found */
   return 0;
 }
+
+/* Load a debug section, following .gnu_debuglink if appropriate
+ * Loads ei from file if not already mapped.
+ * If is_local, will also search sys directories /usr/local/dbg
+ *
+ * Returns 0 on success, failure otherwise.
+ * ei will be mapped to file or the located .gnu_debuglink from file
+ */
+HIDDEN int
+elf_w (load_debuglink) (const char* file, struct elf_image *ei, int is_local)
+{
+  int ret;
+  Elf_W (Shdr) *shdr;
+
+  if (!ei->image)
+    {
+      ret = elf_map_image(ei, file);
+      if (ret)
+       return ret;
+    }
+
+  /* Ignore separate debug files which contain a .gnu_debuglink section. */
+  if (is_local == -1) {
+    return 0;
+  }
+
+  shdr = elf_w (find_section) (ei, ".gnu_debuglink");
+  if (shdr) {
+    if (shdr->sh_size >= PATH_MAX ||
+       (shdr->sh_offset + shdr->sh_size > ei->size))
+      {
+       return 0;
+      }
+
+    {
+      char linkbuf[shdr->sh_size];
+      char *link = ((char *) ei->image) + shdr->sh_offset;
+      char *p;
+      static const char *debugdir = "/usr/lib/debug";
+      char basedir[strlen(file) + 1];
+      char newname[shdr->sh_size + strlen (debugdir) + strlen (file) + 9];
+
+      memcpy(linkbuf, link, shdr->sh_size);
+
+      if (memchr (linkbuf, 0, shdr->sh_size) == NULL)
+       return 0;
+
+      munmap (ei->image, ei->size);
+      ei->image = NULL;
+
+      Debug(1, "Found debuglink section, following %s\n", linkbuf);
+
+      p = strrchr (file, '/');
+      if (p != NULL)
+       {
+         memcpy (basedir, file, p - file);
+         basedir[p - file] = '\0';
+       }
+      else
+       basedir[0] = 0;
+
+      strcpy (newname, basedir);
+      strcat (newname, "/");
+      strcat (newname, linkbuf);
+      ret = elf_w (load_debuglink) (newname, ei, -1);
+
+      if (ret == -1)
+       {
+         strcpy (newname, basedir);
+         strcat (newname, "/.debug/");
+         strcat (newname, linkbuf);
+         ret = elf_w (load_debuglink) (newname, ei, -1);
+       }
+
+      if (ret == -1 && is_local == 1)
+       {
+         strcpy (newname, debugdir);
+         strcat (newname, basedir);
+         strcat (newname, "/");
+         strcat (newname, linkbuf);
+         ret = elf_w (load_debuglink) (newname, ei, -1);
+       }
+
+      return ret;
+    }
+  }
+
+  return 0;
+}
diff --git a/src/elfxx.h b/src/elfxx.h
index aa08f9e..830432c 100644
--- a/src/elfxx.h
+++ b/src/elfxx.h
@@ -55,6 +55,7 @@ extern int elf_w (get_proc_name_in_image) (unw_addr_space_t 
as,
                                            char *buf, size_t buf_len, 
unw_word_t *offp);
 
 extern Elf_W (Shdr)* elf_w (find_section) (struct elf_image *ei, const char* 
secname);
+extern int elf_w (load_debuglink) (const char* file, struct elf_image *ei, int 
is_local);
 
 static inline int
 elf_w (valid_object) (struct elf_image *ei)
diff --git a/src/os-linux.c b/src/os-linux.c
index dc861a8..25fe52a 100644
--- a/src/os-linux.c
+++ b/src/os-linux.c
@@ -37,7 +37,6 @@ tdep_get_elf_image (struct elf_image *ei, pid_t pid, 
unw_word_t ip,
   struct map_iterator mi;
   int found = 0, rc;
   unsigned long hi;
-  char debug_path[PATH_MAX];
 
   if (maps_init (&mi, pid) < 0)
     return -1;
@@ -58,12 +57,7 @@ tdep_get_elf_image (struct elf_image *ei, pid_t pid, 
unw_word_t ip,
     {
       strncpy(path, mi.path, pathlen);
     }
-  snprintf (debug_path, sizeof (debug_path), "/usr/lib/debug%s", mi.path);
-  rc = elf_map_image (ei, debug_path);
-  if (rc != 0)
-    {
-      rc = elf_map_image (ei, mi.path);
-    }
+  rc = elf_map_image (ei, mi.path);
   maps_close (&mi);
   return rc;
 }
-- 
2.8.0-rc2