Patchwork [PATCH+8.3?] Implement dump of mappings with ELF headers by gcore

login
register
mail settings
Submitter Sergio Durigan Junior
Date April 25, 2019, 6:24 p.m.
Message ID <87mukec594.fsf@redhat.com>
Download mbox | patch
Permalink /patch/32418/
State New
Headers show

Comments

Sergio Durigan Junior - April 25, 2019, 6:24 p.m.
On Thursday, April 25 2019, Kevin Buettner wrote:

> On Tue, 23 Apr 2019 19:46:35 -0400
> Sergio Durigan Junior <sergiodj@redhat.com> wrote:
>
>> This patch has a long story, but it all started back in 2015, with
> [...]
>
> Thanks for the detailed commit comment!

Thanks for the review, Kevin.

> [...]
>> I think it makes sense to include this patch into 8.3, since it's
>> pretty "self-contained", but I will leave that decision to the GMs.
>> 
>> gdb/ChangeLog:
>> 2019-04-24  Sergio Durigan Junior  <sergiodj@redhat.com>
>> 
>> 	PR corefiles/11608
>> 	PR corefiles/18187
>> 	* linux-tdep.c (dump_mapping_p): Add new parameters "ADDR" and
>> 	"OFFSET".  Verify if current mapping contains an ELF header.
>> 	(linux_find_memory_regions_full): Adjust call to
>> 	"dump_mapping_p".
>
> I don't think that the double quotes enclosing ADDR and OFFSET are
> needed here.

Ah, OK.  I tend to use them when I'm writing ChangeLog entries, but
that's a matter of style.  I'll remove them.

>> gdb/testsuite/ChangeLog:
>> 2019-04-24  Sergio Durigan Junior  <sergiodj@redhat.com>
>> 
>> 	PR corefiles/11608
>> 	PR corefiles/18187
>> 	* gdb.base/coredump-filter-build-id.exp: New file.
>> 	* lib/future.exp (gdb_find_eu-unstrip): New procedure.
>> ---
>>  gdb/linux-tdep.c                              | 71 +++++++++++++++----
>>  .../gdb.base/coredump-filter-build-id.exp     | 69 ++++++++++++++++++
>>  gdb/testsuite/lib/future.exp                  | 10 +++
>>  3 files changed, 137 insertions(+), 13 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.base/coredump-filter-build-id.exp
>> 
>> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
>> index 5de985def3..d71a00666f 100644
>> --- a/gdb/linux-tdep.c
>> +++ b/gdb/linux-tdep.c
>> @@ -586,8 +586,8 @@ mapping_is_anonymous_p (const char *filename)
>>  }
>>  
>>  /* Return 0 if the memory mapping (which is related to FILTERFLAGS, V,
>> -   MAYBE_PRIVATE_P, and MAPPING_ANONYMOUS_P) should not be dumped, or
>> -   greater than 0 if it should.
>> +   MAYBE_PRIVATE_P, MAPPING_ANONYMOUS_P, ADDR and OFFSET) should not
>> +   be dumped, or greater than 0 if it should.
>>  
>>     In a nutshell, this is the logic that we follow in order to decide
>>     if a mapping should be dumped or not.
>> @@ -625,12 +625,17 @@ mapping_is_anonymous_p (const char *filename)
>>       see 'p' in the permission flags, then we assume that the mapping
>>       is private, even though the presence of the 's' flag there would
>>       mean VM_MAYSHARE, which means the mapping could still be private.
>> -     This should work OK enough, however.  */
>> +     This should work OK enough, however.
>> +
>> +   - Even if, at the end, we decided that we should not dump the
>> +     mapping, we still have to check if it is something like an ELF
>> +     header (of a DSO or an executable, for example).  If it is, and
>> +     if the user is interested in dump it, then we should dump it.  */
>>  
>>  static int
>>  dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
>>  		int maybe_private_p, int mapping_anon_p, int mapping_file_p,
>> -		const char *filename)
>> +		const char *filename, ULONGEST addr, ULONGEST offset)
>
> I was surprised to see that addr is ULONGEST instead of CORE_ADDR, but
> I see that this matches the type of of the variables passed by the caller.
> Both are typedef'd to unsigned long long, so it probably doesn't matter.

Yeah.  I had the same thought, and reached the same conclusion :-).

