[PATCH 2/2] Include count of unexpected core files in gdb.sum summary

[Andrew Burgess]

Pedro Alves <pedro@palves.net> writes:

> On 2022-06-23 19:30, Pedro Alves wrote:
>
>> +# Add our line to the summary line.
>> +sed "/=== gdb Summary ===/{
>> +n
>> +a\
>> +# of unexpected core files	$cores
>> +}" -i gdb.sum
>
> I knew this -i wasn't portable as is, but then forgot to fix it before posting.  I now tried
> the script on a FreeBSD machine on the gcc compile farm, and tweaked this so it works with BSD
> seds too.  That sed invocation now looks like:
>
>  # Add our line to the summary line.
>  sed -i'' -e "/=== gdb Summary ===/{
>  n
>  a\\
>  # of unexpected core files     $cores
>  }" gdb.sum
>
> If -i doesn't work elsewhere, we can just do:
>
>  sed .... < gdb.sum > gdb.sum.tmp
>  mv gdb.sum.tmp gdb.sum
>
> Here's the updated patch.
>
> From 48d48ca9ddeaee581b229a13ba40b3dd5534568e Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Wed, 22 Jun 2022 20:33:01 +0100
> Subject: [PATCH 2/2] 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.

LGTM.

Thanks,
Andrew

>
> Change-Id: I190b8d41856d49ad143854b6e3e6ccd7caa04491
> ---
>  gdb/testsuite/Makefile.in                   |  9 ++++-
>  gdb/testsuite/lib/dg-add-core-file-count.sh | 40 +++++++++++++++++++++
>  2 files changed, 48 insertions(+), 1 deletion(-)
>  create mode 100755 gdb/testsuite/lib/dg-add-core-file-count.sh
>
> diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
> index 790b9e022cc..6a020e05d88 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..abeb0b1c711
> --- /dev/null
> +++ b/gdb/testsuite/lib/dg-add-core-file-count.sh
> @@ -0,0 +1,40 @@
> +#!/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.
> +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 line.
> +sed -i'' -e "/=== gdb Summary ===/{
> +n
> +a\\
> +# of unexpected core files	$cores
> +}" gdb.sum
> -- 
> 2.36.0