Re: [PATCHv2 3/3] gdb/python: add Corefile.mapped_files method

[Andrew Burgess <aburgess@redhat.com>]

Tom de Vries <tdevries@suse.de> writes:

> On 10/8/25 11:29, Andrew Burgess wrote:
>> Andrew Burgess <aburgess@redhat.com> writes:
>> 
>>> Tom de Vries <tdevries@suse.de> writes:
>>>
>>>> On 10/7/25 16:38, Andrew Burgess wrote:
>>>>> Andrew Burgess <aburgess@redhat.com> writes:
>>>>>
>>>>>>
>>>>>> Thanks for the diagnostic help.  I'll work on improving the test to try
>>>>>> and catch these cases.  If that works, then we might be able to adopt
>>>>>> this to help with other tests.  Keeping this in mind, I'll add helper
>>>>>> functions to lib/gdb.exp.
>>>>>
>>>>> Hi Tom,
>>>>>
>>>>> At your leisure, could you check that the patch below resolves the FAIL
>>>>> you reported please.
>>>>>
>>>>
>>>> Hi Andrew,
>>>>
>>>> on Leap 15.6, yes.
>>>>
>>>> On Tumbleweed, unfortunately no.
>>>
>>> <snip>
>>>
>>>> (gdb) PASS: gdb.python/py-corefile.exp: test mapped files data: capture Python based mappings data
>>>> shell diff -s /data/vries/gdb/tumbleweed-20251005/build/gdb/testsuite/outputs/gdb.python/py-corefile/py-corefile-out-1.txt /data/vries/gdb/tumbleweed-20251005/build/gdb/testsuite/outputs/gdb.python/py-corefile/py-corefile-out-2.txt
>>>> Files /data/vries/gdb/tumbleweed-20251005/build/gdb/testsuite/outputs/gdb.python/py-corefile/py-corefile-out-1.txt and /data/vries/gdb/tumbleweed-20251005/build/gdb/testsuite/outputs/gdb.python/py-corefile/py-corefile-out-2.txt are identical
>>>> (gdb) PASS: gdb.python/py-corefile.exp: test mapped files data: diff input and output one
>>>> show-build-ids
>>>> Objfile Build-Id                          Core File Build-Id                        File Name
>>>> 91c6abb593a519ecd18e543eaac25f913999b230  91c6abb593a519ecd18e543eaac25f913999b230  /data/vries/gdb/tumbleweed-20251005/build/gdb/testsuite/outputs/gdb.python/py-corefile/py-corefile
>>>> Python Exception <class 'KeyError'>: 'corefile'
>>>> Error occurred in Python: 'corefile'
>>>
>>> It looks like there's an objfile discovered via
>>> Inferior.progspace.objfiles(), which doesn't correspond to a file mapped
>>> into the core file.  FYI, I only consider objfiles that are _actual_
>>> files, so ignore things like [vdso], etc, and I only consider objfiles
>>> with a build-id.  I figured that everything meeting this specification
>>> had to be something that would appear in the core file...
>>>
>>> The patch below will make the show-build-ids command a little more
>>> resilient, when a file is missing it'll print "missing" in the column
>>> now, rather than throwing the KeyError.
>>>
>>> My guess is we'll find the same file under different names maybe?
>> 
>> Just to follow up, I setup a Tumbleweed VM.  I was able to reproduce the
>> original issue you reported, but for me, the patch I posted[1] resolves
>> the issue.
>> 
>> If you apply the snippet from [2] this should avoid the KeyError
>> exception, and should allow us to debug the problem.
>> 
>
> Hi Andrew,
>
> thanks for following up on this.
>
> I put the gdb.log for Leap 15.6 and Tumbleweed side by side, and noticed 
> that the difference was presence of glibc debuginfo (in the form of 
> separate debug info objfiles).
>
> After doing "zypper install glibc-debuginfo" I could reproduce the FAIL 
> on Leap 15.6.  And after doing "dnf debuginfo-install glibc" in a fedora 
> rawhide container, it also reproduced there.
>
> This fixes it for me in all those 3 cases:
> ...
> diff --git a/gdb/testsuite/gdb.python/py-corefile.py b/gdb/testsuite/gdb.pyt
> hon/py-corefile.py
> index 01665ef0395..bc8082733bf 100644
> --- a/gdb/testsuite/gdb.python/py-corefile.py
> +++ b/gdb/testsuite/gdb.python/py-corefile.py
> @@ -106,7 +106,7 @@ class ShowBuildIds(gdb.Command):
>           longest_build_id = 18
>
>           for o in objfiles:
> -            if not o.is_file or o.build_id is None:
> +            if not o.is_file or o.build_id is None or o.owner is not
>   None:
>                   continue
>               p = pathlib.Path(o.filename).resolve()
>               b = o.build_id