>>  {
>>    /* Initially, we trust in what we received from our caller.  This
>>       value may not be very precise (i.e., it was probably gathered
>> @@ -640,6 +645,7 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
>>       (assuming that the version of the Linux kernel being used
>>       supports it, of course).  */
>>    int private_p = maybe_private_p;
>> +  int dump_p;
>>  
>>    /* We always dump vDSO and vsyscall mappings, because it's likely that
>>       there'll be no file to read the contents from at core load time.
>> @@ -680,13 +686,13 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
>>  	  /* This is a special situation.  It can happen when we see a
>>  	     mapping that is file-backed, but that contains anonymous
>>  	     pages.  */
>> -	  return ((filterflags & COREFILTER_ANON_PRIVATE) != 0
>> -		  || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0);
>> +	  dump_p = ((filterflags & COREFILTER_ANON_PRIVATE) != 0
>> +		    || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0);
>>  	}
>>        else if (mapping_anon_p)
>> -	return (filterflags & COREFILTER_ANON_PRIVATE) != 0;
>> +	dump_p = (filterflags & COREFILTER_ANON_PRIVATE) != 0;
>>        else
>> -	return (filterflags & COREFILTER_MAPPED_PRIVATE) != 0;
>> +	dump_p = (filterflags & COREFILTER_MAPPED_PRIVATE) != 0;
>>      }
>>    else
>>      {
>> @@ -695,14 +701,53 @@ dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
>>  	  /* This is a special situation.  It can happen when we see a
>>  	     mapping that is file-backed, but that contains anonymous
>>  	     pages.  */
>> -	  return ((filterflags & COREFILTER_ANON_SHARED) != 0
>> -		  || (filterflags & COREFILTER_MAPPED_SHARED) != 0);
>> +	  dump_p = ((filterflags & COREFILTER_ANON_SHARED) != 0
>> +		    || (filterflags & COREFILTER_MAPPED_SHARED) != 0);
>>  	}
>>        else if (mapping_anon_p)
>> -	return (filterflags & COREFILTER_ANON_SHARED) != 0;
>> +	dump_p = (filterflags & COREFILTER_ANON_SHARED) != 0;
>>        else
>> -	return (filterflags & COREFILTER_MAPPED_SHARED) != 0;
>> +	dump_p = (filterflags & COREFILTER_MAPPED_SHARED) != 0;
>>      }
>> +
>> +  /* Even if we decided that we shouldn't dump this mapping, we still
>> +     have to check whether (a) the user wants us to dump mappings
>> +     containing an ELF header, and (b) the mapping in question
>> +     contains an ELF header.  If (a) and (b) are true, then we should
>> +     dump this mapping.
>> +
>> +     A mapping contains an ELF header if it is a private mapping, its
>> +     offset is zero, and its first word is ELFMAG.  */
>> +  if (!dump_p && private_p && offset == 0
>> +      && (filterflags & COREFILTER_ELF_HEADERS) != 0)
>> +    {
>> +      /* Let's check if we have an ELF header.  */
>> +      gdb::unique_xmalloc_ptr<char> header;
>> +      int errcode;
>> +
>> +      /* Useful define specifying the size of the ELF magical
>> +	 header.  */
>> +#ifndef SELFMAG
>> +#define SELFMAG 4
>> +#endif
>> +
>> +      /* Read the first SELFMAG bytes and check if it is ELFMAG.  */
>> +      if (target_read_string (addr, &header, SELFMAG, &errcode) == SELFMAG
>> +	  && errcode == 0)
>> +	{
>> +	  const char *h = header.get ();
>> +
>> +	  if (h[EI_MAG0] == ELFMAG0 && h[EI_MAG1] == ELFMAG1
>> +	      && h[EI_MAG2] == ELFMAG2 && h[EI_MAG3] == ELFMAG3)
>
> Somewhere in here, either above the "if" or in the comment for SELFMAG,
> I think it'd be useful to note that the ELFMAG0..ELFMAG3 and 
> EI_MAG0..EI_MAG3 come from elf/common.h.

Alright.

> (My concern here was whether ELFMAG0, etc. were defined by a
> system header.  If so, we can't use those defines here.  But since
> they're defined in elf/common.h, they're perfectly fine.  This is
> why I think it'd be useful to note where we expect them to come
> from.  Otherwise, such a comment would just be clutter.)
>
> Regarding SELFMAG, it's a shame that it's not it's not also defined in
> elf/common.h.  I see that SELFMAG is also used in
> gdbserver/linux-ppc-low.c.  I'm guessing that it obtains this value
> from either /usr/include/elf.h or /usr/include/linux/elf.h.  (I don't
> want to hold up your patch to make that header file change though; there
> might be a good reason to not touch elf/common.h.)

I have a patch here to add SELFMAG and ELFMAG to elf/common.h.  I'm not
entirely sure where I should send it (binutils?  gcc?), but I plan to
propose it once I'm finished with this build-id patch.

>> +	    {
>> +	      /* This mapping contains an ELF header, so we
>> +		 should dump it.  */
>> +	      dump_p = 1;
>> +	    }
>> +	}
>> +    }
>> +
>> +  return dump_p;
>>  }
>>  
>>  /* Implement the "info proc" command.  */
>> @@ -1306,7 +1351,7 @@ linux_find_memory_regions_full (struct gdbarch *gdbarch,
>>  	  if (has_anonymous)
>>  	    should_dump_p = dump_mapping_p (filterflags, &v, priv,
>>  					    mapping_anon_p, mapping_file_p,
>> -					    filename);
>> +					    filename, addr, offset);
>>  	  else
>>  	    {
>>  	      /* Older Linux kernels did not support the "Anonymous:" counter.
>> diff --git a/gdb/testsuite/gdb.base/coredump-filter-build-id.exp b/gdb/testsuite/gdb.base/coredump-filter-build-id.exp
>> new file mode 100644
>> index 0000000000..fc2b039406
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/coredump-filter-build-id.exp
>> @@ -0,0 +1,69 @@
>> +# Copyright 2019 Free Software Foundation, Inc.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +# Test whether GDB's gcore/generate-core-file command can dump memory
>> +# mappings with ELF headers, containing a build-id note.
>> +#
>> +# Due to the fact that we don't have an easy way to process a corefile
>> +# and look for specific notes using GDB/dejagnu, we rely on an
>> +# external tool, eu-unstrip, to verify if the corefile contains
>> +# build-ids.
>> +
>> +standard_testfile "normal.c"
>> +
>> +# This test is Linux x86_64 only.
>> +if { ![istarget *-*-linux*] } {
>> +    untested "$testfile.exp"
>> +    return -1
>> +}
>> +if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
>> +    untested "$testfile.exp"
>> +    return -1
>> +}
>> +
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
>> +    return -1
>> +}
>> +
>> +if { ![runto_main] } {
>> +    untested "could not run to main"
>> +    return -1
>> +}
>> +
>> +# First we need to generate a corefile.
>> +set corefilename "[standard_output_file gcore.test]"
>> +if { ![gdb_gcore_cmd "$corefilename" "save corefile"] } {
>> +    verbose -log "Could not save corefile"
>> +    untested "$testfile.exp"
>> +    return -1
>> +}
>> +
>> +# Determine if GDB dumped the mapping containing the build-id.  This
>> +# is done by invoking an external program (eu-unstrip).
>> +if { [catch "exec [gdb_find_eu-unstrip] -n --core $corefilename" output] == 0 } {
>> +    set line [lindex [split $output "\n"] 0]
>> +    set test "gcore dumped mapping with build-id"
>> +
>> +    verbose -log "First line of eu-unstrip: $line"
>> +
>> +    if { [regexp "^${hex}\\+${hex} \[a-f0-9\]+@${hex}.*[string_to_regexp $binfile]$" $line] } {
>> +	pass "$test"
>> +    } else {
>> +	fail "$test"
>> +    }
>> +} else {
>> +    verbose -log "Could not execute eu-unstrip program"
>> +    untested "$testfile.exp"
>> +}
>> diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
>> index a56cd019b4..122e652858 100644
>> --- a/gdb/testsuite/lib/future.exp
>> +++ b/gdb/testsuite/lib/future.exp
>> @@ -162,6 +162,16 @@ proc gdb_find_readelf {} {
>>      return $readelf
>>  }
>>  
>> +proc gdb_find_eu-unstrip {} {
>> +    global EU_UNSTRIP_FOR_TARGET
>> +    if [info exists EU_UNSTRIP_FOR_TARGET] {
>> +	set eu_unstrip $EU_UNSTRIP_FOR_TARGET
>> +    } else {
>> +	set eu_unstrip [transform eu-unstrip]
>> +    }
>> +    return $eu_unstrip
>> +}
>> +
>>  proc gdb_default_target_compile {source destfile type options} {
>>      global target_triplet
>>      global tool_root_dir
>
> The test case LGTM though I'm not familiar with eu-unstrip.
>
> I think it's okay for master with the nits that I found fixed.  I don't
> have an opinion about including it in 8.3.

Thanks, I pushed the patch below to master:

57e5e645010430b3d73f8c6a757d09f48dc8f8d5

I'll wait for another GM to give an opinion about including this into
8.3

Thanks,
Kevin Buettner - April 25, 2019, 9:04 p.m.
On Thu, 25 Apr 2019 14:24:39 -0400
Sergio Durigan Junior <sergiodj@redhat.com> wrote:

> >> 	* linux-tdep.c (dump_mapping_p): Add new parameters "ADDR" and
> >> 	"OFFSET".  Verify if current mapping contains an ELF header.
> >> 	(linux_find_memory_regions_full): Adjust call to
> >> 	"dump_mapping_p".  
> >
> > I don't think that the double quotes enclosing ADDR and OFFSET are
> > needed here.  
> 
> Ah, OK.  I tend to use them when I'm writing ChangeLog entries, but
> that's a matter of style.  I'll remove them.

I think that quotes are appropriate when the actual names of the
parameters are used.  But (as I recall) the GNU convention is to
change the names to all caps when referring to parameters in
documentation.  ChangeLog entries are a form of documenation, so no
quotes. 

So, had you said:

  Add new parameters "addr" and "offset".

or:

  Add new parameters ``addr'' and ``offset''.

...I would have been happy with the quotes.

But since you uppercased them, I don't think the quotes are needed
or even desired.

[...]
> > I think it's okay for master with the nits that I found fixed.  I don't
> > have an opinion about including it in 8.3.  
> 
> Thanks, I pushed the patch below to master:
> 
> 57e5e645010430b3d73f8c6a757d09f48dc8f8d5

I read your revised patch - didn't see any problems.

> I'll wait for another GM to give an opinion about including this into
> 8.3

If 8.3 is getting close, I think it should only go in master.  But
if 8.3 still has some outstanding issues and testing yet to be done,
it might go in, especially if non-Fedora users are noticing problems.

Kevin
Sergio Durigan Junior - April 26, 2019, 4:45 p.m.
On Thursday, April 25 2019, Kevin Buettner wrote:

> If 8.3 is getting close, I think it should only go in master.  But
> if 8.3 still has some outstanding issues and testing yet to be done,
> it might go in, especially if non-Fedora users are noticing problems.

Sure, no problem.  If it's decided that this shouldn't go in 8.3, I can
always include this patch in the next Fedora GDB release as well (since
there's a Fedora bug related to this).

Thanks,
Pedro Alves - April 26, 2019, 4:48 p.m.
On 4/25/19 10:04 PM, Kevin Buettner wrote:
> On Thu, 25 Apr 2019 14:24:39 -0400
> Sergio Durigan Junior <sergiodj@redhat.com> wrote:
> 
>>>> 	* linux-tdep.c (dump_mapping_p): Add new parameters "ADDR" and
>>>> 	"OFFSET".  Verify if current mapping contains an ELF header.
>>>> 	(linux_find_memory_regions_full): Adjust call to
>>>> 	"dump_mapping_p".  
>>>
>>> I don't think that the double quotes enclosing ADDR and OFFSET are
>>> needed here.  
>>
>> Ah, OK.  I tend to use them when I'm writing ChangeLog entries, but
>> that's a matter of style.  I'll remove them.
> 
> I think that quotes are appropriate when the actual names of the
> parameters are used.  But (as I recall) the GNU convention is to
> change the names to all caps when referring to parameters in
> documentation.  ChangeLog entries are a form of documenation, so no
> quotes. 

All caps is used when referring to the values of the parameters, not
the parameters themselves.
From <https://www.gnu.org/prep/standards/standards.html>:

 "The comment on a function is much clearer if you use the argument
  names to speak about the argument values. The variable name itself
  should be lower case, but write it in upper case when you are
  speaking about the value rather than the variable itself. Thus,
  “the inode number NODE_NUM” rather than “an inode”. "

So in this case it should be lowercase, since you're talking about the name of
the variable/parameter.

I usually say "New parameter 'parameter'.", with single quotes.  You can
find past examples in the ChangeLogs.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 5de985def3..c1666d189a 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -586,8 +586,8 @@  mapping_is_anonymous_p (const char *filename)
 }
 
 /* Return 0 if the memory mapping (which is related to FILTERFLAGS, V,
-   MAYBE_PRIVATE_P, and MAPPING_ANONYMOUS_P) should not be dumped, or
-   greater than 0 if it should.
+   MAYBE_PRIVATE_P, MAPPING_ANONYMOUS_P, ADDR and OFFSET) should not
+   be dumped, or greater than 0 if it should.
 
    In a nutshell, this is the logic that we follow in order to decide
    if a mapping should be dumped or not.
@@ -625,12 +625,17 @@  mapping_is_anonymous_p (const char *filename)
      see 'p' in the permission flags, then we assume that the mapping
      is private, even though the presence of the 's' flag there would
      mean VM_MAYSHARE, which means the mapping could still be private.
-     This should work OK enough, however.  */
+     This should work OK enough, however.
+
+   - Even if, at the end, we decided that we should not dump the
+     mapping, we still have to check if it is something like an ELF
+     header (of a DSO or an executable, for example).  If it is, and
+     if the user is interested in dump it, then we should dump it.  */
 
 static int
 dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
 		int maybe_private_p, int mapping_anon_p, int mapping_file_p,
-		const char *filename)
+		const char *filename, ULONGEST addr, ULONGEST offset)
 {
   /* Initially, we trust in what we received from our caller.  This
      value may not be very precise (i.e., it was probably gathered
@@ -640,6 +645,7 @@  dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
      (assuming that the version of the Linux kernel being used
      supports it, of course).  */
   int private_p = maybe_private_p;
+  int dump_p;
 
   /* We always dump vDSO and vsyscall mappings, because it's likely that
      there'll be no file to read the contents from at core load time.
@@ -680,13 +686,13 @@  dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
 	  /* This is a special situation.  It can happen when we see a
 	     mapping that is file-backed, but that contains anonymous
 	     pages.  */
-	  return ((filterflags & COREFILTER_ANON_PRIVATE) != 0
-		  || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0);
+	  dump_p = ((filterflags & COREFILTER_ANON_PRIVATE) != 0
+		    || (filterflags & COREFILTER_MAPPED_PRIVATE) != 0);
 	}
       else if (mapping_anon_p)
-	return (filterflags & COREFILTER_ANON_PRIVATE) != 0;
+	dump_p = (filterflags & COREFILTER_ANON_PRIVATE) != 0;
       else
-	return (filterflags & COREFILTER_MAPPED_PRIVATE) != 0;
+	dump_p = (filterflags & COREFILTER_MAPPED_PRIVATE) != 0;
     }
   else
     {
@@ -695,14 +701,55 @@  dump_mapping_p (filter_flags filterflags, const struct smaps_vmflags *v,
 	  /* This is a special situation.  It can happen when we see a
 	     mapping that is file-backed, but that contains anonymous
 	     pages.  */
-	  return ((filterflags & COREFILTER_ANON_SHARED) != 0
-		  || (filterflags & COREFILTER_MAPPED_SHARED) != 0);
+	  dump_p = ((filterflags & COREFILTER_ANON_SHARED) != 0
+		    || (filterflags & COREFILTER_MAPPED_SHARED) != 0);
 	}
       else if (mapping_anon_p)
-	return (filterflags & COREFILTER_ANON_SHARED) != 0;
+	dump_p = (filterflags & COREFILTER_ANON_SHARED) != 0;
       else
-	return (filterflags & COREFILTER_MAPPED_SHARED) != 0;
+	dump_p = (filterflags & COREFILTER_MAPPED_SHARED) != 0;
     }
