From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: [PATCHv2] Fix the crash at the end of the runtest
Date: Thu, 22 Jul 2021 17:53:34 +0100 [thread overview]
Message-ID: <20210722165334.GE1872618@embecosm.com> (raw)
In-Reply-To: <20210722132020.GD1872618@embecosm.com>
Thanks for the feedback. Below is the patch I intend to push first
thing tomorrow unless anyone objects.
Thanks,
Andrew
---
commit 41424acf01830d3cafb838a7d865985182edf47a
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Thu Jul 22 14:07:15 2021 +0100
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.
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e79e0622f9d..aeb8585bab9 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
}
next prev parent reply other threads:[~2021-07-22 16:53 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-22 11:10 [PATCH] " Bernd Edlinger
2021-07-22 12:44 ` Andrew Burgess
2021-07-22 12:59 ` Bernd Edlinger
2021-07-22 13:20 ` Andrew Burgess
2021-07-22 15:14 ` Simon Marchi via Gdb-patches
2021-07-22 15:16 ` Bernd Edlinger
2021-07-22 16:53 ` Andrew Burgess [this message]
2021-07-22 19:24 ` [PATCHv2] " Simon Marchi via Gdb-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210722165334.GE1872618@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=bernd.edlinger@hotmail.de \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox