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>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 2 Apr 2013 12:22:48 -0400
- Subject: Re: [patch 6/6] gdbserver build-id attribute generator
- 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> <5154ADD2 dot 9040206 at qnx dot com> <20130331174322 dot GB21374 at host2 dot jankratochvil dot net>
Attached patch addresses your comments. See inline for details.
On 13-03-31 01:43 PM, Jan Kratochvil wrote:
On Thu, 28 Mar 2013 21:53:38 +0100, Aleksandar Ristovski wrote:
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.
I wrote at "find_phdr_p":
But I do not understand why this function exists
which probably corresponds to this your comment.
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.
Oops, you are right:
7ffff7ddc000-7ffff7dfd000 r-xp 00000000 fd:01 51415762 /usr/lib64/ld-2.17.so
7ffff7ffc000-7ffff7ffe000 rw-p 00020000 fd:01 51415762 /usr/lib64/ld-2.17.so
(gdb) p/x 0x7ffff7ffc000-0x7ffff7ddc000
$1 = 0x220000
It is mapped by additional displacement of 2MB. Adjustment is suggested below.
BTW for gdbserver compatibility I have posted:
[patch 1/2+7.6] /proc/PID/smaps: filename fix
http://sourceware.org/ml/gdb-patches/2013-03/msg01120.html
[patch 2/2+7.6] /proc/PID/smaps: Linux kernel 3.8.3 compat.
http://sourceware.org/ml/gdb-patches/2013-03/msg01119.html
but you probably did not face this compat. problem.
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.
Typo: NT_GNU_BUILD_ID
[AR]
Done.
@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;
}
+
Extra empty line, there should be only one empty line.
[AR] Ok.
+/* 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);
GNU Coding Standards formatting:
typedef int (*find_phdr_p_ftype) (const void *phdr, int is_elf64,
GDB uses *_ftype types without that * pointer,
use then find_phdr_p_ftype *find_phdr_p.
[AR] Ok.
+
+/* Linearly traverse pheaders given in PHDR until supplied
program headers given between PHDR_BEGIN and PHDR_END
+ predicate function returns 1. If supplied predicate function
+ did return 1, stop traversal and return that PHDR. */
that program header.
+
+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,
Use find_phdr_p_ftype *find_phdr_p.
+ const void *const data)
+{
+#define SIZEOFHDR(hdr) (is_elf64? sizeof((hdr)._64) : sizeof((hdr)._32))
GNU Coding Standards formatting:
#define SIZEOFHDR(hdr) (is_elf64 ? sizeof ((hdr)._64) : sizeof ((hdr)._32))
But in fact I do not see why you define a macro which is used only once.
[AR]: For readability, so the generic part of the code does not mention
"64" or "32".
+#define PHDR_NEXT(hdrp) ((void *) ((char *)(hdrp) + SIZEOFHDR(*hdrp)))
GNU Coding Standards formatting and also parentheses around each macro
parameter:
#define PHDR_NEXT(hdrp) ((void *) ((char *) (hdrp) + SIZEOFHDR (*hdrp)))
But as GDB codebase allows void * arithmetics it can be simplified to (also
putting there const otherwise it deconstifies the passed in pointer):
#define PHDR_NEXT(hdrp) (((const void *) (hdrp) + SIZEOFHDR (*hdrp)))
[AR]: void* arithmetic is not defined even if gcc allows it. Changed to
casting to const gdb_byte for the arithmetic part.
+
+ 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
+}
+
Missing function comment.
[AR] Ok.
+
+static int
+find_phdr_p (const void *const phdr, const int is_elf64,
+ const void *const data)
But I do not understand why this function exists - it could be integrated in
find_phdr as very every caller of find_phdr passse as find_phdr_p parameter
this find_phdr_p implementation.
[AR] Yes, but I am eyeing solib-svr4.c loops over pheaders
generalization - find_phdr could be moved to 'common' and possibly
called 'iterate_phdrs' with the ability to pass in any function, not
necessarily a predicate only. E.g. svr4_exec_displacement, etc...)
+{
+ 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
+{
Here should be the line:
/* Fields are populated from linux_find_memory_region parameters. */
[AR] ok.
+ 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. */
It is NT_GNU_BUILD_ID note from PT_NOTE segment.
.note.gnu.build-id is section name, PT_NOTE is segment name. Those do not
match.
[AR] ok.
+
+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))
These macros are already defined above, use only one definition. It would be
appropriate to make their name slightly longer in such case, not required.
[AR]. Moved unions and defines up, removed "undef". Changed naming to be
slightly less prone to collisions and clearer.
+ 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)
phdr_buf is void * and GDB codebase allows void * arithmetics so the cast
could be removed.
[AR] I really dislike using void* even if gdb uses it. It's not correct.
find_phdr_p probably should not be passed, as discussed above.
[AR] I can remove it, but the idea is to make generic phdr iterator
similar to 'maps' parser.
Assignment needs to be on a separate line according to GNU Coding Standards,
therefore:
for (;;)
{
phdr = find_phdr (p->is_elf64, phdr,
(phdr_buf
+ HDR (ehdr, e_phnum) * HDR (ehdr, e_phentsize)),
find_phdr_p, &p_type);
if (phdr == NULL)
break;
+ {
+ void *const pt_note = xmalloc (HDR (*phdr, p_memsz));
+ const void *const pt_end
+ = (char*) pt_note + HDR (*phdr, p_memsz);
When it does not fit on a single line use separate declaration line:
const void *pt_end;
pt_end = (gdb_byte *) pt_note + HDR (*phdr, p_memsz);
And also you use 'char' for byte but GDB - even with recent Pedro's changes
- prefers to use gdb_byte in such case. It is not a printable character.
+
+ 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);
Again it is more readable if split, also please keep the order as present in
the file:
const size_t note_sz;
note_sz = (SIZEOFHDR (*nhdr) + HDR (*nhdr, n_namesz)
+ HDR (*nhdr, n_descsz));
But there is also missing alignment to 4 bytes of both n_namesz and n_descsz:
Padding is present, if necessary, to ensure 4-byte alignment for the
descriptor. Such padding is not included in namesz.
[AR] Ok, code re-arranged; sizes rounded up to appropriate size.
+
Padding is present, if necessary, to ensure 4-byte alignment for the
next note entry. Such padding is not included in descsz.
+
+ if (((char *) nhdr + note_sz) > (char *) pt_end)
gdb_byte *
And I asked for checking NOTE_SZ == 0 but you still do not check it. If there
will be NOTE_SZ == 0 then bin2hex below will use the code path:
/* May use a length, or a nul-terminated string as input. */
if (count == 0)
count = strlen ((char *) bin);
which may in a worst case even crash GDB on invalid file running out through
non-zero bytes out of mapped memory.
[AR] It is now being checked.
+ {
+ warning ("Malformed PT_NOTE\n");
+ break;
+ }
You need to check here also the name content, it is "GNU\x00" (n_namesz == 4).
[AR], can it be any other name if type is NT_GNU_BUILD_ID? Added the
check but seems like an overkill to me.
+ 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);
I wrote last time:
GNU Coding Standard formatting:
bin2hex ((gdb_byte *) nhdr, bil->hex_build_id, note_sz);
But this is not the build-id.
readelf -n
Build ID: 1dfca42f0dac568cf81fedc2a9a37a98789a03e4
vs. gdbserver:
<library name="/lib64/ld-linux-x86-64.so.2" lm="0x7ffff7ffd998" l_addr="0x7ffff7ddc000" l_ld="0x7ffff7ffcdf0" build-id="040000001400000003000000474e55001dfca42f0dac568cf81fedc2a9a37a98789a03e4"/>
You need to send only the real build-id bytes, omitting the note header and the
note name ("GNU\0").
Then it will not match GDB itself, it also needs to be updated to process only
the build-id bytes, not the header.
[AR] Ok.
+ xfree (pt_note);
+ return;
+ }
+ nhdr = (void*) ((char *) nhdr + note_sz);
I wrote last time:
GNU Coding Standard formatting + simplification:
nhdr = (void *) nhdr + note_sz;
[AR] I re-arranged the code. Void * arithmetic is not right, so I'm
avoiding it.
+ }
+ xfree (pt_note);
+ }
+ }
+ else
+ warning ("Reading build-id failed.");
I would omit these warnings. But otherwise please put there some better
identifiers, such as vaddr and/or filename. Seeing just many such errors is
not too useful as I have found during the debugging today.
[AR] Removed the warning.
+#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;
I had here this layout:
7ffff7ddc000-7ffff7dfd000 r-xp 00000000 fd:01 51415762 /usr/lib64/ld-2.17.so
7ffff7ff9000-7ffff7ffa000 rw-p 00000000 00:00 0
7ffff7ffa000-7ffff7ffc000 r-xp 00000000 00:00 0 [vdso]
7ffff7ffc000-7ffff7ffe000 rw-p 00020000 fd:01 51415762 /usr/lib64/ld-2.17.so
so it should rather be:
for (p = rbegin - 1; p >= rend; --p)
if (p->offset == 0 && p->inode == rbegin->inode)
return p;
Fortunately it should not have any real performance impact.
[AR] Ok, though we are not adding mappings with inode == 0. See
'find_memory_region_callback'. I changed it anyway, seems a bit more
robust at the expense of rather unlikely event where mapping with offset
0 does not exist.
+
+ 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;
This variable declaration should be moved to the more inner block below.
+ mapping_entry_s *const bil_min
+ = lrfind_mapping_entry (bil,
+ VEC_address (mapping_entry_s, data->list));
When it does not fit the line make the declaration separate, such as:
mapping_entry_s *const bil_min;
bil_min = lrfind_mapping_entry (bil, VEC_address (mapping_entry_s,
data->list));
[AR] Not sure what is the advantage, but ok.
There should be an empty line between declarations and code statements.
+ 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.");
build-id
The name of the feature is "build-id" so it always should be presented this
way to the user. Variable names are not interesting to users.
[AR] Ok. Also, please note the change: I assign BUILD_ID_INVALID here
rather than "" so that we can determine this at document creation time.
I'd rather not emit build-id attribute at all than emitting empty
build-id when it could not be fetched.
+ }
+ }
+
+ 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 (
There should not be opening paren at the end.
[AR] Ok.
+ lwpid_of (get_thread_lwp (current_inferior)),
When the parameters are too long for proper indentation use a temporary
variable for the value.
[AR] Ok, used 'pid' which had been there already.
+ 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
Thanks,
Jan
Thanks,
Aleksandar
>From 799c0ca7884b2f3761762928e23ff755fe7f41b0 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.
(ElfXX_Ehdr, ElfXX_Phdr, ElfXX_Nhdr): New.
(ELFXX_FLD, ELFXX_SIZEOF, ELFXX_ROUNDUP, BUILD_ID_INVALID): New.
(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 | 409 +++++++++++++++++++++++++++++++++---
3 files changed, 402 insertions(+), 37 deletions(-)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3b63d01..25d8eea 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40329,6 +40329,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_BUILD_ID} note, if it exists.
@end itemize
Additionally the single @code{main-lm} attribute specifies address of
@@ -40346,7 +40348,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
@@ -40355,13 +40357,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..61da37c 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
@@ -118,6 +120,33 @@ typedef struct
} Elf64_auxv_t;
#endif
+typedef union ElfXX_Ehdr
+{
+ Elf32_Ehdr _32;
+ Elf64_Ehdr _64;
+} ElfXX_Ehdr;
+
+typedef union ElfXX_Phdr
+{
+ Elf32_Phdr _32;
+ Elf64_Phdr _64;
+} ElfXX_Phdr;
+
+typedef union ElfXX_Nhdr
+{
+ Elf32_Nhdr _32;
+ Elf64_Nhdr _64;
+} ElfXX_Nhdr;
+
+#define ELFXX_FLD(hdr, fld) ((is_elf64) ? (hdr)._64.fld : (hdr)._32.fld)
+#define ELFXX_SIZEOF(hdr) ((is_elf64) ? sizeof ((hdr)._64) \
+ : sizeof ((hdr)._32))
+#define ELFXX_ROUNDUP(what) ((is_elf64) ? (((what) + sizeof (Elf64_Word) - 1) \
+ & ~(sizeof (Elf64_Word) - 1)) \
+ : (((what) + sizeof (Elf32_Word) - 1) \
+ & ~(sizeof (Elf32_Word) - 1)))
+#define BUILD_ID_INVALID "?"
+
/* ``all_threads'' is keyed by the LWP ID, which we use as the GDB protocol
representation of the thread ID.
@@ -5432,15 +5461,76 @@ 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 *const find_phdr_p,
+ const void *const data)
+{
+#define PHDR_NEXT(hdrp) ((const void *) ((const gdb_byte *) (hdrp) + \
+ ELFXX_SIZEOF (*hdrp)))
+
+ const ElfXX_Phdr *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
+}
+
+/* Predicate function for find_phdr iteration. */
+
+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 +5544,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 +5578,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 +5736,254 @@ struct link_map_offsets
int l_prev_offset;
};
+
+/* Structure for holding a mapping. Only mapping
+ containing l_ld can have hex_build_id set. */
+
+struct mapping_entry
+{
+ /* Fields are populated from linux_find_memory_region parameters. */
+
+ 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 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)
+{
+ const int is_elf64 = p->is_elf64;
+ ElfXX_Ehdr ehdr;
+
+ if (linux_read_memory (load_addr, (unsigned char *) &ehdr,
+ ELFXX_SIZEOF (ehdr)) == 0
+ && ELFXX_FLD (ehdr, e_ident[EI_MAG0]) == ELFMAG0
+ && ELFXX_FLD (ehdr, e_ident[EI_MAG1]) == ELFMAG1
+ && ELFXX_FLD (ehdr, e_ident[EI_MAG2]) == ELFMAG2
+ && ELFXX_FLD (ehdr, e_ident[EI_MAG3]) == ELFMAG3)
+ {
+ const ElfXX_Phdr *phdr;
+ void *phdr_buf;
+ const ULONGEST p_type = PT_NOTE;
+ const unsigned e_phentsize = ELFXX_FLD (ehdr, e_phentsize);
+
+ gdb_assert (ELFXX_FLD (ehdr, e_phnum) < 100); /* Basic sanity check. */
+ gdb_assert (e_phentsize == ELFXX_SIZEOF (*phdr));
+ phdr_buf = alloca (ELFXX_FLD (ehdr, e_phnum) * e_phentsize);
+
+ if (linux_read_memory (load_addr + ELFXX_FLD (ehdr, e_phoff), phdr_buf,
+ ELFXX_FLD (ehdr, e_phnum) * e_phentsize) != 0)
+ {
+ warning ("Could not read program header.");
+ return;
+ }
+
+ phdr = phdr_buf;
+
+ for (;;)
+ {
+ gdb_byte *pt_note;
+ const gdb_byte *pt_end;
+ const ElfXX_Nhdr *nhdr;
+
+ phdr = find_phdr (p->is_elf64, phdr, (gdb_byte *) phdr_buf
+ + ELFXX_FLD (ehdr, e_phnum) * e_phentsize,
+ find_phdr_p, &p_type);
+ if (phdr == NULL)
+ break;
+ pt_note = xmalloc (ELFXX_FLD (*phdr, p_memsz));
+ pt_end = (gdb_byte*) pt_note + ELFXX_FLD (*phdr, p_memsz);
+
+ if (linux_read_memory (ELFXX_FLD (*phdr, p_vaddr) + l_addr, pt_note,
+ ELFXX_FLD (*phdr, p_memsz)) != 0)
+ {
+ xfree (pt_note);
+ warning ("Could not read note.");
+ break;
+ }
+
+ nhdr = (void *) pt_note;
+ while ((void *) nhdr < (void *) pt_end)
+ {
+ const size_t namesz
+ = ELFXX_ROUNDUP (ELFXX_FLD (*nhdr, n_namesz));
+ const size_t descsz
+ = ELFXX_ROUNDUP (ELFXX_FLD (*nhdr, n_descsz));
+ const size_t note_sz = ELFXX_SIZEOF (*nhdr) + namesz + descsz;
+
+ if (((gdb_byte *) nhdr + note_sz) > pt_end || note_sz == 0
+ || descsz == 0)
+ {
+ warning ("Malformed PT_NOTE\n");
+ break;
+ }
+ if (ELFXX_FLD (*nhdr, n_type) == NT_GNU_BUILD_ID
+ && ELFXX_FLD (*nhdr, n_namesz) == 4)
+ {
+ const char gnu[4] = "GNU\0";
+ const char *const pname
+ = (char *) nhdr + ELFXX_SIZEOF (*nhdr);
+
+ if (memcmp (pname, gnu, 4) == 0)
+ {
+ const size_t n_descsz = ELFXX_FLD (*nhdr, n_descsz);
+
+ bil->hex_build_id = xmalloc (n_descsz * 2 + 1);
+ bin2hex ((gdb_byte*) pname + namesz, bil->hex_build_id,
+ n_descsz);
+ xfree (pt_note);
+ return;
+ }
+ }
+ nhdr = (void*) ((gdb_byte *) nhdr + note_sz);
+ }
+ xfree (pt_note);
+ }
+ }
+}
+
+/* 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)
+ if (p->offset == 0 && p->inode == rbegin->inode)
+ 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)
+ {
+ mapping_entry_s *bil_min;
+
+ bil_min = lrfind_mapping_entry (bil, VEC_address (mapping_entry_s,
+ data->list));
+ if (bil_min != NULL)
+ read_build_id (data, bil, bil_min->vaddr, l_addr);
+ else
+ {
+ /* Do not try to find hex_build_id again. */
+ bil->hex_build_id = xstrdup (BUILD_ID_INVALID);
+ 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 +5996,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 +6032,13 @@ 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 (pid, 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 +6113,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 +6122,29 @@ 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
+ && strcmp (hex_enc_build_id, BUILD_ID_INVALID) != 0)
+ p += sprintf (p, " build-id=\"%s\"", hex_enc_build_id);
+ p += sprintf(p, "/>");
free (name);
}
else if (lm_prev == 0)
@@ -5819,6 +6179,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