From: Pedro Alves <pedro@palves.net>
Date: Thu, 23 Jun 2022 12:48:17 +0000 (+0100)
Subject: Improve core file path detection & put cores in output dir
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=c715d0732a4b580382b70aa32536fa02fff64d3e;p=binutils-gdb.git

Improve core file path detection & put cores in output dir

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 a find_core_file routine
that searches core files in a way that works with my core 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
---

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..975e861b0f6 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 if 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 found)"
+    }
+}
+
 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} {