[binutils-gdb] Include count of unexpected core files in gdb.sum summary

[binutils-gdb] Include count of unexpected core files in gdb.sum summary

[Pedro Alves]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e83907ff5ffbac3d0224d31ee99e6dc056205f39

commit e83907ff5ffbac3d0224d31ee99e6dc056205f39
Author: Pedro Alves <pedro@palves.net>
Date:   Wed Jun 22 20:33:01 2022 +0100

    Include count of unexpected core files in gdb.sum summary
    
    If GDB, GDBserver, a testcase program, Valgrind, etc. unexpectedly
    crash while running the GDB testsuite, and you've setup your machine
    such that core files are dumped in the current directory instead of
    being shoved somewhere by abrt, apport, or similar (as you should for
    proper GDB testing), you'll end up with an unexpected core file in the
    $build/gdb/testsuite/ directory.
    
    It can happen that GDB, GDBserver, etc. even crashes _after_ gdb_exit,
    during teardown, and thus such a crash won't be noticed by looking at
    the gdb.sum file at all.  This commit aims at improving that, by
    including a new "unexpected core files" line in the testrun summary.
    
    For example, here's what I get on x86-64 Ubuntu 20.04, with this
    patch:
    
                     === gdb Summary ===
    
     # of unexpected core files      12          << new info
     # of expected passes            107557
     # of unexpected failures        35
     # of expected failures          77
     # of unknown successes          2
     # of known failures             114
     # of untested testcases         31
     # of unsupported tests          139
    
    I have my core pattern setup like this:
    
     $ cat /proc/sys/kernel/core_pattern
     core.%e.%p.%h.%t
    
    That's:
    
     %e: executable filename
     %p: pid
     %h: hostname
     %t: UNIX time of dump
    
    and so I get these core files:
    
     $ ls -1 testsuite/core.*
     testsuite/core.connect-with-no.216191.nelson.1656002431
     testsuite/core.connect-with-no.217729.nelson.1656002431
     testsuite/core.gdb.194247.nelson.1656002423
     testsuite/core.gdb.226014.nelson.1656002435
     testsuite/core.gdb.232078.nelson.1656002438
     testsuite/core.gdb.352268.nelson.1656002441
     testsuite/core.gdb.4152093.nelson.1656002337
     testsuite/core.gdb.4154515.nelson.1656002338
     testsuite/core.gdb.4156668.nelson.1656002339
     testsuite/core.gdb.4158871.nelson.1656002341
     testsuite/core.gdb.468495.nelson.1656002444
     testsuite/core.vgdb.4192247.nelson.1656002366
    
    where we can see that GDB crashed a number of times, but also
    Valgrind's vgdb, and a couple testcase programs.  Neither of which is
    good.
    
    If your core_pattern is just "core" (but why??), then I guess that you
    may end up with just a single core file in testsuite/.  Still, that is
    one core file too many.
    
    Above, we see a couple cores for "connect-with-no", which are the
    result of gdb.server/connect-with-no-symbol-file.exp.  This is a case
    mentioned above -- while the program crashed, that happens during
    testcase teardown, and it goes unnoticed (without this commit) by
    gdb.sum results.  Vis:
    
     $ make check TESTS="gdb.server/connect-with-no-symbol-file.exp"
     ...
                     === gdb Summary ===
    
     # of unexpected core files      2
     # of expected passes            8
    
     ...
     $
    
    The tests fully passed, but still the testcase program crashed
    somehow:
    
     $ ls -1 testsuite/core.*
     testsuite/core.connect-with-no.941561.nelson.1656003317
     testsuite/core.connect-with-no.941682.nelson.1656003317
    
    Against --target_board=native-extended-gdbserver it's even worse.  I
    get:
    
     # of unexpected core files      26
    
    and note that when GDBserver hits an assertion failure, it exits with
    error, instead of crashing with SIGABRT.  I think that should be
    changed, at least on development builds, but that would be for another
    patch.  After such patch, I suspect the number of unexpected cores
    will be higher, as there are likely teardown GDBserver assertions that
    we're not noticing.
    
    I decided to put this new info in the "gdb Summary" section, as that's
    a place people already are used to looking at, either when looking at
    the tail of gdb.sum, or when diffing gdb.sum files, and we've already
    extended this section before, to include the count of DUPLICATE and
    PATH problems, so there's precedent.
    
    Implementation-wise, the new line is appended after DejaGnu is
    finished, with a shell script that is invoked by the Makefile.  It is
    done this way so that serial and parallel testing work the same way.
    My initial cut at an implementation was in TCL, straight in
    testsuite/lib/check-test-names.exp, where DUPLICATES and PATH are
    handled, like so:
    
     @@ -148,6 +159,10 @@ namespace eval ::CheckTestNames {
                 $counts(paths,$which)
             maybe_show_count "# of duplicate test names\t" \
                 $counts(duplicates,$which)
     +
     +       set cores [glob -nocomplain -directory $::objdir core*]
     +       maybe_show_count "# of unexpected core files\t" \
     +           [llength $cores]
          }
    
    But that would only work for serial testing, as in parallel testing,
    the final gdb.sum is generated by aggregating the results of all the
    individual gdb.sum files, and dg-extract-results.sh doesn't know about
    our new summary line.  And I don't think that dg-extract-results.sh
    should be taught about it, since the count of core files is not
    something that we want to count many times, once per testcase, and
    then add up the subcounts at the end.  Every time we count the core
    files, we're already counting the final count.
    
    I considered using the Tcl implementation in serial mode, and the
    script approach for parallel testing, but that has the obvious
    downside of implementing and maintaining the same thing twice.  In the
    end, I settled on the script approach for serial mode too, which
    requires making the "check-single" rule print the tail end of the
    gdb.sum file, with a side effect being that if you look at the
    terminal after a run (instead of at the gdb.sum file), you'll see the
    "gdb Summary" section twice, once without the unexpected core lines
    printed, and then another with.  IMO, this isn't an issue; when
    testing in parallel mode, if you look at the terminal after "make -jN
    check", you'll also see multiple "gdb Summary" sections printed.
    
    Change-Id: I190b8d41856d49ad143854b6e3e6ccd7caa04491

