[PATCH] Core file build-ids
Sergio Durigan Junior
sergiodj@redhat.com
Wed Jun 5 18:57:00 GMT 2019
On Wednesday, June 05 2019, Keith Seitz wrote:
> 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.
First of all, thanks for doing this.
Just a few comments that came to my mind while looking at the patch.
> I have run this patch through GDB's buildbot, and it has detected
> no problems.
>
> I'm a BFD novice, and the approach used here could be more conservative
> (such as only reading build-id notes) -- careful review is warranted.
>
> Keith
>
> bfd/ChangeLog:
>
> * bfd-in.h (_bfd_elf_core_find_build_id): New function.
> * bfd-in2.h: Regenerate.
> * 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/bfd-in.h | 2 +
> bfd/bfd-in2.h | 2 +
> bfd/elf-bfd.h | 8 ++++
> bfd/elf.c | 73 ++++++++++++++++++++++--------------
> bfd/elfcore.h | 92 ++++++++++++++++++++++++++++++++++++++++++++++
> bfd/elfxx-target.h | 4 ++
> 6 files changed, 153 insertions(+), 28 deletions(-)
>
> diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
> index 890a79d409..868be0bedd 100644
> --- a/bfd/bfd-in.h
> +++ b/bfd/bfd-in.h
> @@ -763,6 +763,8 @@ extern bfd_boolean bfd_cr16_elf32_create_embedded_relocs
> (bfd *, struct bfd_link_info *, struct bfd_section *, struct bfd_section *,
> char **);
>
> +extern void _bfd_elf_core_find_build_id (bfd *, bfd_vma);
> +
> /* SunOS shared library support routines for the linker. */
>
> extern struct bfd_link_needed_list *bfd_sunos_get_needed_list
> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 450c7b7fb1..c9b7171eea 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -770,6 +770,8 @@ extern bfd_boolean bfd_cr16_elf32_create_embedded_relocs
> (bfd *, struct bfd_link_info *, struct bfd_section *, struct bfd_section *,
> char **);
>
> +extern void _bfd_elf_core_find_build_id (bfd *, bfd_vma);
> +
> /* SunOS shared library support routines for the linker. */
>
> extern struct bfd_link_needed_list *bfd_sunos_get_needed_list
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index 0d12f4533a..4a4a47d188 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -1371,6 +1371,8 @@ struct elf_backend_data
> int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
> bfd_size_type len));
>
> + void (*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 *);
> @@ -2350,6 +2352,8 @@ extern bfd_boolean bfd_elf32_core_file_matches_executable_p
> (bfd *, bfd *);
> extern int bfd_elf32_core_file_pid
> (bfd *);
> +extern void _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 *);
> @@ -2396,6 +2400,8 @@ extern bfd_boolean bfd_elf64_core_file_matches_executable_p
> (bfd *, bfd *);
> extern int bfd_elf64_core_file_pid
> (bfd *);
> +extern void _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 *);
> @@ -2704,6 +2710,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 b463f1df8b..1d86a0a4d6 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);
>
> @@ -3008,6 +3006,13 @@ _bfd_elf_make_section_from_phdr (bfd *abfd,
> return TRUE;
> }
>
> +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);
> +}
> +
> bfd_boolean
> bfd_section_from_phdr (bfd *abfd, Elf_Internal_Phdr *hdr, int hdr_index)
> {
> @@ -3019,7 +3024,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");
> @@ -11667,34 +11676,42 @@ elf_parse_notes (bfd *abfd, char *buf, size_t size, file_ptr offset,
>
> case bfd_core:
> {
> + if (in.namesz == sizeof "GNU" && strcmp (in.namedata, "GNU") == 0)
> + {
> + if (! elfobj_grok_gnu_note (abfd, &in))
> + return FALSE;
> + }
> + else
> + {
You could rewrite this to get rid of the "else" part, which would make
the patch smaller:
if (in.namesz == sizeof "GNU" && strcmp (in.namedata, "GNU") == 0
&& ! elfobj_grok_gnu_note (abfd, &in))
return FALSE;
> #define GROKER_ELEMENT(S,F) {S, sizeof (S) - 1, F}
> - struct
> - {
> - const char * string;
> - size_t len;
> - bfd_boolean (* func)(bfd *, Elf_Internal_Note *);
> - }
> - grokers[] =
> - {
> - GROKER_ELEMENT ("", elfcore_grok_note),
> - GROKER_ELEMENT ("FreeBSD", elfcore_grok_freebsd_note),
> - 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)
> - };
> + struct
> + {
> + const char * string;
> + size_t len;
> + bfd_boolean (* func)(bfd *, Elf_Internal_Note *);
> + }
> + grokers[] =
> + {
> + GROKER_ELEMENT ("", elfcore_grok_note),
> + GROKER_ELEMENT ("FreeBSD", elfcore_grok_freebsd_note),
> + 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)
> + };
> #undef GROKER_ELEMENT
> - int i;
> + int i;
This seems unrelated to the patch.
>
> - for (i = ARRAY_SIZE (grokers); i--;)
> - {
> - if (in.namesz >= grokers[i].len
> - && strncmp (in.namedata, grokers[i].string,
> - grokers[i].len) == 0)
> + for (i = ARRAY_SIZE (grokers); i--;)
> {
> - if (! grokers[i].func (abfd, & in))
> - return FALSE;
> - break;
> + if (in.namesz >= grokers[i].len
> + && strncmp (in.namedata, grokers[i].string,
> + grokers[i].len) == 0)
> + {
> + if (! grokers[i].func (abfd, & in))
> + return FALSE;
> + break;
> + }
> }
> }
> break;
> @@ -11721,7 +11738,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 395feb5ef3..395870942c 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,88 @@ 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. */
> +
> +void
> +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)
> + return;
> +
> + /* Read in the ELF header in external format. */
> + if (bfd_bread (&x_ehdr, sizeof (x_ehdr), abfd) != sizeof (x_ehdr))
> + return;
> +
> + /* 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)
> + return;
> +
> + /* 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))
> + return;
> + break;
> + case ELFDATA2LSB: /* Little-endian */
> + if (! bfd_header_little_endian (abfd))
> + return;
> + break;
> + case ELFDATANONE: /* No data encoding specified */
> + default: /* Unknown data encoding specified */
> + return;
> + }
> +
> + 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)
> + return;
> +
> + /* Read in program headers. */
> + i_phdr = (Elf_Internal_Phdr *) bfd_alloc2 (abfd, i_ehdr.e_phnum,
> + sizeof (*i_phdr));
> + if (i_phdr == NULL)
> + return;
> +
> + if (bfd_seek (abfd, (file_ptr) (offset + i_ehdr.e_phoff), SEEK_SET) != 0)
> + return;
> +
> + /* 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))
> + return;
> + 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;
> + }
> + }
> +}
> diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
> index e4e7546243..96b96ec62a 100644
> --- a/bfd/elfxx-target.h
> +++ b/bfd/elfxx-target.h
> @@ -517,6 +517,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
> @@ -852,6 +855,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.20.1
The rest makes sense to me, but I'm not a BFD expert :-).
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
More information about the Binutils
mailing list