Core file build-ids

Message ID 20190605184627.11230-1-keiths@redhat.com
State New
Headers show
Series
  • Core file build-ids
Related show

Commit Message

Keith Seitz June 5, 2019, 6:46 p.m.
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.

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(-)

-- 
2.20.1

Comments

Sergio Durigan Junior June 5, 2019, 6:57 p.m. | #1
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/
Keith Seitz June 5, 2019, 7:19 p.m. | #2
On 6/5/19 11:57 AM, Sergio Durigan Junior wrote:
> On Wednesday, June 05 2019, Keith Seitz wrote:

>> diff --git a/bfd/elf.c b/bfd/elf.c

>> index b463f1df8b..1d86a0a4d6 100644

>> --- a/bfd/elf.c

>> +++ b/bfd/elf.c

>> @@ -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:

> 


Actually, I think I can make this even simpler. Not sure why I didn't do
this originally, but I'm blaming the weather.

@@ -11681,7 +11690,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;

Thank you for taking a peek,
Keith
Nick Clifton June 7, 2019, 4:16 p.m. | #3
Hi Keith,

  I do have a couple of comments on this patch:

> +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.


> -      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.


> +/* 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.


> +  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.

Cheers
  Nick

Patch

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
+	      {
 #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;
 
-	    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,