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

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[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  <[hidden email]>

        * 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
 }
 
Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-2
>>>>> "Tom" == Tom de Vries <[hidden email]> 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