From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 6C382385AE4F for ; Fri, 24 Jun 2022 09:50:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6C382385AE4F Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-369-sLIi30oKPPOym9ELXw4kfg-1; Fri, 24 Jun 2022 05:50:52 -0400 X-MC-Unique: sLIi30oKPPOym9ELXw4kfg-1 Received: by mail-wm1-f70.google.com with SMTP id k16-20020a7bc310000000b0038e6cf00439so1261519wmj.0 for ; Fri, 24 Jun 2022 02:50:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=LHfYFfElzExdwwXgJEE58NQlX+ue78b9LYguBGerYts=; b=6GF6dOCzvcMhIpACPyh/2cg9H9q864uuPaQN3zpxTHBKaZ9SHhNtIf3WtnU1UGcbWP xqdRkB0sCU4FVqrO2DoNosEbjKkINxUVVEUQuyuF8SKnwPbIMaN/yfuy7TROpFdVtFHR 5kEJJqHW05thTR/v70a2UwU2lvLDL63fEYFzRspcgpvx1tOcQERp3UlsAt9IQWfKS1o7 UnXK7vHALqwi6DxOHRJuniUzXN/+3TGcmR1C61qPQq3Cghy2qjiNkAHMcYPLbPNtlnGF KKhasiB297TC1SIc72hwqtocY1dahAK8tMrEWWramzeNIdS2RMZHowFCcdy0uQoZ46uk PDyg== X-Gm-Message-State: AJIora8NfpAkpLkn2nqy0Iimsq8Q4suPQ0sB8VECsauhzCukHmPnYLsT 2h4oWtIY5+A1On4DTzt8G/TuBErB4oAPGwDU/+IOeLmUDhW+btaYKODGFlFkFTZtu3iwkcgrsQP ekCyDM636iqSMEgy6z+i85g== X-Received: by 2002:a1c:7903:0:b0:3a0:3936:b71f with SMTP id l3-20020a1c7903000000b003a03936b71fmr2852065wme.168.1656064251379; Fri, 24 Jun 2022 02:50:51 -0700 (PDT) X-Google-Smtp-Source: AGRyM1t2wneqNs4mivLaquca6LPoBfta/lWMiDCUN2IQASGCuuehe7BjX3pDuzJGnqLHhROt9morYw== X-Received: by 2002:a1c:7903:0:b0:3a0:3936:b71f with SMTP id l3-20020a1c7903000000b003a03936b71fmr2852021wme.168.1656064250893; Fri, 24 Jun 2022 02:50:50 -0700 (PDT) Received: from localhost ([195.213.152.79]) by smtp.gmail.com with ESMTPSA id t3-20020a0560001a4300b0021b9cc87aa9sm1815089wry.111.2022.06.24.02.50.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jun 2022 02:50:50 -0700 (PDT) From: Andrew Burgess To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Improve core file path detection & put cores in output dir In-Reply-To: <20220623183053.172430-2-pedro@palves.net> References: <20220623183053.172430-1-pedro@palves.net> <20220623183053.172430-2-pedro@palves.net> Date: Fri, 24 Jun 2022 10:50:49 +0100 Message-ID: <87y1xmbdae.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, 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 09:50:56 -0000 Pedro Alves writes: > After a testrun, I noticed that I have some kernel-produced cores for > testcase programs, under build/gdb/testsuite/, which shouldn't be > there: > > $ ls -1 testsuite/core.* > testsuite/core.annota1.1274351.nelson.1656004407 > testsuite/core.annota3.1288474.nelson.1656004414 > testsuite/core.exitsignal.1240674.nelson.1656004391 > > 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 > > so it's easy to tell which program produced the core from the core > file name. > > From above, we can tell that the corresponding testcases are > gdb.base/annota1.exp, gdb.base/annota3.exp and > gdb.base/exitsignal.exp. > > At least gdb.base/annota1.exp and gdb.base/annota3.exp have code in > them to delete the core file. However, that isn't working for me, > because said code only looks for cores named exactly either "core" or > "core.PID", and my core_pattern doesn't match that. > > Another issue I noticed, is that I have not been running > gdb.base/bigcore.exp, for a similar reason. I get: > > Program terminated with signal SIGABRT, Aborted. > The program no longer exists. > (gdb) PASS: gdb.base/bigcore.exp: signal SIGABRT > UNTESTED: gdb.base/bigcore.exp: can't generate a core file > > But I actually have a core file under the testcase's output dir: > > $ find . -name "core.*" > ./testsuite/outputs/gdb.base/bigcore/core.bigcore.2306705.nelson.1656005213 > $ > > This commit fixes these things, by adding find_core_file routine that > searches core files in a way that works with my pattern as well. This > then also adds a convenience remove_core routine as a wrapper around > find_core_file that removes the found core file. > > In addition, it changes some testcases that expect to have their > program dump core, to switch the inferior's cwd to the testcase's > output dir, so that the core is dumped there instead of in > build/gdb/testsuite/. Some testcases were already doing that, but not > all. The idea is that any core file dumped in build/gdb/testsuite/ is > an unexpected core file. The next patch will add a count of such > unexpected core files to gdb.sum. > > Another change is that the directory changing is now done with "set > cwd" instead of with "cd". "set cwd" only affects the inferior cwd, > while "cd" affects GDB's cwd too. By using "set cwd" instead of "cd", > if GDB dumps core in these testcases, the GDB core dump will still end > up in build/gdb/testsuite/, and can thus be detected as an unexpected > core. > > Change-Id: I45068f21ffd4814350aaa8a3cc65cad5e3107607 > --- > gdb/testsuite/gdb.base/annota1.exp | 18 +++--- > gdb/testsuite/gdb.base/annota3.exp | 18 +++--- > gdb/testsuite/gdb.base/bigcore.exp | 38 +++--------- > gdb/testsuite/gdb.base/exitsignal.exp | 11 +++- > gdb/testsuite/lib/gdb.exp | 86 ++++++++++++++++++++++++++- > 5 files changed, 117 insertions(+), 54 deletions(-) > > diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp > index ac6ad6478f4..42a261e5614 100644 > --- a/gdb/testsuite/gdb.base/annota1.exp > +++ b/gdb/testsuite/gdb.base/annota1.exp > @@ -385,6 +385,12 @@ gdb_test_multiple "display value" "set up display" { > } > } > > +# Get the core into the output directory. > +if {![is_remote host]} { > + gdb_test -prompt "$gdb_prompt$" \ > + "set cwd [file dirname $binfile]" "" \ > + "set inferior cwd to test directory" > +} > > # should ask query. Test annotate-query. > # we don't care about anything else here, only the query. > @@ -479,17 +485,7 @@ if [target_info exists gdb,nosignals] { > } > > # Check for production of a core file and remove it! > - > -set test "cleanup core file" > -if { [remote_file host exists core] } { > - remote_file host delete core > - pass "$test (removed)" > -} elseif { $pid != -1 && [remote_file host exists core.$pid] } { > - remote_file host delete core.$pid > - pass "$test (removed)" > -} else { > - pass "$test (not dumped)" > -} > +remove_core $pid > > proc thread_test {} { > global subdir srcdir testfile srcfile binfile > diff --git a/gdb/testsuite/gdb.base/annota3.exp b/gdb/testsuite/gdb.base/annota3.exp > index b747e03718c..4fae6f066ab 100644 > --- a/gdb/testsuite/gdb.base/annota3.exp > +++ b/gdb/testsuite/gdb.base/annota3.exp > @@ -273,6 +273,12 @@ gdb_expect_list "set up display" "$gdb_prompt$" { > "1: value = 7\r\n" > } > > +# Get the core into the output directory. > +if {![is_remote host]} { > + gdb_test -prompt "$gdb_prompt$" \ > + "set cwd [file dirname $binfile]" "" \ > + "set inferior cwd to test directory" > +} > > # should ask query. Test annotate-query. > # we don't care about anything else here, only the query. > @@ -379,17 +385,7 @@ if [target_info exists gdb,nosignals] { > > > # Check for production of a core file and remove it! > - > -set test "cleanup core file" > -if { [remote_file host exists core] } { > - remote_file host delete core > - pass "$test (removed)" > -} elseif { $pid != -1 && [remote_file host exists core.$pid] } { > - remote_file host delete core.$pid > - pass "$test (removed)" > -} else { > - pass "$test (not dumped)" > -} > +remove_core $pid > > # restore the original prompt for the rest of the testsuite > > diff --git a/gdb/testsuite/gdb.base/bigcore.exp b/gdb/testsuite/gdb.base/bigcore.exp > index a03a30eca32..13b70f7dd5d 100644 > --- a/gdb/testsuite/gdb.base/bigcore.exp > +++ b/gdb/testsuite/gdb.base/bigcore.exp > @@ -53,10 +53,7 @@ gdb_test_no_output "set print sevenbit-strings" > gdb_test_no_output "set width 0" > > # Get the core into the output directory. > -if {![is_remote host]} { > - gdb_test "cd [file dirname $corefile]" "Working directory .*" \ > - "cd to test directory" > -} > +set_inferior_cwd_to_output_dir > > if ![runto_main] then { > return 0 > @@ -116,17 +113,7 @@ gdb_test_no_output "set \$bytes_allocated = bytes_allocated" "save heap size" > # May 2003) create cores named "core.PID". > > # Save the process ID. Some systems dump the core into core.PID. > -set test "grab pid" > -gdb_test_multiple "info program" $test { > - -re "child process (\[0-9\]+).*$gdb_prompt $" { > - set inferior_pid $expect_out(1,string) > - pass $test > - } > - -re "$gdb_prompt $" { > - set inferior_pid unknown > - pass $test > - } > -} > +set inferior_pid [get_inferior_pid] > > # Dump core using SIGABRT > set oldtimeout $timeout > @@ -134,18 +121,11 @@ set timeout 600 > gdb_test "signal SIGABRT" "Program terminated with signal SIGABRT, .*" > set timeout $oldtimeout > > -# Find the corefile > -set file "" > -foreach pat [list core.${inferior_pid} ${testfile}.core core] { > - set names [glob -nocomplain [standard_output_file $pat]] > - if {[llength $names] == 1} { > - set file [lindex $names 0] > - remote_exec build "mv $file $corefile" > - break > - } > -} > - > -if { $file == "" } { > +# Find the corefile. > +set file [find_core_file $inferior_pid] > +if { $file != "" } { > + remote_exec build "mv $file $corefile" > +} else { > untested "can't generate a core file" > return 0 > } > @@ -186,9 +166,7 @@ if {! $core_ok} { > # Now load up that core file > > set test "load corefile" > -# We use [file tail] because gdb is still "cd"d to the > -# output directory. > -gdb_test_multiple "core [file tail $corefile]" "$test" { > +gdb_test_multiple "core $corefile" "$test" { > -re "A program is being debugged already. Kill it. .y or n. " { > send_gdb "y\n" > exp_continue > diff --git a/gdb/testsuite/gdb.base/exitsignal.exp b/gdb/testsuite/gdb.base/exitsignal.exp > index 27eb6c5d263..b139bac1be4 100644 > --- a/gdb/testsuite/gdb.base/exitsignal.exp > +++ b/gdb/testsuite/gdb.base/exitsignal.exp > @@ -32,11 +32,17 @@ if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { > return -1 > } > > -# Run to main > +# Run to main. But, before, change cwd to get the core into the > +# output directory. > +set_inferior_cwd_to_output_dir > + > if { ![runto_main] } { > return -1 > } > > +# Get the inferior's PID for later. > +set pid [get_inferior_pid] > + > # Print $_exitsignal. It should be void now, because nothing > # happened. > gdb_test "print \$_exitsignal" " = void" \ > @@ -53,6 +59,9 @@ gdb_test "continue" "Program received signal SIGSEGV.*" "trigger SIGSEGV" > gdb_test "continue" "Program terminated with signal SIGSEGV.*" \ > "program terminated with SIGSEGV" > > +# We don't need the core file, remove it. > +remove_core $pid > + > # Now, print $_exitsignal again. It should be 11 (SIGSEGV). > gdb_test "print \$_exitsignal" " = 11" \ > "\$_exitsignal is 11 (SIGSEGV) after SIGSEGV." > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 4bc5f4f144c..f3fbb8a6391 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -7368,6 +7368,88 @@ if {[info exists GDB_PARALLEL]} { > } > } > > +# Set the inferior's cwd to the output directory, in order to have it > +# dump core there. This must be called before the inferior is > +# started. > + > +proc set_inferior_cwd_to_output_dir {} { > + # Note this sets the inferior's cwd ("set cwd"), not GDB's ("cd"). > + # If GDB crashes, we want its core dump in gdb/testsuite/, not in > + # the testcase's dir, so we can detect the unexpected core at the > + # end of the test run. > + if {![is_remote host]} { > + set output_dir [standard_output_file ""] > + gdb_test_no_output "set cwd $output_dir" \ > + "set inferior cwd to test directory" > + } > +} > + > +# Get the inferior's PID. > + > +proc get_inferior_pid {} { > + set pid -1 > + gdb_test_multiple "inferior" "get inferior pid" { > + -re "process (\[0-9\]*).*$::gdb_prompt $" { > + set pid $expect_out(1,string) > + pass $gdb_test_name > + } > + } > + return $pid > +} > + > +# Find the kernel-produced core file dumped for the current testfile > +# program. PID was the inferior's pid, saved before the inferior > +# exited with a signal, or -1 if not known. If not on a remote host, > +# this assumes the core was generated in the output directory. > +# Returns the name of the core dump, or empty string is not found. > + > +proc find_core_file {pid} { > + # For non-remote hosts, since cores are assumed to be in the > + # output dir, which we control, we use a laxer "core.*" glob. For > + # remote hosts, as we don't know whether the dir is being reused > + # for parallel runs, we use stricter names with no globs. It is > + # not clear whether this is really important, but it preserves > + # status quo ante. > + set files {} > + if {![is_remote host]} { > + lappend files core.* > + } elseif {$pid != -1} { > + lappend files core.$pid > + } > + lappend files [list ${::testfile}.core core] > + > + foreach file $files { > + if {![is_remote host]} { > + set names [glob -nocomplain [standard_output_file $file]] > + if {[llength $names] == 1} { > + return [lindex $names 0] > + } > + } else { > + if {[remote_file host exists $file]} { > + return $file > + } > + } > + } > + return "" > +} > + > +# Check for production of a core file and remove it. PID is the > +# inferior's pid or -1 if not known. TEST is the test's message. > + > +proc remove_core {pid {test ""}} { > + if {$test == ""} { > + set test "cleanup core file" > + } > + > + set file [find_core_file $pid] > + if {$file != ""} { > + remote_file host delete $file > + pass "$test (removed)" > + } else { > + pass "$test (not dumped)" I wonder if 'not found' would be a more acurate description for this last pass? With a particular core file pattern it might be that there is a core file, we just can't find it, right? Otherwise, this looks good. Thanks, Andrew > + } > +} > + > proc core_find {binfile {deletefiles {}} {arg ""}} { > global objdir subdir > > @@ -7398,7 +7480,9 @@ proc core_find {binfile {deletefiles {}} {arg ""}} { > set found 1 > } > } > - # Check for "core.PID". > + # Check for "core.PID", "core.EXEC.PID.HOST.TIME", etc. It's fine > + # to use a glob here as we're looking inside a directory we > + # created. Also, this procedure only works on non-remote hosts. > if { $found == 0 } { > set names [glob -nocomplain -directory $coredir core.*] > if {[llength $names] == 1} { > -- > 2.36.0