[RFC][gdb/testsuite] Add gdb.base/valgrind-infcall-2.exp

[RFC][gdb/testsuite] Add gdb.base/valgrind-infcall-2.exp

[Tom de Vries]

Hi,

In commit ee3c5f8968 "Fix GDB crash when registers cannot be modified", we
fix a GDB crash:
...
$ valgrind /usr/bin/sleep 10000
==31595== Memcheck, a memory error detector
==31595== Command: /usr/bin/sleep 10000
==31595==
$ gdb /usr/bin/sleep
(gdb) target remote | vgdb --pid=31595
Remote debugging using | vgdb --pid=31595
  ...
$hex in __GI___nanosleep () at nanosleep.c:27
27	  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
(gdb) p printf ("bla")
terminate called after throwing an instance of 'gdb_exception_error'
Aborted (core dumped)
...

This patch adds a test-case for it.

Unfortunately, I was not able to trigger the error condition using a regular
vgdb_start, so I've added a parameter active_at_startup, and when set to 0
this causes valgrind to be started without --vgdb-error=0.

Tested on x86_64-linux.

Tested with the commit mentioned above reverted, resulting in:
...
(gdb) p printf ("bla")^M
terminate called after throwing an instance of 'gdb_exception_error'^M
ERROR: GDB process no longer exists
GDB process exited with wait status 6152 exp10 0 0 CHILDKILLED SIGABRT SIGABRT
UNRESOLVED: gdb.base/valgrind-infcall-2.exp: do printf
...

Any comments?

Thanks,
- Tom

[gdb/testsuite] Add gdb.base/valgrind-infcall-2.exp

gdb/testsuite/ChangeLog:

2020-04-12  Tom de Vries  <tdevries@suse.de>

	* gdb.base/valgrind-infcall-2.c: New test.
	* gdb.base/valgrind-infcall-2.exp: New file.
	* lib/valgrind.exp (vgdb_start): Add and handle active_at_startup.

---
 gdb/testsuite/gdb.base/valgrind-infcall-2.c   | 25 +++++++++
 gdb/testsuite/gdb.base/valgrind-infcall-2.exp | 75 +++++++++++++++++++++++++++
 gdb/testsuite/lib/valgrind.exp                | 38 +++++++++-----
 3 files changed, 125 insertions(+), 13 deletions(-)

