From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id C0C84398E470 for ; Wed, 9 Jun 2021 15:43:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C0C84398E470 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 159Fh0ot017074 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 9 Jun 2021 11:43:04 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 159Fh0ot017074 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 207441E01F; Wed, 9 Jun 2021 11:43:00 -0400 (EDT) Subject: Re: [PATCH] gdb/testsuite: capture GDB tty name in default_gdb_spawn To: Tom Tromey , Simon Marchi via Gdb-patches References: <20210608212126.1673188-1-simon.marchi@polymtl.ca> <874ke7jcrv.fsf@tromey.com> <269336aa-5500-c03e-9e00-81279c24e5be@polymtl.ca> From: Simon Marchi Message-ID: Date: Wed, 9 Jun 2021 11:42:59 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <269336aa-5500-c03e-9e00-81279c24e5be@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 9 Jun 2021 15:43:00 +0000 X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Jun 2021 15:43:07 -0000 On 2021-06-09 11:15 a.m., Simon Marchi via Gdb-patches wrote: > On 2021-06-09 11:09 a.m., Tom Tromey wrote: >> Simon> + # Capture GDB's TTY name, so we can save it below. >> Simon> + rename spawn builtin_spawn >> Simon> + rename spawn_capture_tty_name spawn >> Simon> + set code [catch {uplevel 1 { >> Simon> + remote_spawn host "$::GDB $::INTERNAL_GDBFLAGS $::GDBFLAGS [host_info gdb_opts]" >> Simon> + }} res] >> Simon> + rename spawn spawn_capture_tty_name >> Simon> + rename builtin_spawn spawn >> Simon> + >> Simon> + # If remote_spawn threw an error, propagate it. >> Simon> + if { $code == 1 } { >> Simon> + return -code $code -errorinfo $::errorInfo -errorcode $::errorCode $res >> Simon> + } >> >> It would be simpler to just always rename+wrap spawn, and then only >> stash the result in this particular case. However, it's also fine this >> way. > > Yeah, I wanted to avoid overriding it all the time, to avoid bad > suprises. Although if we don't introduce bugs, it should be fine ;). Here's the version that overrides all the time. >From abc7ae3e7cb93aa723db58beb07a677686831466 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 8 Jun 2021 17:21:26 -0400 Subject: [PATCH] gdb/testsuite: capture GDB tty name in default_gdb_spawn The TUI test gdb.tui/empty.exp fails with the native-extended-gdbserver board, and takes a very long time to run due to numerous timeouts. The symptom, when looking at the logs, is that the TUI windows that we expect to be resized are not resized. Digging down, I found that GDB didn't receive any SIGWINCH that should have resulted from Term::resize's stty calls. The reason for this is: - The native-extended-gdbserver overrides gdb_start to first start GDB, then start GDBserver with --multi, then connect GDB to GDBserver. This means that two TCL "spawn"s are done, one for GDB and one for GDBserver. - The TUI test framework wants to know GDB's TTY name, so it can pass it to stty, to fake terminal resizes. To do so, it overrides the spawn built-in proc to capture the tty name from the internals of the built-in proc. It saves the TTY name to the gdb_spawn_name global variable. - Because the native-extended-gdbserver boards starts both GDB and GDBserver, the final value of gdb_spawn_name is the name of GDBserver's TTY. - When the TUI test framework attempts to resize GDB's terminal, it in fact resizes GDBserver's terminal. So obviously, GDB doesn't get the SIGWINCH, and we don't get the expected TUI redraw. Fix that by moving the hack to lib/gdb.exp, overriding the builtin spawn all the time. The override saves the TTY name in the last_spawn_tty_name global. The default_gdb_spawn proc then saves it in the gdb_tty_name global. This way, we specifically capture GDB's TTY name in gdb_tty_name, not the TTY name of other spawned processes. Remove tuiterm_env_init and tuiterm_env_finish, since they are now empty. In turn, the gdb_finish_hooks mechanism is now unused, remove it as well. It would be easy to add them back if needed. gdb/ChangeLog: * lib/gdb.exp (default_gdb_exit): Unset gdb_tty_name. (spawn_capture_tty_name): New, override builtin spawn. (default_gdb_spawn): Capture GDB's TTY name. * lib/tuiterm.exp (tuiterm_spawn): Remove. (tuiterm_env_init, tuiterm_env_finish): Remove spawn override. (Term) : Use new variable name. (tuiterm_env_init, tuiterm_env_finish): Remove. (tuiterm_env): Don't call tuiterm_env_init and register tuiterm_env_finish in gdb_finish_hooks. (gdb_finish_hooks): Remove. (gdb_finish): Don't call finish hooks. Change-Id: Ia5ab74184a52a996416022308f8d0cc523355a78 --- gdb/testsuite/lib/gdb.exp | 31 ++++++++++++++++++++++++------- gdb/testsuite/lib/tuiterm.exp | 35 ++--------------------------------- 2 files changed, 26 insertions(+), 40 deletions(-) diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 8469ec9801c0..d8c684c72389 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -1909,6 +1909,7 @@ proc default_gdb_exit {} { remote_close host } unset gdb_spawn_id + unset ::gdb_tty_name unset inferior_spawn_id } @@ -2037,6 +2038,28 @@ proc gdb_file_cmd { arg } { } } +# The expect "spawn" function puts the tty name into the spawn_out +# array; but dejagnu doesn't export this globally. So, we have to +# wrap spawn with our own function and poke in the built-in spawn +# so that we can capture this value. +# +# If available, the TTY name is saved to the LAST_SPAWN_TTY_NAME global. +# Otherwise, LAST_SPAWN_TTY_NAME is unset. + +proc spawn_capture_tty_name { args } { + set result [uplevel builtin_spawn $args] + upvar spawn_out spawn_out + if { [info exists spawn_out] } { + set ::last_spawn_tty_name $spawn_out(slave,name) + } else { + unset ::last_spawn_tty_name + } + return $result +} + +rename spawn builtin_spawn +rename spawn_capture_tty_name spawn + # Default gdb_spawn procedure. proc default_gdb_spawn { } { @@ -2074,6 +2097,7 @@ proc default_gdb_spawn { } { } set gdb_spawn_id $res + set ::gdb_tty_name $::last_spawn_tty_name return 0 } @@ -7800,13 +7824,6 @@ proc with_override { name override body } { # finalization function. proc tuiterm_env { } { load_lib tuiterm.exp - - # Do initialization. - tuiterm_env_init - - # Schedule finalization. - global gdb_finish_hooks - lappend gdb_finish_hooks tuiterm_env_finish } # Dejagnu has a version of note, but usage is not allowed outside of dejagnu. diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp index fdd9f4d2188f..222583f291fb 100644 --- a/gdb/testsuite/lib/tuiterm.exp +++ b/gdb/testsuite/lib/tuiterm.exp @@ -15,36 +15,6 @@ # An ANSI terminal emulator for expect. -# The expect "spawn" function puts the tty name into the spawn_out -# array; but dejagnu doesn't export this globally. So, we have to -# wrap spawn with our own function, so that we can capture this value. -# The value is later used in calls to stty. -proc tuiterm_spawn { args } { - set result [uplevel builtin_spawn $args] - global gdb_spawn_name - upvar spawn_out spawn_out - if { [info exists spawn_out] } { - set gdb_spawn_name $spawn_out(slave,name) - } else { - unset gdb_spawn_name - } - return $result -} - -# Initialize tuiterm.exp environment. -proc tuiterm_env_init { } { - # Override spawn with tui_spawn. - rename spawn builtin_spawn - rename tuiterm_spawn spawn -} - -# Finalize tuiterm.exp environment. -proc tuiterm_env_finish { } { - # Restore spawn. - rename spawn tuiterm_spawn - rename builtin_spawn spawn -} - namespace eval Term { # Size of the terminal. variable _rows @@ -890,13 +860,12 @@ namespace eval Term { variable _cols variable _resize_count - global gdb_spawn_name # expect handles each argument to stty separately. This means # that gdb will see SIGWINCH twice. Rather than rely on this # behavior (which, after all, could be changed), we make it # explicit here. This also simplifies waiting for the redraw. _do_resize $rows $_cols - stty rows $_rows < $gdb_spawn_name + stty rows $_rows < $::gdb_tty_name # Due to the strange column resizing behavior, and because we # don't care about this intermediate resize, we don't check # the size here. @@ -906,7 +875,7 @@ namespace eval Term { # than what we request from expect. We hide this weird # details from the caller. _do_resize $_rows $cols - stty columns [expr {$_cols + 1}] < $gdb_spawn_name + stty columns [expr {$_cols + 1}] < $::gdb_tty_name wait_for "@@ resize done $_resize_count, size = ${_cols}x${rows}" incr _resize_count } -- 2.32.0