From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2527 invoked by alias); 28 Mar 2013 20:53:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2513 invoked by uid 89); 28 Mar 2013 20:53:47 -0000 X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,MISSING_HEADERS,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_CP autolearn=no version=3.3.1 Received: from na3sys009aog138.obsmtp.com (HELO na3sys009aog138.obsmtp.com) (74.125.149.19) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 28 Mar 2013 20:53:43 +0000 Received: from mx20.qnx.com ([72.1.200.103]) (using TLSv1) by na3sys009aob138.postini.com ([74.125.148.12]) with SMTP ID DSNKUVSt1O/p5iPBYq7V/I0sL0qNNaSp3qq2@postini.com; Thu, 28 Mar 2013 13:53:42 PDT Received: by mx20.qnx.com (Postfix, from userid 500) id 16307210FC; Thu, 28 Mar 2013 16:53:40 -0400 (EDT) Received: from exhts.ott.qnx.com (exch1 [10.222.2.137]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by mx20.qnx.com (Postfix) with ESMTPS id E559820E2D; Thu, 28 Mar 2013 16:53:38 -0400 (EDT) Received: from [10.222.96.215] (10.222.2.5) by EXCH1.ott.qnx.com (10.222.2.137) with Microsoft SMTP Server id 14.2.318.4; Thu, 28 Mar 2013 16:53:38 -0400 Message-ID: <5154ADD2.9040206@qnx.com> Date: Fri, 29 Mar 2013 00:13:00 -0000 From: Aleksandar Ristovski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 MIME-Version: 1.0 Newsgroups: gmane.comp.gdb.patches CC: Jan Kratochvil , "gdb-patches@sourceware.org" Subject: Re: [patch 6/6] gdbserver build-id attribute generator References: <51278984.3070208@qnx.com> <20130310210843.GG21130@host2.jankratochvil.net> <514C56D4.1060906@qnx.com> <20130326204157.GC12291@host2.jankratochvil.net> <51530465.30503@qnx.com> <20130327145028.GA17905@host2.jankratochvil.net> <515353CF.40601@qnx.com> In-Reply-To: <515353CF.40601@qnx.com> Content-Type: multipart/mixed; boundary="------------070307080202030901020402" X-Virus-Found: No X-SW-Source: 2013-03/txt/msg01077.txt.bz2 --------------070307080202030901020402 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1713 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 > --------------070307080202030901020402 Content-Type: text/x-patch; name="0006-gdbserver-build-id-attribute-generator.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0006-gdbserver-build-id-attribute-generator.patch" Content-length: 17863 >From 80cd24335bcff6625b5c69c1b2f2d43142db08d1 Mon Sep 17 00:00:00 2001 From: Aleksandar Ristovski 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: + l_ld="0x152350" build-id="9afccf7cc41e6293476223fe72480854"/> @end smallexample @@ -40349,13 +40351,14 @@ The format of an SVR4 library list is described by this DTD: @smallexample - - + + - - - - + + + + + @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 @@ - - + + - - - - + + + + + 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 @@ -43,6 +44,7 @@ #include "gdb_stat.h" #include #include +#include #ifndef ELFMAG0 /* Don't include 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, "", + "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 --------------070307080202030901020402-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3253 invoked by alias); 28 Mar 2013 20:54:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 3239 invoked by uid 89); 28 Mar 2013 20:54:05 -0000 X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FSL_HELO_BARE_IP_2,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_NUMERIC_HELO,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP autolearn=no version=3.3.1 Received: from plane.gmane.org (HELO plane.gmane.org) (80.91.229.3) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 28 Mar 2013 20:54:01 +0000 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1ULJq0-0005eM-0Z for gdb-patches@sourceware.org; Thu, 28 Mar 2013 21:54:20 +0100 Received: from 209.226.137.106 ([209.226.137.106]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 28 Mar 2013 21:54:20 +0100 Received: from ARistovski by 209.226.137.106 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Thu, 28 Mar 2013 21:54:20 +0100 To: gdb-patches@sourceware.org From: Aleksandar Ristovski Subject: Re: [patch 6/6] gdbserver build-id attribute generator Date: Fri, 29 Mar 2013 00:20:00 -0000 Message-ID: <5154ADD2.9040206@qnx.com> References: <51278984.3070208@qnx.com> <20130310210843.GG21130@host2.jankratochvil.net> <514C56D4.1060906@qnx.com> <20130326204157.GC12291@host2.jankratochvil.net> <51530465.30503@qnx.com> <20130327145028.GA17905@host2.jankratochvil.net> <515353CF.40601@qnx.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------070307080202030901020402" Cc: Jan Kratochvil , "gdb-patches@sourceware.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 In-Reply-To: <515353CF.40601@qnx.com> X-Virus-Found: No X-SW-Source: 2013-03/txt/msg01078.txt.bz2 Message-ID: <20130329002000.IAf0_Zp32SI2NUVhmfjUdndm38Vtxp7rLv26OaGFHPo@z> This is a multi-part message in MIME format. --------------070307080202030901020402 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1713 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 > --------------070307080202030901020402 Content-Type: text/x-patch; name="0006-gdbserver-build-id-attribute-generator.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0006-gdbserver-build-id-attribute-generator.patch" Content-length: 17863 >From 80cd24335bcff6625b5c69c1b2f2d43142db08d1 Mon Sep 17 00:00:00 2001 From: Aleksandar Ristovski 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: + l_ld="0x152350" build-id="9afccf7cc41e6293476223fe72480854"/> @end smallexample @@ -40349,13 +40351,14 @@ The format of an SVR4 library list is described by this DTD: @smallexample - - + + - - - - + + + + + @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 @@ - - + + - - - - + + + + + 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 @@ -43,6 +44,7 @@ #include "gdb_stat.h" #include #include +#include #ifndef ELFMAG0 /* Don't include 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, "", + "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 --------------070307080202030901020402--