Thanks for tracking that down.  I figured out why, despite having the
debug info installed, I wasn't hitting this issue at my end (my mistake,
not really important), and could then reproduce this.  I agree with your
fix.

Here's the patch I propose to push, this includes my original fix, plus
your Objfile.owner check.

Let me know what you think.

Thanks,
Andrew

---

commit 0f2a152fe641f82e074e8c995c456e27de4b9e5e
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Oct 7 15:24:01 2025 +0100

    gdb/testsuite: fix failure from gdb.python/py-corefile.exp
    
    After commit:
    
      commit f69c1d03c4d6c68ae3f90facd63245426c028047
      Date:   Mon Aug 25 16:48:22 2025 +0100
    
          gdb/python: add Corefile.mapped_files method
    
    Tom reported a failure:
    
      (gdb) check-build-ids
      Python Exception <class 'AssertionError'>: build-id mismatch for /lib64/libc.so.6
      Error occurred in Python: build-id mismatch for /lib64/libc.so.6
      (gdb) FAIL: gdb.python/py-corefile.exp: test mapped files data: check-build-ids
    
    The discussion thread can be found here:
    
      https://inbox.sourceware.org/gdb-patches/de21b43c-e3bd-4354-aace-bd3f50c1c64c@suse.de
    
    There are a couple of problems.
    
    First, there is an issue where some versions of the linker didn't
    place the build-id within the first page of an ELF.  As a result, the
    Linux kernel would not include the build-id in the generated core
    file, and so GDB cannot to find the build-id.
    
    In this patch I've added mitigation for this problem.
    
    I changed the 'check-build-ids' command (added via Python as part of
    the test) to 'show-build-ids'.  The updated command prints a table
    containing the build-ids for each objfile as found via GDB's
    Progspace.objfiles, and via the Corefile.mapped_files.  This table is
    then read by the TCL test script, and the build-ids are checked.  If
    there's a difference, then GDB can analyse the on disk ELF and work
    out if the difference is due to the linker issue mentioned above.  If
    it is, then the difference is ignored.
    
    In order to check for this linker issue I added a new helper proc to
    lib/gdb.exp, expect_build_id_in_core_file.
    
    The second problem with the original test is that it would consider
    separate debug files as files that should appear in the core file.
    There was Python code in the test that filtered the objfile list to
    disregard entries that would not appear in the core file, but this
    code needed extending to cover separate debug files.
    
    The final issue is that I'm only aware of GNU/Linux forcing the first
    page of every mapped ELF into the generated core files, so this test
    would likely fail on non-Linux systems.  I've made the part of the
    test that relies on this behaviour Linux only.
    
    This change should resolve the FAIL that Tom reported.  Giving Tom a
    Co-Author credit as he fixed the second issue, and helped a lot
    debugging the first issue.
    
    Co-Authored-By: Tom de Vries <tdevries@suse.de>

