This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 6/6] gdbserver build-id attribute generator
- From: Aleksandar Ristovski <aristovski at qnx dot com>
- Cc: Jan Kratochvil <jan dot kratochvil at redhat dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Thu, 28 Mar 2013 16:53:38 -0400
- Subject: Re: [patch 6/6] gdbserver build-id attribute generator
- Newsgroups: gmane.comp.gdb.patches
- References: <51278984 dot 3070208 at qnx dot com> <20130310210843 dot GG21130 at host2 dot jankratochvil dot net> <514C56D4 dot 1060906 at qnx dot com> <20130326204157 dot GC12291 at host2 dot jankratochvil dot net> <51530465 dot 30503 at qnx dot com> <20130327145028 dot GA17905 at host2 dot jankratochvil dot net> <515353CF dot 40601 at qnx dot com>
Fixed patch. I haven't addressed any of your concerns except fixed what
was broken. There are two things changed:
1) get_dynamic, you will see I left it unfinished when switched to
generic "find_phdr" to address phdr traversal duplication code.
2) lrfind_mapping_entry can not check for vaddr + offset as offset is
file offset, and for some shared objects this will not match even though
the vaddr of the entry with zero offset is valid.
Stepping through code now shows some of the things you couldn't see,
like e.g. why is there so->build_id, and where is it being set (you
couldn't see it being set before as qXfer_library was broken).
---
Aleksandar
On 13-03-27 04:17 PM, Aleksandar Ristovski wrote:
Addressed Jan's comments.
On 13-03-27 10:50 AM, Jan Kratochvil wrote:
On Wed, 27 Mar 2013 15:38:29 +0100, Aleksandar Ristovski wrote:
On 13-03-26 04:41 PM, Jan Kratochvil wrote:
+ if (build_id_list_p)
+ qsort (VEC_address (build_id_list_s, data.list),
+ VEC_length (build_id_list_s, data.list),
+ sizeof (build_id_list_s), compare_build_id_list);
It is always already sorted by Linux kernel, rather a for cycle to
verify it
really is sorted.
Can we guarantee this is always the case?
Yes.
The problem is that if it is unsorted there is a bug somewhere and
that qsort
will hide that bug.
Qsort removed. I didn't put any traversal; we are making assumption that
the list will be sorted. The checks in the other bits make sure that we
either find the right mapping or none at all, so worst case scenario is
we don't get build-id communicated to gdb.
Thanks,
Aleksandar
>From 80cd24335bcff6625b5c69c1b2f2d43142db08d1 Mon Sep 17 00:00:00 2001
From: Aleksandar Ristovski <ARistovski@qnx.com>
Date: Wed, 27 Mar 2013 11:56:57 -0400
Subject: [PATCH 6/8] gdbserver build-id attribute generator
* doc/gdb.texinfo (Library List Format for SVR4 Targets): Add
'build-id' in description, example, new attribute in dtd.
* features/library-list-svr4.dtd (library-list-svr4): New
'build-id' attribute.
* linux-low.c (linux-maps.h, search.h): Include.
(find_phdr_p_ftype, find_phdr, find_phdr_p): New.
(get_dynamic): Use find_pdhr to traverse program headers.
(struct mapping_entry): New structure.
(mapping_entry_s): New typedef, new vector type def.
(free_mapping_entry, compare_mapping_entry,
compare_mapping_entry_range, compare_mapping_entry_inode): New.
(struct find_memory_region_callback_data): New.
(find_memory_region_callback): New fwd. declaration.
(read_build_id, find_memory_region_callback, get_hex_build_id): New.
(linux_qxfer_libraries_svr4): Add optional build-id attribute
to reply XML document.
---
gdb/doc/gdb.texinfo | 17 +-
gdb/features/library-list-svr4.dtd | 13 +-
gdb/gdbserver/linux-low.c | 388 +++++++++++++++++++++++++++++++++---
3 files changed, 381 insertions(+), 37 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 38ce259..7c17209 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40323,6 +40323,8 @@ memory address. It is a displacement of absolute memory address against
address the file was prelinked to during the library load.
@item
@code{l_ld}, which is memory address of the @code{PT_DYNAMIC} segment
+@item
+@code{build-id}, hex encoded @code{NT_GNU_BUID_ID} note, if it exists.
@end itemize
Additionally the single @code{main-lm} attribute specifies address of
@@ -40340,7 +40342,7 @@ looks like this:
<library name="/lib/ld-linux.so.2" lm="0xe4f51c" l_addr="0xe2d000"
l_ld="0xe4eefc"/>
<library name="/lib/libc.so.6" lm="0xe4fbe8" l_addr="0x154000"
- l_ld="0x152350"/>
+ l_ld="0x152350" build-id="9afccf7cc41e6293476223fe72480854"/>
</library-list-svr>
@end smallexample
@@ -40349,13 +40351,14 @@ The format of an SVR4 library list is described by this DTD:
@smallexample
<!-- library-list-svr4: Root element with versioning -->
<!ELEMENT library-list-svr4 (library)*>
-<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
-<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
+<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
+<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
<!ELEMENT library EMPTY>
-<!ATTLIST library name CDATA #REQUIRED>
-<!ATTLIST library lm CDATA #REQUIRED>
-<!ATTLIST library l_addr CDATA #REQUIRED>
-<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library name CDATA #REQUIRED>
+<!ATTLIST library lm CDATA #REQUIRED>
+<!ATTLIST library l_addr CDATA #REQUIRED>
+<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library build-id CDATA #IMPLIED>
@end smallexample
@node Memory Map Format
diff --git a/gdb/features/library-list-svr4.dtd b/gdb/features/library-list-svr4.dtd
index cae7fd8..fdd6ec0 100644
--- a/gdb/features/library-list-svr4.dtd
+++ b/gdb/features/library-list-svr4.dtd
@@ -6,11 +6,12 @@
<!-- library-list-svr4: Root element with versioning -->
<!ELEMENT library-list-svr4 (library)*>
-<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
-<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
+<!ATTLIST library-list-svr4 version CDATA #FIXED "1.0">
+<!ATTLIST library-list-svr4 main-lm CDATA #IMPLIED>
<!ELEMENT library EMPTY>
-<!ATTLIST library name CDATA #REQUIRED>
-<!ATTLIST library lm CDATA #REQUIRED>
-<!ATTLIST library l_addr CDATA #REQUIRED>
-<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library name CDATA #REQUIRED>
+<!ATTLIST library lm CDATA #REQUIRED>
+<!ATTLIST library l_addr CDATA #REQUIRED>
+<!ATTLIST library l_ld CDATA #REQUIRED>
+<!ATTLIST library build-id CDATA #IMPLIED>
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 72c51e0..aa248e9 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -20,6 +20,7 @@
#include "linux-low.h"
#include "linux-osdata.h"
#include "agent.h"
+#include "linux-maps.h"
#include "gdb_wait.h"
#include <stdio.h>
@@ -43,6 +44,7 @@
#include "gdb_stat.h"
#include <sys/vfs.h>
#include <sys/uio.h>
+#include <search.h>
#ifndef ELFMAG0
/* Don't include <linux/elf.h> here. If it got included by gdb_proc_service.h
then ELFMAG0 will have been defined. If it didn't get included by
@@ -5432,15 +5434,81 @@ get_phdr_phnum_from_proc_auxv (const int pid, const int is_elf64,
return 0;
}
+
+/* Predicate function type returns 1 if the given phdr is what is
+ being looked for. Returns 0 otherwise. */
+
+typedef int (*find_phdr_p_ftype)(const void *phdr, int is_elf64,
+ const void *data);
+
+/* Linearly traverse pheaders given in PHDR until supplied
+ predicate function returns 1. If supplied predicate function
+ did return 1, stop traversal and return that PHDR. */
+
+static const void *
+find_phdr (int is_elf64, const void *const phdr_begin,
+ const void *const phdr_end, find_phdr_p_ftype find_phdr_p,
+ const void *const data)
+{
+#define SIZEOFHDR(hdr) (is_elf64? sizeof((hdr)._64) : sizeof((hdr)._32))
+#define PHDR_NEXT(hdrp) ((void *) ((char *)(hdrp) + SIZEOFHDR(*hdrp)))
+
+ union ElfXX_Phdr
+ {
+ Elf32_Phdr _32;
+ Elf64_Phdr _64;
+ } const *phdr = phdr_begin;
+
+ if (phdr == NULL)
+ return NULL;
+
+ while (PHDR_NEXT (phdr) <= phdr_end)
+ {
+ if (find_phdr_p (phdr, is_elf64, data) == 1)
+ return phdr;
+ phdr = PHDR_NEXT (phdr);
+ }
+
+ return NULL;
+#undef PHDR_NEXT
+#undef SIZEOFHDR
+}
+
+
+static int
+find_phdr_p (const void *const phdr, const int is_elf64,
+ const void *const data)
+{
+ const ULONGEST *const type = data;
+
+ if (is_elf64)
+ {
+ const Elf64_Phdr *const p = phdr;
+
+ if (p->p_type == *type)
+ return 1;
+ }
+ else
+ {
+ const Elf32_Phdr *const p = phdr;
+
+ if (p->p_type == *type)
+ return 1;
+ }
+ return 0;
+}
+
/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present. */
static CORE_ADDR
get_dynamic (const int pid, const int is_elf64)
{
CORE_ADDR phdr_memaddr, relocation;
- int num_phdr, i;
+ int num_phdr;
unsigned char *phdr_buf;
const int phdr_size = is_elf64 ? sizeof (Elf64_Phdr) : sizeof (Elf32_Phdr);
+ const void *phdr;
+ ULONGEST p_type;
if (get_phdr_phnum_from_proc_auxv (pid, is_elf64, &phdr_memaddr, &num_phdr))
return 0;
@@ -5454,21 +5522,24 @@ get_dynamic (const int pid, const int is_elf64)
/* Compute relocation: it is expected to be 0 for "regular" executables,
non-zero for PIE ones. */
relocation = -1;
- for (i = 0; relocation == -1 && i < num_phdr; i++)
- if (is_elf64)
- {
- Elf64_Phdr *const p = (Elf64_Phdr *) (phdr_buf + i * phdr_size);
+ p_type = PT_PHDR;
+ phdr = find_phdr (is_elf64, phdr_buf, phdr_buf + num_phdr * phdr_size,
+ find_phdr_p, &p_type);
+ if (phdr != NULL)
+ {
+ if (is_elf64)
+ {
+ const Elf64_Phdr *const p = phdr;
- if (p->p_type == PT_PHDR)
relocation = phdr_memaddr - p->p_vaddr;
- }
- else
- {
- Elf32_Phdr *const p = (Elf32_Phdr *) (phdr_buf + i * phdr_size);
+ }
+ else
+ {
+ const Elf32_Phdr *const p = phdr;
- if (p->p_type == PT_PHDR)
relocation = phdr_memaddr - p->p_vaddr;
- }
+ }
+ }
if (relocation == -1)
{
@@ -5485,21 +5556,23 @@ get_dynamic (const int pid, const int is_elf64)
return 0;
}
- for (i = 0; i < num_phdr; i++)
+ p_type = PT_DYNAMIC;
+ phdr = find_phdr (is_elf64, phdr_buf, phdr_buf + num_phdr * phdr_size,
+ find_phdr_p, &p_type);
+
+ if (phdr != NULL)
{
if (is_elf64)
{
- Elf64_Phdr *const p = (Elf64_Phdr *) (phdr_buf + i * phdr_size);
+ const Elf64_Phdr *const p = phdr;
- if (p->p_type == PT_DYNAMIC)
- return p->p_vaddr + relocation;
+ return p->p_vaddr + relocation;
}
else
{
- Elf32_Phdr *const p = (Elf32_Phdr *) (phdr_buf + i * phdr_size);
+ const Elf32_Phdr *const p = phdr;
- if (p->p_type == PT_DYNAMIC)
- return p->p_vaddr + relocation;
+ return p->p_vaddr + relocation;
}
}
@@ -5641,6 +5714,255 @@ struct link_map_offsets
int l_prev_offset;
};
+
+/* Structure for holding a mapping. Only mapping
+ containing l_ld can have hex_build_id set.
+
+ Fields are populated from linux_find_memory_region parameters. */
+
+struct mapping_entry
+{
+ ULONGEST vaddr;
+ ULONGEST size;
+ ULONGEST offset;
+ ULONGEST inode;
+
+ /* Hex encoded string allocated using xmalloc, and
+ needs to be freed. It can be NULL. */
+
+ char *hex_build_id;
+};
+
+typedef struct mapping_entry mapping_entry_s;
+
+DEF_VEC_O(mapping_entry_s);
+
+static void
+free_mapping_entry (VEC (mapping_entry_s) *lst)
+{
+ int ix;
+ mapping_entry_s *p;
+
+ for (ix = 0; VEC_iterate (mapping_entry_s, lst, ix, p); ++ix)
+ xfree (p->hex_build_id);
+
+ VEC_free (mapping_entry_s, lst);
+}
+
+/* Used for finding a mapping containing the given
+ l_ld passed in K. */
+
+static int
+compare_mapping_entry_range (const void *const k, const void *const b)
+{
+ const ULONGEST key = *(CORE_ADDR*) k;
+ const mapping_entry_s *const p = b;
+
+ if (key < p->vaddr)
+ return -1;
+
+ if (key < p->vaddr + p->size)
+ return 0;
+
+ return 1;
+}
+
+struct find_memory_region_callback_data
+{
+ unsigned is_elf64;
+
+ /* Return. Ordered list of all object mappings sorted in
+ ascending order by VADDR. Must be freed with free_mapping_entry. */
+ VEC (mapping_entry_s) *list;
+};
+
+static linux_find_memory_region_ftype find_memory_region_callback;
+
+/* Read .note.gnu.build-id from PT_NOTE. */
+
+static void
+read_build_id (struct find_memory_region_callback_data *const p,
+ mapping_entry_s *const bil, const CORE_ADDR load_addr,
+ const CORE_ADDR l_addr)
+{
+ union ElfXX_Ehdr
+ {
+ Elf32_Ehdr _32;
+ Elf64_Ehdr _64;
+ } ehdr;
+ union ElfXX_Phdr
+ {
+ Elf32_Phdr _32;
+ Elf64_Phdr _64;
+ } const *phdr;
+ union ElfXX_Nhdr
+ {
+ Elf32_Nhdr _32;
+ Elf64_Nhdr _64;
+ } *nhdr;
+#define HDR(hdr, fld) (((p)->is_elf64)? (hdr)._64.fld : (hdr)._32.fld)
+#define SIZEOFHDR(hdr) (((p)->is_elf64)?sizeof((hdr)._64):sizeof((hdr)._32))
+ if (linux_read_memory (load_addr, (unsigned char *) &ehdr, SIZEOFHDR (ehdr))
+ == 0
+ && HDR (ehdr, e_ident[EI_MAG0]) == ELFMAG0
+ && HDR (ehdr, e_ident[EI_MAG1]) == ELFMAG1
+ && HDR (ehdr, e_ident[EI_MAG2]) == ELFMAG2
+ && HDR (ehdr, e_ident[EI_MAG3]) == ELFMAG3)
+ {
+ void *phdr_buf;
+ const ULONGEST p_type = PT_NOTE;
+
+ gdb_assert (HDR (ehdr, e_phnum) < 100); /* Basic sanity check. */
+ gdb_assert (HDR (ehdr, e_phentsize) == SIZEOFHDR (*phdr));
+ phdr_buf = alloca (HDR (ehdr, e_phnum) * HDR (ehdr, e_phentsize));
+
+ if (linux_read_memory (load_addr + HDR (ehdr, e_phoff), phdr_buf,
+ HDR (ehdr, e_phnum) * HDR (ehdr, e_phentsize))
+ != 0)
+ {
+ warning ("Could not read program header.");
+ return;
+ }
+
+ phdr = phdr_buf;
+
+ while ((phdr = find_phdr (p->is_elf64, phdr, (char *) phdr_buf
+ + HDR (ehdr, e_phnum) * HDR (ehdr, e_phentsize),
+ find_phdr_p, &p_type)) != NULL)
+ {
+ void *const pt_note = xmalloc (HDR (*phdr, p_memsz));
+ const void *const pt_end
+ = (char*) pt_note + HDR (*phdr, p_memsz);
+
+ if (linux_read_memory (HDR (*phdr, p_vaddr) + l_addr,
+ pt_note, HDR (*phdr, p_memsz)) != 0)
+ {
+ xfree (pt_note);
+ warning ("Could not read note.");
+ break;
+ }
+
+ nhdr = pt_note;
+ while ((void *) nhdr < pt_end)
+ {
+ const size_t note_sz
+ = HDR (*nhdr, n_namesz) + HDR (*nhdr, n_descsz)
+ + SIZEOFHDR (*nhdr);
+
+ if (((char *) nhdr + note_sz) > (char *) pt_end)
+ {
+ warning ("Malformed PT_NOTE\n");
+ break;
+ }
+ if (HDR (*nhdr, n_type) == NT_GNU_BUILD_ID)
+ {
+ bil->hex_build_id = xmalloc (note_sz * 2 + 1);
+ bin2hex ((gdb_byte*) nhdr, bil->hex_build_id, note_sz);
+ xfree (pt_note);
+ return;
+ }
+ nhdr = (void*) ((char *) nhdr + note_sz);
+ }
+ xfree (pt_note);
+ }
+ }
+ else
+ warning ("Reading build-id failed.");
+#undef HDR
+#undef SIZEOFHDR
+}
+
+
+/* Add mapping_entry. */
+
+static int
+find_memory_region_callback (ULONGEST vaddr, ULONGEST size, ULONGEST offset,
+ ULONGEST inode, int read, int write, int exec,
+ int modified, const char *filename, void *data)
+{
+ if (inode != 0)
+ {
+ struct find_memory_region_callback_data *const p = data;
+ mapping_entry_s bil;
+
+ bil.vaddr = vaddr;
+ bil.size = size;
+ bil.offset = offset;
+ bil.inode = inode;
+ bil.hex_build_id = NULL;
+
+ VEC_safe_push (mapping_entry_s, p->list, &bil);
+ }
+
+ /* Continue the traversal. */
+ return 0;
+}
+
+/* Linear reverse find starting from RBEGIN towards REND looking for
+ the lowest vaddr mapping of the same inode and zero offset. */
+
+static mapping_entry_s *
+lrfind_mapping_entry (mapping_entry_s *const rbegin,
+ const mapping_entry_s *const rend)
+{
+ mapping_entry_s *p;
+
+ for (p = rbegin - 1; p >= rend && p->inode == rbegin->inode; --p)
+ if (p->offset == 0)
+ return p;
+
+ return NULL;
+}
+
+/* Get build-id for the given L_LD. DATA must point to
+ already filled list of mapping_entry elements.
+
+ Return build_id as stored in the list element corresponding
+ to L_LD.
+
+ NULL may be returned if build-id could not be fetched.
+
+ Returned string must not be freed explicitly. */
+
+static const char *
+get_hex_build_id (const CORE_ADDR l_addr, const CORE_ADDR l_ld,
+ struct find_memory_region_callback_data *const data)
+{
+ mapping_entry_s *bil;
+
+ if (VEC_address (mapping_entry_s, data->list) == NULL)
+ return NULL;
+
+ bil = bsearch (&l_ld, VEC_address (mapping_entry_s, data->list),
+ VEC_length (mapping_entry_s, data->list),
+ sizeof (mapping_entry_s), compare_mapping_entry_range);
+
+ if (bil == NULL)
+ return NULL;
+
+ if (bil->hex_build_id == NULL)
+ {
+ CORE_ADDR load_addr;
+ mapping_entry_s *const bil_min
+ = lrfind_mapping_entry (bil,
+ VEC_address (mapping_entry_s, data->list));
+ if (bil_min != NULL)
+ {
+ load_addr = bil_min->vaddr;
+ read_build_id (data, bil, load_addr, l_addr);
+ }
+ else
+ {
+ /* Do not try to find hex_build_id again. */
+ bil->hex_build_id = xstrdup ("");
+ warning ("Could not determine load address; "
+ "build_id can not be used.");
+ }
+ }
+
+ return bil->hex_build_id;
+}
+
/* Construct qXfer:libraries-svr4:read reply. */
static int
@@ -5653,6 +5975,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
struct process_info_private *const priv = current_process ()->private;
char filename[PATH_MAX];
int pid, is_elf64;
+ struct find_memory_region_callback_data data;
static const struct link_map_offsets lmo_32bit_offsets =
{
@@ -5688,6 +6011,14 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
is_elf64 = elf_64_file_p (filename, &machine);
lmo = is_elf64 ? &lmo_64bit_offsets : &lmo_32bit_offsets;
+ data.is_elf64 = is_elf64;
+ data.list = NULL;
+ VEC_reserve (mapping_entry_s, data.list, 16);
+ if (linux_find_memory_regions_full (
+ lwpid_of (get_thread_lwp (current_inferior)),
+ find_memory_region_callback, &data, NULL) < 0)
+ warning ("Finding memory regions failed");
+
if (priv->r_debug == 0)
priv->r_debug = get_r_debug (pid, is_elf64);
@@ -5762,6 +6093,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
/* 6x the size for xml_escape_text below. */
size_t len = 6 * strlen ((char *) libname);
char *name;
+ const char *hex_enc_build_id = NULL;
if (!header_done)
{
@@ -5770,21 +6102,28 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
header_done = 1;
}
- while (allocated < p - document + len + 200)
+ name = xml_escape_text ((char *) libname);
+ hex_enc_build_id = get_hex_build_id (l_addr, l_ld, &data);
+
+ while (allocated < (p - document + len + 200
+ + (hex_enc_build_id != NULL
+ ? strlen (hex_enc_build_id) : 0)))
{
/* Expand to guarantee sufficient storage. */
- uintptr_t document_len = p - document;
+ const ptrdiff_t document_len = p - document;
- document = xrealloc (document, 2 * allocated);
allocated *= 2;
+ document = xrealloc (document, allocated);
p = document + document_len;
}
- name = xml_escape_text ((char *) libname);
p += sprintf (p, "<library name=\"%s\" lm=\"0x%lx\" "
- "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
+ "l_addr=\"0x%lx\" l_ld=\"0x%lx\"",
name, (unsigned long) lm_addr,
(unsigned long) l_addr, (unsigned long) l_ld);
+ if (hex_enc_build_id != NULL)
+ p += sprintf (p, " build-id=\"%s\"", hex_enc_build_id);
+ p += sprintf(p, "/>");
free (name);
}
else if (lm_prev == 0)
@@ -5819,6 +6158,7 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
memcpy (readbuf, document + offset, len);
xfree (document);
+ free_mapping_entry (data.list);
return len;
}
--
1.7.10.4