From 44710bb280e0cbb1d42367f82b35e095e9cded4b Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 22 Jul 2021 14:07:15 +0100 Subject: [PATCH] gdb/testsuite: don't error when trying to unset last_spawn_tty_name In spawn_capture_tty_name (lib/gdb.exp) we either set or unset last_spawn_tty_name depending on whether spawn_out(slave,name) exists or not. One situation that might cause spawn_out(slave,name) to not exists is if the spawn function is called with the argument -leaveopen, which is how it is called when processes are created as part of a pipeline, the created process has no tty, instead its output is written to a file descriptor. If a pipeline is created consisting of multiple processes then there will be multiple sequential calls to spawn, all using -leaveopen. The first of these calls is fine, spawn_out(slave,name) is not set, and so in spawn_capture_tty_name we unset last_spawn_tty_name. However, on the second call to spawn, spawn_out(slave,name) is still not set and so in spawn_capture_tty_name we again try to unset last_spawn_tty_name, this now throws an error (as last_spawn_tty_name is already unset). Fix this issue by using -nocomplain with the call to unset in spawn_capture_tty_name. Before this commit I was seeing gdb.base/gnu-debugdata.exp report 1 pass, and 1 unsupported test. After this commit I now see 16 passes from this test script. I have also improved the code that used to do this: if { [info exists spawn_out] } { set ::last_spawn_tty_name $spawn_out(slave,name) } else { ... } The problem here is that we check for the existence of spawn_out, and then unconditionally read spawn_out(slave,name). A situation could arise where some other element of spawn_out is set, e.g. spawn_out(foo), in which case we would enter the if block and try to read a non-existent variable. After this commit we now check specifically for spawn_out(slave,name). Finally, it is worth noting that before this issue was fixed runtest itself, or rather the expect process behind runtest, would segfault while exiting. I haven't looked at all into what the problem is here that caused expect to crash, as fixing the bug in GDB's testing scripts made the segfault go away. --- gdb/testsuite/lib/gdb.exp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 16c46647101..2f95989440a 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -2029,10 +2029,20 @@ proc gdb_file_cmd { arg } { proc spawn_capture_tty_name { args } { set result [uplevel builtin_spawn $args] upvar spawn_out spawn_out - if { [info exists spawn_out] } { + if { [info exists spawn_out(slave,name)] } { set ::last_spawn_tty_name $spawn_out(slave,name) } else { - unset ::last_spawn_tty_name + # If a process is spawned as part of a pipe line (e.g. passing + # -leaveopen to the spawn proc) then the spawned process is no + # assigned a tty and spawn_out(slave,name) will not be set. + # In that case we want to ensure that last_spawn_tty_name is + # not set. + # + # If the previous process spawned was also not assigned a tty + # (e.g. multiple processed chained in a pipeline) then + # last_spawn_tty_name will already be unset, so, if we don't + # use -nocomplain here we would otherwise get an error. + unset -nocomplain ::last_spawn_tty_name } return $result } -- 2.30.2