[PATCH] Core file build-ids
Keith Seitz
keiths@redhat.com
Tue Oct 1 13:08:00 GMT 2019
Hi, Nick,
Wowser, it's been a busy quarter! I'm anxious to get back to this patch.
I appreciate your patience.
On 6/7/19 9:16 AM, Nick Clifton wrote:
>> +void
>> +_bfd_elf_core_find_build_id (bfd *templ, bfd_vma offset)> +{> + return (*get_elf_backend_data (templ)->elf_backend_core_find_build_id)> + (templ, offset);
>
> I would recommend being a little bit paranoid here and checking
> that templ is an elf format bfd before attempting to access the
> backend data.
Done.
>> - return _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "load");
>> + if (! _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "load"))
>> + return FALSE;
>> + if (bfd_get_format (abfd) == bfd_core && abfd->build_id == NULL)
>> + _bfd_elf_core_find_build_id (abfd, hdr->p_offset);
>> + return TRUE;
>
> If _bfd_elf_core_find_build_id can fail (see above and below) then
> it would be worth checking the return value for an error result.
My concern here is what happens if we fail to find a build-id in the
core file (I/O error jumping through a header or it's just plain missing).
If _bfd_elf_core_find_build_id returns FALSE and we bail here, this will
abort constructing any more phdrs.
After implementing your recommendation, I can actually demonstrate
this scenario with one of gdb's tests: gdb.arch/i386-biarch-core.exp. This
test will FAIL with a memory read error while attempting to print the
contents of some memory location in the core file.
This location exists in a section that is internalized after we attempt
to find a (missing) build-id.
That's why I made this an optional (as in, we don't care if it fails)
step when constructing the section.
Maybe I've misunderstood what you were really asking me to do?
>> +/* Attempt to find a build-id in a core file from the core file BFD.
>> + OFFSET is the file offset to a PT_LOAD segment that may contain
>> + the build-id note. */
>> +
>> +void
>> +NAME(_bfd_elf,core_find_build_id)
>
> Given that things can wrong when attempting to find a build_id, I think
> that this function should return a bfd_boolean. I appreciate that the
> caller could just check abfd->build_id != NULL, (assuming that it was NULL
> to start with), but I think that it is cleaner that the function itself
> should tell its caller whether ot not it succeeded.
I've made this change in this version, but at the one place it is called,
the return value is ignored (see above). Nonetheless, I could imagine a
use for this outside of my/gdb's specific use case, and I think returning
bfd_boolean is a better/safer/more-forward-thinking option, so I've left
it (updated per your request) in this version.
>> + Elf_External_Ehdr x_ehdr; /* Elf file header, external form */
>> + Elf_Internal_Ehdr i_ehdr; /* Elf file header, internal form */
>
> Minor formatting nit: comments should end with a full stop and two spaces.
Fixed.
New version below.
Keith
Core file build-ids
This patch adds support for reading build-ids from core files. I've
opted to maintain the current interfaces that BFD and GDB already use.
The approach taken is fairly straightforward (YMMV). When a core file
is opened, BFD loops over the various segments in the ELF file and
program headers. During this time, whenever a segment whose type is
PT_LOAD is encountered, and the build-id is unknown, I've added a call
to inspect the segment for an ELF header. If a valid ELF file header
is found, it attempts to read in the program headers and scan for any
notes.
For those unfamiliar with build-ids, please see
https://fedoraproject.org/wiki/Releases/FeatureBuildId .
Yes, this feature has been in Fedora GDB for over a decade, but it exists
as a copy-n-paste of a lot of BFD's internals. For the brave, you can
read the original submission at
https://sourceware.org/ml/gdb-patches/2010-11/msg00353.html .
This is an attempt to clean that up.
I have run this patch through GDB's buildbot, and it has detected
no problems.
bfd/ChangeLog:
* elf-bfd.h (elf_backend_data) <elf_backend_core_find_build_id>: New
field.
(_bfd_elf32_core_find_build_id, _bfd_elf64_core_find_build_id): New
functions.
(elf_read_notes): Add declaration.
* elf.c (elf_read_notes): Move elf-bfd.h.
(_bfd_elf_core_find_build_id): New function.
(bfd_section_from_phdr): Scan core file PT_LOAD segments for
build-id if none is known.
(elf_parse_notes): For core files, scan for notes.
* elfcore.h (elf_core_file_matches_executable_p): If both
BFDs have identical build-ids, then they match.
(_bfd_elf_core_find_build_id): New function.
* elfxx-target.h (elf_backend_core_find_build_id): Define.
(elfNN_bed): Add elf_backend_core_find_build_id.
---
bfd/elf-bfd.h | 8 ++++
bfd/elf.c | 23 +++++++---
bfd/elfcore.h | 106 +++++++++++++++++++++++++++++++++++++++++++++
bfd/elfxx-target.h | 4 ++
4 files changed, 136 insertions(+), 5 deletions(-)
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 0a83c17332..2d214be87f 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1377,6 +1377,8 @@ struct elf_backend_data
int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
bfd_size_type len));
+ bfd_boolean (*elf_backend_core_find_build_id) (bfd *, bfd_vma);
+
/* This function is used by `_bfd_elf_get_synthetic_symtab';
see elf.c. */
bfd_vma (*plt_sym_val) (bfd_vma, const asection *, const arelent *);
@@ -2398,6 +2400,8 @@ extern bfd_boolean bfd_elf32_core_file_matches_executable_p
(bfd *, bfd *);
extern int bfd_elf32_core_file_pid
(bfd *);
+extern bfd_boolean _bfd_elf32_core_find_build_id
+ (bfd *, bfd_vma);
extern bfd_boolean bfd_elf32_swap_symbol_in
(bfd *, const void *, const void *, Elf_Internal_Sym *);
@@ -2444,6 +2448,8 @@ extern bfd_boolean bfd_elf64_core_file_matches_executable_p
(bfd *, bfd *);
extern int bfd_elf64_core_file_pid
(bfd *);
+extern bfd_boolean _bfd_elf64_core_find_build_id
+ (bfd *, bfd_vma);
extern bfd_boolean bfd_elf64_swap_symbol_in
(bfd *, const void *, const void *, Elf_Internal_Sym *);
@@ -2768,6 +2774,8 @@ extern bfd_boolean _bfd_elf_merge_object_attributes
extern bfd_boolean _bfd_elf_merge_unknown_attribute_low (bfd *, bfd *, int);
extern bfd_boolean _bfd_elf_merge_unknown_attribute_list (bfd *, bfd *);
extern Elf_Internal_Shdr *_bfd_elf_single_rel_hdr (asection *sec);
+extern bfd_boolean elf_read_notes (bfd *, file_ptr, bfd_size_type,
+ size_t align) ;
extern bfd_boolean _bfd_elf_parse_gnu_properties
(bfd *, Elf_Internal_Note *);
diff --git a/bfd/elf.c b/bfd/elf.c
index 664eae5c66..2ae3ec53c8 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -53,8 +53,6 @@ static int elf_sort_sections (const void *, const void *);
static bfd_boolean assign_file_positions_except_relocs (bfd *, struct bfd_link_info *);
static bfd_boolean prep_headers (bfd *);
static bfd_boolean swap_out_syms (bfd *, struct elf_strtab_hash **, int) ;
-static bfd_boolean elf_read_notes (bfd *, file_ptr, bfd_size_type,
- size_t align) ;
static bfd_boolean elf_parse_notes (bfd *abfd, char *buf, size_t size,
file_ptr offset, size_t align);
@@ -3060,6 +3058,16 @@ _bfd_elf_make_section_from_phdr (bfd *abfd,
return TRUE;
}
+static bfd_boolean
+_bfd_elf_core_find_build_id (bfd *templ, bfd_vma offset)
+{
+ /* The return value is ignored. Build-ids are considered optional. */
+ if (templ->xvec->flavour == bfd_target_elf_flavour)
+ return (*get_elf_backend_data (templ)->elf_backend_core_find_build_id)
+ (templ, offset);
+ return FALSE;
+}
+
bfd_boolean
bfd_section_from_phdr (bfd *abfd, Elf_Internal_Phdr *hdr, int hdr_index)
{
@@ -3071,7 +3079,11 @@ bfd_section_from_phdr (bfd *abfd, Elf_Internal_Phdr *hdr, int hdr_index)
return _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "null");
case PT_LOAD:
- return _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "load");
+ if (! _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "load"))
+ return FALSE;
+ if (bfd_get_format (abfd) == bfd_core && abfd->build_id == NULL)
+ _bfd_elf_core_find_build_id (abfd, hdr->p_offset);
+ return TRUE;
case PT_DYNAMIC:
return _bfd_elf_make_section_from_phdr (abfd, hdr, hdr_index, "dynamic");
@@ -11790,7 +11802,8 @@ elf_parse_notes (bfd *abfd, char *buf, size_t size, file_ptr offset,
GROKER_ELEMENT ("NetBSD-CORE", elfcore_grok_netbsd_note),
GROKER_ELEMENT ( "OpenBSD", elfcore_grok_openbsd_note),
GROKER_ELEMENT ("QNX", elfcore_grok_nto_note),
- GROKER_ELEMENT ("SPU/", elfcore_grok_spu_note)
+ GROKER_ELEMENT ("SPU/", elfcore_grok_spu_note),
+ GROKER_ELEMENT ("GNU", elfobj_grok_gnu_note)
};
#undef GROKER_ELEMENT
int i;
@@ -11830,7 +11843,7 @@ elf_parse_notes (bfd *abfd, char *buf, size_t size, file_ptr offset,
return TRUE;
}
-static bfd_boolean
+bfd_boolean
elf_read_notes (bfd *abfd, file_ptr offset, bfd_size_type size,
size_t align)
{
diff --git a/bfd/elfcore.h b/bfd/elfcore.h
index 3550eaac27..751e831914 100644
--- a/bfd/elfcore.h
+++ b/bfd/elfcore.h
@@ -49,6 +49,13 @@ elf_core_file_matches_executable_p (bfd *core_bfd, bfd *exec_bfd)
return FALSE;
}
+ /* If both BFDs have identical build-ids, then they match. */
+ if (core_bfd->build_id != NULL && exec_bfd->build_id != NULL
+ && core_bfd->build_id->size == exec_bfd->build_id->size
+ && memcmp (core_bfd->build_id->data, exec_bfd->build_id->data,
+ core_bfd->build_id->size) == 0)
+ return TRUE;
+
/* See if the name in the corefile matches the executable name. */
corename = elf_tdata (core_bfd)->core->program;
if (corename != NULL)
@@ -313,3 +320,102 @@ wrong:
fail:
return NULL;
}
+
+/* Attempt to find a build-id in a core file from the core file BFD.
+ OFFSET is the file offset to a PT_LOAD segment that may contain
+ the build-id note. Returns TRUE upon success, FALSE otherwise. */
+
+bfd_boolean
+NAME(_bfd_elf,core_find_build_id)
+ (bfd *abfd,
+ bfd_vma offset)
+{
+ Elf_External_Ehdr x_ehdr; /* Elf file header, external form. */
+ Elf_Internal_Ehdr i_ehdr; /* Elf file header, internal form. */
+ Elf_Internal_Phdr *i_phdr;
+ unsigned int i;
+
+ /* Seek to the position of the segment at OFFSET. */
+ if (bfd_seek (abfd, offset, SEEK_SET) != 0)
+ goto fail;
+
+ /* Read in the ELF header in external format. */
+ if (bfd_bread (&x_ehdr, sizeof (x_ehdr), abfd) != sizeof (x_ehdr))
+ {
+ if (bfd_get_error () != bfd_error_system_call)
+ goto wrong;
+ else
+ goto fail;
+ }
+
+ /* Now check to see if we have a valid ELF file, and one that BFD can
+ make use of. The magic number must match, the address size ('class')
+ and byte-swapping must match our XVEC entry, and it must have a
+ section header table (FIXME: See comments re sections at top of this
+ file). */
+
+ if (! elf_file_p (&x_ehdr)
+ || x_ehdr.e_ident[EI_VERSION] != EV_CURRENT
+ || x_ehdr.e_ident[EI_CLASS] != ELFCLASS)
+ goto wrong;
+
+ /* Check that file's byte order matches xvec's */
+ switch (x_ehdr.e_ident[EI_DATA])
+ {
+ case ELFDATA2MSB: /* Big-endian */
+ if (! bfd_header_big_endian (abfd))
+ goto wrong;
+ break;
+ case ELFDATA2LSB: /* Little-endian */
+ if (! bfd_header_little_endian (abfd))
+ goto wrong;
+ break;
+ case ELFDATANONE: /* No data encoding specified */
+ default: /* Unknown data encoding specified */
+ goto wrong;
+ }
+
+ elf_swap_ehdr_in (abfd, &x_ehdr, &i_ehdr);
+#if DEBUG & 1
+ elf_debug_file (&i_ehdr);
+#endif
+
+ if (i_ehdr.e_phentsize != sizeof (Elf_External_Phdr) || i_ehdr.e_phnum == 0)
+ goto fail;
+
+ /* Read in program headers. */
+ i_phdr = (Elf_Internal_Phdr *) bfd_alloc2 (abfd, i_ehdr.e_phnum,
+ sizeof (*i_phdr));
+ if (i_phdr == NULL)
+ goto fail;
+
+ if (bfd_seek (abfd, (file_ptr) (offset + i_ehdr.e_phoff), SEEK_SET) != 0)
+ goto fail;
+
+ /* Read in program headers and parse notes. */
+ for (i = 0; i < i_ehdr.e_phnum; ++i, ++i_phdr)
+ {
+ Elf_External_Phdr x_phdr;
+
+ if (bfd_bread (&x_phdr, sizeof (x_phdr), abfd) != sizeof (x_phdr))
+ goto fail;
+ elf_swap_phdr_in (abfd, &x_phdr, i_phdr);
+
+ if (i_phdr->p_type == PT_NOTE && i_phdr->p_filesz > 0)
+ {
+ elf_read_notes (abfd, offset + i_phdr->p_offset,
+ i_phdr->p_filesz, i_phdr->p_align);
+ if (abfd->build_id != NULL)
+ return TRUE;
+ }
+ }
+
+ /* Having gotten this far, we have a valid ELF section, but no
+ build-id was found. */
+ goto fail;
+
+wrong:
+ bfd_set_error (bfd_error_wrong_format);
+fail:
+ return FALSE;
+}
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 78a1f6314d..29ad2e8481 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -521,6 +521,9 @@
#ifndef elf_backend_bfd_from_remote_memory
#define elf_backend_bfd_from_remote_memory _bfd_elfNN_bfd_from_remote_memory
#endif
+#ifndef elf_backend_core_find_build_id
+#define elf_backend_core_find_build_id _bfd_elfNN_core_find_build_id
+#endif
#ifndef elf_backend_got_header_size
#define elf_backend_got_header_size 0
#endif
@@ -860,6 +863,7 @@ static struct elf_backend_data elfNN_bed =
elf_backend_mips_rtype_to_howto,
elf_backend_ecoff_debug_swap,
elf_backend_bfd_from_remote_memory,
+ elf_backend_core_find_build_id,
elf_backend_plt_sym_val,
elf_backend_common_definition,
elf_backend_common_section_index,
--
2.21.0
More information about the Binutils
mailing list