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

[Andrew Burgess <aburgess@redhat.com>]

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.

Thanks,
Andrew

---

commit 6d6ec8321fcbbd12dc1544487367e2adefdc8ac9
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
    
    The problem 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.
    
    The check-build-ids command was defined in Python, and previously did
    all the checks, printing "PASS" if everything was fine, otherwise
    raising a Python exception.
    
    This has now changed to a show-build-ids Python command.  This
    extracts the build-ids from the inferior's objfiles and core file, and
    prints a table.  The .exp test script can then process this table, and
    decide if a difference is bad or not.
    
    When processing the show-build-ids table, if the build-id from the
    objfile object matches the build-id from the core file, then this is
    fine.
    
    If we have an objfile build-id and a different core file build-id,
    then this is an error, GDB managed to find the _wrong_ build-id.  Very
    bad.
    
    If we have an objfile build-id, and no core file build-id, then we use
    a new helper function added to lib/gdb.exp,
    expect_build_id_in_core_file.  This helper uses readelf to check where
    the build-id is located within the file.  If the build-id is within
    the first page of the ELF then we expect the build-id to appear in the
    core file, and GDB should be able to find it, in this case we have
    found a GDB bug, and the test will FAIL.
    
    If however, readelf says that the build-id is outside the first page
    of the ELF, then we don't expect GDB to be able to find the
    build-id (as it likely isn't in the core file).  In this case we don't
    count this as a test failure.
    
    I'm only aware of GNU/Linux forcing the first page of every mapped ELF
    into the generated core files, and so, I've made the part of the test
    that relies on this Linux only.
    
    This change should resolve the FAIL that Tom reported.

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..5c8ee7b2f0c 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:
                 continue
             p = pathlib.Path(o.filename).resolve()
             b = o.build_id
-            path_to_build_id[p] = b
+            path_to_build_id[p] = {"objfile": b}
+            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 {} {