+
+  /* Even if we decided that we shouldn't dump this mapping, we still
+     have to check whether (a) the user wants us to dump mappings
+     containing an ELF header, and (b) the mapping in question
+     contains an ELF header.  If (a) and (b) are true, then we should
+     dump this mapping.
+
+     A mapping contains an ELF header if it is a private mapping, its
+     offset is zero, and its first word is ELFMAG.  */
+  if (!dump_p && private_p && offset == 0
+      && (filterflags & COREFILTER_ELF_HEADERS) != 0)
+    {
+      /* Let's check if we have an ELF header.  */
+      gdb::unique_xmalloc_ptr<char> header;
+      int errcode;
+
+      /* Useful define specifying the size of the ELF magical
+	 header.  */
+#ifndef SELFMAG
+#define SELFMAG 4
+#endif
+
+      /* Read the first SELFMAG bytes and check if it is ELFMAG.  */
+      if (target_read_string (addr, &header, SELFMAG, &errcode) == SELFMAG
+	  && errcode == 0)
+	{
+	  const char *h = header.get ();
+
+	  /* The EI_MAG* and ELFMAG* constants come from
+	     <elf/common.h>.  */
+	  if (h[EI_MAG0] == ELFMAG0 && h[EI_MAG1] == ELFMAG1
+	      && h[EI_MAG2] == ELFMAG2 && h[EI_MAG3] == ELFMAG3)
+	    {
+	      /* This mapping contains an ELF header, so we
+		 should dump it.  */
+	      dump_p = 1;
+	    }
+	}
+    }
+
+  return dump_p;
 }
 
 /* Implement the "info proc" command.  */