diff --git a/gdb/testsuite/gdb.base/valgrind-infcall-2.c b/gdb/testsuite/gdb.base/valgrind-infcall-2.c
new file mode 100644
index 0000000000..bf2f8cd1c4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/valgrind-infcall-2.c
@@ -0,0 +1,25 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   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/>.  */
+
+#include <unistd.h>
+
+int
+main (void)
+{
+  sleep (60);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/valgrind-infcall-2.exp b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
new file mode 100644
index 0000000000..a3983f7564
--- /dev/null
+++ b/gdb/testsuite/gdb.base/valgrind-infcall-2.exp
@@ -0,0 +1,75 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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/>.
+
+# This test-case tests the scenario for the crash fixed by commit ee3c5f8968
+# "Fix GDB crash when registers cannot be modified":
+# $ valgrind /usr/bin/sleep 10000
+# ==31595== Memcheck, a memory error detector
+# ==31595== Command: /usr/bin/sleep 10000
+# ==31595==
+# $ gdb /usr/bin/sleep
+# (gdb) target remote | vgdb --pid=31595
+# Remote debugging using | vgdb --pid=31595
+#   ...
+# $hex in __GI___nanosleep () at nanosleep.c:27
+# 27	  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+# (gdb) p printf ("bla")
+# terminate called after throwing an instance of 'gdb_exception_error'
+# Aborted (core dumped)
+
+load_lib valgrind.exp
+
+if [is_remote target] {
+    # The test always runs locally.
+    return 0
+}
+
+standard_testfile .c
+if {[build_executable $testfile.exp $testfile $srcfile {debug}] == -1} {
+    return -1
+}
+
+set active_at_startup 0
+if { [vgdb_start $active_at_startup] == -1 } {
+    return -1
+}
+
+# Determine whether we're at nanosleep.
+gdb_test_multiple "bt 1" "do printf" {
+    -re -wrap "nanosleep.*" {
+	# If gdb doesn't crash, we get something like:
+	# (gdb) p printf ("bla")
+	# Could not write register "rdi"; remote failure reply 'E.
+	# ERROR changing register rdi regno 5
+	# gdb commands changing registers (pc, sp, ...) (e.g. 'jump',
+	# set pc, calling from gdb a function in the debugged process, ...)
+	# can only be accepted if the thread is VgTs_Runnable or VgTs_Yielding
+	# state
+	# Thread status is VgTs_WaitSys
+	# '
+	# (gdb)
+	gdb_test "p printf (\"bla\")" \
+	    "can only be accepted if the thread is .*" \
+	    $gdb_test_name
+    }
+    -re -wrap "" {
+	# For some reason the error condition does not trigger if we're not
+	# at nanosleep at the point that we're connecting to vgdb.  See also
+	# comment at "exec sleep 1" in vgdb_start.
+	unsupported $gdb_test_name
+    }
+}
+
+vgdb_stop
diff --git a/gdb/testsuite/lib/valgrind.exp b/gdb/testsuite/lib/valgrind.exp
index 619cf5854e..7bbcb21053 100644
--- a/gdb/testsuite/lib/valgrind.exp
+++ b/gdb/testsuite/lib/valgrind.exp
@@ -19,12 +19,19 @@
 # Start a vgdb server, and connect gdb to it.  Return 0 on success, and -1 on
 # error.
 #
-proc vgdb_start { } {
+proc vgdb_start { {active_at_startup 1} } {
     global binfile use_gdb_stub board testfile
     global valgrind_spawn_id gdb_spawn_id
+    global decimal
 
     set test "spawn valgrind"
-    set cmd "valgrind --vgdb-error=0 $binfile"
+    set cmd_list [list]
+    lappend cmd_list "valgrind"
+    if { $active_at_startup } {
+	lappend cmd_list "--vgdb-error=0"
+    }
+    lappend cmd_list $binfile
+    set cmd [join $cmd_list]
     set res [remote_spawn host $cmd]
     if { $res < 0 || $res == "" } {
 	verbose -log "Spawning $cmd failed."
@@ -42,7 +49,8 @@ proc vgdb_start { } {
     set test "valgrind started"
     # The trailing '.' differs for different memcheck versions.
     gdb_test_multiple "" $test {
-	-re "Memcheck, a memory error detector\\.?\r\n" {
+	-re "==($decimal)== Memcheck, a memory error detector\\.?\r\n" {
+	    set vgdbpid $expect_out(1,string)
 	    pass $test
 	}
 	-re "valgrind: failed to start tool 'memcheck' for platform '.*': No such file or directory" {
@@ -66,14 +74,6 @@ proc vgdb_start { } {
 	}
     }
 
-    set test "vgdb prompt"
-    gdb_test_multiple "" $test {
-	-re "  (target remote | \[^\r\n\]*/vgdb \[^\r\n\]*)\r\n" {
-	    set vgdbcmd $expect_out(1,string)
-	    pass $test
-	}
-    }
-
     # Do not kill valgrind.
     set valgrind_spawn_id [board_info host fileid]
     unset gdb_spawn_id
@@ -87,10 +87,22 @@ proc vgdb_start { } {
     # gdbserver and connect to it.
     gdb_test "disconnect" ".*"
 
-    gdb_test "$vgdbcmd" " in \\.?_start .*" "target remote for vgdb"
+    set vgdbcmd "target remote | vgdb --pid=$vgdbpid"
+    if { $active_at_startup } {
+	gdb_test "$vgdbcmd" " in \\.?_start .*" "target remote for vgdb"
+    } else {
+	# Let $binfile run a bit before attaching.  This is a bit of a hack,
+	# in that it lets test-case valgrind-infcall-2.exp run to the point of
+	# nanosleep, which seems to be required to trigger the error condition.
+	# So, without this, we hit
+	# "UNSUPPORTED: gdb.base/valgrind-infcall-2.exp: do printf".
+	exec sleep 1
 
-    gdb_test "monitor v.set gdb_output" "valgrind output will go to gdb.*"
+	# Connect to vgdb.  Don't expect to be anywhere in particular.
+	gdb_test "$vgdbcmd" "" "target remote for vgdb"
+    }
 
+    gdb_test "monitor v.set gdb_output" "valgrind output will go to gdb.*"
     return 0
 }
 

Re: [RFC][gdb/testsuite] Add gdb.base/valgrind-infcall-2.exp

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> In commit ee3c5f8968 "Fix GDB crash when registers cannot be modified", we
Tom> fix a GDB crash:
...
Tom> Unfortunately, I was not able to trigger the error condition using a regular
Tom> vgdb_start, so I've added a parameter active_at_startup, and when set to 0
Tom> this causes valgrind to be started without --vgdb-error=0.

This seems fine to me.  Thanks for writing this test.

Tom