From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) by sourceware.org (Postfix) with ESMTPS id 00481384D16A for ; Fri, 24 Jun 2022 11:17:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 00481384D16A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f47.google.com with SMTP id c130-20020a1c3588000000b0039c6fd897b4so3139518wma.4 for ; Fri, 24 Jun 2022 04:17:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:from:to:references:in-reply-to :content-transfer-encoding; bh=KccyGN6VnoXFAl7tBHKBMMBPmtRYcDEXyKutU1PwVnA=; b=1cbb8ca7rmqSW17oe/ySVmjzvw+d+TBMp7kptTqtrTPWHTu6N9DERiCT2qym7DEACy XqCNziQnPfN3BugW79B/8Z6umw58GCk/5HcT1fTtUoI5N4tLbjL+B1abQvUlxp2cNFbA XD51OWtFwD7q/f+7T7miCZQBL7ziPD/Ubt6KvTzl0Oa6T200wntW+33vCk+JZUtuH90x omW7VL0mrPJHzK1ePXHqHPBNNXEKhSfrEBri9jZuXIeQteYx8SXGNtBS+D4Eg5RPbIvg Q7cJna/sNx/TI9IWQ2cLyiErd4zEUM8aGXEn8NasfyoiSUA9f3whwm6mOSc0/NRPoqWC LcBg== X-Gm-Message-State: AJIora84ZrTsFfUFOpr2xjaEMYTJiIT3/ur9ZR7Gm4fuG014k+S+4+x5 hlPnCnNc3Xp0JfSBL4Qt+07XMp+CYsI= X-Google-Smtp-Source: AGRyM1vSJim0ReQ33BbvlM2rFFswaiDieq0xIEhnBFCaZVFOblmFD9SIydx7bjD0xzRp+OSSyesKeA== X-Received: by 2002:a05:600c:34c4:b0:39c:9236:4e9e with SMTP id d4-20020a05600c34c400b0039c92364e9emr3230691wmq.67.1656069439857; Fri, 24 Jun 2022 04:17:19 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id z2-20020a5d44c2000000b0021a3d94c7bdsm2057389wrr.28.2022.06.24.04.17.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Jun 2022 04:17:18 -0700 (PDT) Message-ID: <70aad7c9-2104-e105-67cf-93569dc4bda0@palves.net> Date: Fri, 24 Jun 2022 12:17:18 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH 2/2] Include count of unexpected core files in gdb.sum summary Content-Language: en-US From: Pedro Alves To: John Baldwin , gdb-patches@sourceware.org References: <20220623183053.172430-1-pedro@palves.net> <20220623183053.172430-3-pedro@palves.net> <2d2f3af3-a9d2-b84e-d0db-14585a6ae3db@palves.net> <9c1ff5a8-a08f-3994-f25e-416ad0b483bd@FreeBSD.org> <4b7bd1ec-ddb3-236d-e613-9fe71505ac9b@palves.net> In-Reply-To: <4b7bd1ec-ddb3-236d-e613-9fe71505ac9b@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Jun 2022 11:17:23 -0000 On 2022-06-24 12:13, Pedro Alves wrote: > On 2022-06-23 23:09, John Baldwin wrote: >> Thanks for testing on FreeBSD. :) >> >> I like the idea.  The only thing I see is that you might want to use a >> pattern of '*core*' to look for cores instead of 'core*.'.  On BSD's at >> least core files by default are named .core (e.g. gdb.core) and >> don't match the 'core*' glob.  If '*core*' has too many false positives >> perhaps you can use 'core* *.core' as the patterns to look for? >> > > *core* currently doesn't match anything else, so I think that would be fine. > > Here's the updated patch that I plan to merge. I included a comment > about the lax pattern. ... and ... I forgot to update the "rm" commands in the Makefile. Fixed in this version... Nth time's the charm? >From 5a728f1d057efcc3022d7b38c9539682d79f6a52 Mon Sep 17 00:00:00 2001 From: Pedro Alves 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. Change-Id: I190b8d41856d49ad143854b6e3e6ccd7caa04491 --- gdb/testsuite/Makefile.in | 9 ++++- gdb/testsuite/lib/dg-add-core-file-count.sh | 41 +++++++++++++++++++++ 2 files changed, 49 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..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 . + +# 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..PID", ".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 -- 2.36.0