@@ -1306,7 +1353,7 @@  linux_find_memory_regions_full (struct gdbarch *gdbarch,
 	  if (has_anonymous)
 	    should_dump_p = dump_mapping_p (filterflags, &v, priv,
 					    mapping_anon_p, mapping_file_p,
-					    filename);
+					    filename, addr, offset);
 	  else
 	    {
 	      /* Older Linux kernels did not support the "Anonymous:" counter.
diff --git a/gdb/testsuite/gdb.base/coredump-filter-build-id.exp b/gdb/testsuite/gdb.base/coredump-filter-build-id.exp
new file mode 100644
index 0000000000..fc2b039406
--- /dev/null
+++ b/gdb/testsuite/gdb.base/coredump-filter-build-id.exp
@@ -0,0 +1,69 @@ 
+# Copyright 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test whether GDB's gcore/generate-core-file command can dump memory
+# mappings with ELF headers, containing a build-id note.
+#
+# Due to the fact that we don't have an easy way to process a corefile
+# and look for specific notes using GDB/dejagnu, we rely on an
+# external tool, eu-unstrip, to verify if the corefile contains
+# build-ids.
+
+standard_testfile "normal.c"
+
+# This test is Linux x86_64 only.
+if { ![istarget *-*-linux*] } {
+    untested "$testfile.exp"
+    return -1
+}
+if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
+    untested "$testfile.exp"
+    return -1
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+    return -1
+}
+
+if { ![runto_main] } {
+    untested "could not run to main"
+    return -1
+}
+
+# First we need to generate a corefile.
+set corefilename "[standard_output_file gcore.test]"
+if { ![gdb_gcore_cmd "$corefilename" "save corefile"] } {
+    verbose -log "Could not save corefile"
+    untested "$testfile.exp"
+    return -1
+}
+
+# Determine if GDB dumped the mapping containing the build-id.  This
+# is done by invoking an external program (eu-unstrip).
+if { [catch "exec [gdb_find_eu-unstrip] -n --core $corefilename" output] == 0 } {
+    set line [lindex [split $output "\n"] 0]
+    set test "gcore dumped mapping with build-id"
+
+    verbose -log "First line of eu-unstrip: $line"
+
+    if { [regexp "^${hex}\\+${hex} \[a-f0-9\]+@${hex}.*[string_to_regexp $binfile]$" $line] } {
+	pass "$test"
+    } else {
+	fail "$test"
+    }
+} else {
+    verbose -log "Could not execute eu-unstrip program"
+    untested "$testfile.exp"
+}
diff --git a/gdb/testsuite/lib/future.exp b/gdb/testsuite/lib/future.exp
index a56cd019b4..122e652858 100644
--- a/gdb/testsuite/lib/future.exp
+++ b/gdb/testsuite/lib/future.exp
@@ -162,6 +162,16 @@  proc gdb_find_readelf {} {
     return $readelf
 }
 
+proc gdb_find_eu-unstrip {} {
+    global EU_UNSTRIP_FOR_TARGET
+    if [info exists EU_UNSTRIP_FOR_TARGET] {
+	set eu_unstrip $EU_UNSTRIP_FOR_TARGET
+    } else {
+	set eu_unstrip [transform eu-unstrip]
+    }
+    return $eu_unstrip
+}
+
 proc gdb_default_target_compile {source destfile type options} {
     global target_triplet
     global tool_root_dir