[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libunwind-devel] [PATCH V2] elf: Follow .gnu_debuglink when resolvi
From: |
Bert Wesarg |
Subject: |
Re: [Libunwind-devel] [PATCH V2] elf: Follow .gnu_debuglink when resolving function names |
Date: |
Sat, 18 Feb 2017 13:44:22 +0100 |
Dave,
thats looks good. Though not tested yet.
Thanks.
Bert
On Fri, Feb 17, 2017 at 6:37 PM, Dave Watson <address@hidden> wrote:
> 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
>