diff --git a/gdb/testsuite/gdb.python/py-corefile.exp b/gdb/testsuite/gdb.python/py-corefile.exp
index 3b57cc0e250..866b60a0dbe 100644
--- a/gdb/testsuite/gdb.python/py-corefile.exp
+++ b/gdb/testsuite/gdb.python/py-corefile.exp
@@ -211,8 +211,53 @@ with_test_prefix "test mapped files data" {
 	"Files \[^\r\n\]+-out-1.txt and \[^\r\n\]+-out-2.txt are identical" \
 	"diff input and output one"
 
-    # Check build-ids within the core file mapping data.
-    gdb_test "check-build-ids" "^PASS"
+    # Check build-ids within the core file mapping data.  I'm only
+    # aware of GNU/Linux placing the first page of each mapped ELF
+    # into the generated core file so that the build-id can be found.
+    if {[istarget *-*-linux*]} {
+	set results [list]
+	gdb_test_multiple "show-build-ids" "" {
+	    -re "^show-build-ids\r\n" {
+		exp_continue
+	    }
+	    -re "^Objfile Build-Id\\s+Core File Build-Id\\s+File Name\\s*\r\n" {
+		exp_continue
+	    }
+	    -re "^(\\S+)\\s+(\\S+)\\s+(\[^\r\n\]+)\r\n" {
+		set objfile_build_id $expect_out(1,string)
+		set core_file_build_id $expect_out(2,string)
+		set file_name $expect_out(3,string)
+		lappend results [list $objfile_build_id \
+				      $core_file_build_id \
+				      $file_name]
+		exp_continue
+	    }
+	    -re "^$gdb_prompt " {
+		pass $gdb_test_name
+	    }
+	}
+
+	set bad_count 0
+	foreach entry $results {
+	    set objfile_build_id [lindex $entry 0]
+	    set core_file_build_id [lindex $entry 1]
+	    set file_name [lindex $entry 2]
+	    if { $objfile_build_id ne $core_file_build_id } {
+		if { $core_file_build_id ne "None" } {
+		    verbose -log "Mismatched build-ids $objfile_build_id vs $core_file_build_id for $file_name"
+		    incr bad_count
+		} elseif { [expect_build_id_in_core_file $file_name] } {
+		    verbose -log "Failed to find build-id for $file_name"
+		    incr bad_count
+		} else {
+		    verbose -log "This build-id was likely not in the core file"
+		}
+	    }
+	}
+
+	gdb_assert { $bad_count == 0 } \
+	    "found expected build-ids in core file"
+    }
 
     # Check the is_main_executable flag in the mapping data.
     gdb_test "check-main-executable" "^PASS"
diff --git a/gdb/testsuite/gdb.python/py-corefile.py b/gdb/testsuite/gdb.python/py-corefile.py
index cffd037a23b..bfb1c820d5d 100644
--- a/gdb/testsuite/gdb.python/py-corefile.py
+++ b/gdb/testsuite/gdb.python/py-corefile.py
@@ -78,9 +78,23 @@ class InfoProcPyMappings(gdb.Command):
 InfoProcPyMappings()
 
 
-class CheckBuildIds(gdb.Command):
+# Assume that a core file is currently loaded.
+#
+# Look through all the objfiles for the current inferior, and record
+# any that have a build-id.
+#
+# Then look through the core file mapped files.  Look for entries that
+# correspond with the loaded objfiles.  For these matching entries,
+# capture the build-id extracted from the core file.
+#
+# Finally, print a table with the build-id from the objfile, the
+# build-id from the core file, and the file name.
+#
+# This is then processed from the test script to check the build-ids
+# match.
+class ShowBuildIds(gdb.Command):
     def __init__(self):
-        gdb.Command.__init__(self, "check-build-ids", gdb.COMMAND_DATA)
+        gdb.Command.__init__(self, "show-build-ids", gdb.COMMAND_DATA)
 
     def invoke(self, args, from_tty):
         inf = gdb.selected_inferior()
@@ -88,12 +102,17 @@ class CheckBuildIds(gdb.Command):
 
         path_to_build_id = {}
 
+        # Initial length based on column headings.
+        longest_build_id = 18
+
         for o in objfiles:
-            if not o.is_file or o.build_id is None:
+            if not o.is_file or o.build_id is None or o.owner is not None:
                 continue
             p = pathlib.Path(o.filename).resolve()
             b = o.build_id
-            path_to_build_id[p] = b
+            path_to_build_id[p] = {"objfile": b, "corefile": "missing"}
+            if len(b) > longest_build_id:
+                longest_build_id = len(b)
 
         count = 0
         core_mapped_files = inf.corefile.mapped_files()
@@ -101,16 +120,39 @@ class CheckBuildIds(gdb.Command):
             p = pathlib.Path(m.filename).resolve()
             b = m.build_id
 
+            if b is not None and len(b) > longest_build_id:
+                longest_build_id = len(b)
+
             if p in path_to_build_id:
-                count += 1
-                assert path_to_build_id[p] == b, "build-id mismatch for %s" % p
+                path_to_build_id[p]["corefile"] = b
 
-        assert count > 0, "no mapped files checked"
+        format_str = (
+            "%-" + str(longest_build_id) + "s  %-" + str(longest_build_id) + "s  %s"
+        )
 
-        print("PASS")
+        def make_title(string, length=0):
+            if length > 0:
+                padding_len = length - len(string)
+            else:
+                padding_len = 0
+
+            padding = " " * padding_len
+            style = gdb.Style("title")
+            return style.apply(string) + padding
+
+        print(
+            "%s  %s  %s"
+            % (
+                make_title("Objfile Build-Id", longest_build_id),
+                make_title("Core File Build-Id", longest_build_id),
+                make_title("File Name"),
+            )
+        )
+        for p, b in path_to_build_id.items():
+            print(format_str % (b["objfile"], b["corefile"], p))
 
 
-CheckBuildIds()
+ShowBuildIds()
 
 
 class CheckMainExec(gdb.Command):
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e07aab4cdd8..5dfcfefd7c1 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -11596,6 +11596,63 @@ proc section_get {exec section} {
     return $retval
 }
 
+# Return true if we expect the build-id from FILENAME to be included
+# in a core file.
+#
+# On GNU/Linux, when creating a core file, the kernel places the first
+# page of an ELF into the core file.  If the build-id is within that
+# page then GDB can find the build-id from the core file.
+#
+# This proc checks that the target is GNU/Linux, and then uses readelf
+# to find the offset of the build-id within the ELF.  If there is a
+# build-id, and it is within the first page, then return true.
+# Otherwise, return false.
+
+proc expect_build_id_in_core_file { filename } {
+    # I'm not sure if other kernels take care to add the first page of
+    # each ELF into the core file.  If they do then this test can be
+    # relaxed.
+    if {![istarget *-*-linux*]} {
+	return false
+    }
+
+    # Use readelf to find the build-id note in FILENAME.
+    set readelf_program [gdb_find_readelf]
+    set cmd [list $readelf_program -WS $filename | grep ".note.gnu.build-id"]
+    set res [catch {exec {*}$cmd} output]
+    verbose -log "running: $cmd"
+    verbose -log "result: $res"
+    verbose -log "output: $output"
+    if { $res != 0 } {
+	return false
+    }
+
+    # Extract the OFFSET from the readelf output.
+    set res [regexp {NOTE[ \t]+([0-9a-f]+)[ \t]+([0-9a-f]+)} \
+		 $output dummy addr offset]
+    if { $res != 1 } {
+	return false
+    }
+
+    # Convert OFFSET to decimal.
+    set offset [expr 0x$offset + 0]
+
+    # Now figure out the page size.  This should be fine for Linux
+    # hosts, see the istarget check above.
+    if {[catch {exec getconf PAGESIZE} page_size]} {
+	# Failed to fetch page size.
+	return false
+    }
+
+    # If the build-id is within the first page, then we expect the
+    # kernel to include it in the core file.  There is actually a
+    # kernel setting (see coredump_filter) that could prevent this,
+    # but the default behaviour is to include the first page of the
+    # ELF, so for now, we just assume this is on.
+    verbose -log "Page size is $page_size, Offset is $offset"
+    return [expr $offset < $page_size]
+}
+
 # Return 1 if the compiler supports __builtin_trap, else return 0.
 
 gdb_caching_proc have_builtin_trap {} {