Diff:
---
 gdb/testsuite/Makefile.in                   |  9 ++++++-
 gdb/testsuite/lib/dg-add-core-file-count.sh | 41 +++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index 790b9e022cc..87ba522c9e0 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -208,7 +208,12 @@ check-gdb.%:
 	$(MAKE) check TESTS="gdb.$*/*.exp"
 
 check-single:
-	$(DO_RUNTEST) $(RUNTESTFLAGS) $(expanded_tests_or_none) $(TIMESTAMP)
+	-rm -f *core*
+	$(DO_RUNTEST) $(RUNTESTFLAGS) $(expanded_tests_or_none) $(TIMESTAMP); \
+	result=$$?; \
+	$(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
+	sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
+	exit $$result
 
 check-single-racy:
 	-rm -rf cache racy_outputs temp
@@ -231,6 +236,7 @@ check-single-racy:
 	sed -n '/=== gdb Summary ===/,$$ p' racy.sum
 
 check-parallel:
+	-rm -f *core*
 	-rm -rf cache outputs temp
 	$(MAKE) -k do-check-parallel; \
 	result=$$?; \
@@ -238,6 +244,7 @@ check-parallel:
 	  `find outputs -name gdb.sum -print` > gdb.sum; \
 	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
 	  `find outputs -name gdb.log -print` > gdb.log; \
+	$(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
 	sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
 	exit $$result
 
diff --git a/gdb/testsuite/lib/dg-add-core-file-count.sh b/gdb/testsuite/lib/dg-add-core-file-count.sh
new file mode 100755
index 00000000000..702be062e86
--- /dev/null
+++ b/gdb/testsuite/lib/dg-add-core-file-count.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+# Copyright (C) 2022 Free Software Foundation, Inc.
+
+# This file is part of GDB.
+
+# 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/>.
+
+# Count number of core files in the current directory and if non-zero,
+# add a line to the gdb.sum file.  This scripts assumes it is run from
+# the build/gdb/testsuite/ directory.  It is normally invoked by the
+# Makefile.
+
+# Count core files portably, using POSIX compliant shell, avoiding ls,
+# find, wc, etc.  Spawning a subshell isn't strictly needed, but it's
+# clearer.  The "*core*" pattern is this lax in order to find all of
+# "core", "core.PID", "core.<program>.PID", "<program>.core", etc.
+cores=$(set -- *core*; [ $# -eq 1 -a ! -e "$1" ] && shift; echo $#)
+
+# If no cores found, then don't add our summary line.
+if [ "$cores" -eq "0" ]; then
+    exit
+fi
+
+# Add our line to the summary.
+sed -i'' -e "/=== gdb Summary ===/{
+n
+a\\
+# of unexpected core files	$cores
+}" gdb.sum