[PATCH][gdb/testsuite] Don't leak tuiterm.exp spawn override

Tom de Vries
2020-06-05T10:06:42Z

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

[ was: Re: [PATCH 3/3][gdb/testsuite] Warn about leaked global array ]

On 04-06-2020 13:40, Tom de Vries wrote:
> On 03-06-2020 11:49, Pedro Alves wrote:

>> BTW, global variables alone aren't the full scope of the
>> bleeding between testcases. There's also the case of
>> testcases overriding procedures, like gdb.base/dbx.exp,
>> but those are perhaps rare enough that we can continue
>> handling it "manually" as before.
>
> AFAICT, that test-case does an effort to undo the override, though I'm
> not sure how certain it is that the undo will be executed.
>
> Also, while working on the latest proposed patch, I also ran into
> trouble related to the renaming of spawn in lib/tuiterm.exp. I wonder if
> to address this we could wrap every tui test-case in
> ...
> tui_env {
> ...
> }
> ...
> and letting the tui_env proc deal with renaming and restoring the spawn.
> We could even automate this in gdb_init/gdb_finish by calling
> gdb_init_$subdir/gdb_finish_$subdir and handling it there.
>

That approach turned out to be not precise enough because also
gdb.python/tui-window.exp imports tuiterm.exp.

This fix uses a load_lib override.

Any comments?

Thanks
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Don-t-leak-tuiterm.exp-spawn-override.patch --]
[-- Type: text/x-patch, Size: 3873 bytes --]

[gdb/testsuite] Don't leak tuiterm.exp spawn override

In lib/tuiterm.exp the builtin spawn is overridden by a tui-specific version.

After running the first test-case that imports tuiterm.exp, the override
remains active, so it can cause trouble in subsequent test-cases, even if they
do not import tuiterm.exp. See f.i. commit c8d4f6dfd9 "[gdb/testsuite] Fix
spawn in tuiterm.exp".

Fix this by:
- adding a mechanism in load_lib that allows a file foo.exp to
define procs gdb_init_foo and gdb_finish_foo, where gdb_init_foo is executed
during load_lib, and gdb_finish_foo is executed during gdb_finish.
- using this mechanism to do the spawn override in gdb_init_tuiterm, and
restoring the original spawn in gdb_finish_tuiterm.

Tested on x86_64-linux.

gdb/testsuite/ChangeLog:

2020-06-05 Tom de Vries <tdevries@suse.de>

* lib/gdb.exp (gdb_finish_hooks): New global var.
(load_lib): New override proc.
(gdb_finish): Handle gdb_finish_hooks.
* lib/tuiterm.exp (spawn): Rename to ...
(tui_spawn): ... this.
(toplevel): Move rename of spawn ...
(gdb_init_tuiterm): ... here. New proc.
(gdb_finish_tuiterm): New proc.

---
gdb/testsuite/lib/gdb.exp | 42 ++++++++++++++++++++++++++++++++++++++++++
gdb/testsuite/lib/tuiterm.exp | 17 +++++++++++++++--
2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9a0620a2bf..986cfca99f 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -25,6 +25,42 @@ if {$tool == ""} {
exit 2
}

+# List of procs to run in gdb_finish.
+set gdb_finish_hooks [list]
+
+# Override proc load_lib.
+rename load_lib saved_load_lib
+
+# Run overriden load_lib. For FILE == foo.exp, run gdb_init_foo and schedule
+# gdb_finish_foo, if they exist.
+proc load_lib { file } {
+
+ # Call overridden load_lib version.
+ set code [catch "saved_load_lib $file" result]
+
+ # Do lib-specific initialization.
+ set lib_id [regsub -all {\.exp$} $file ""]
+ if { [info procs "gdb_init_${lib_id}"] != "" } {
+ gdb_init_${lib_id}
+ }
+
+ # Schedule lib-specific finalization.
+ if { [info procs "gdb_finish_${lib_id}"] != "" } {
+ global gdb_finish_hooks
+ lappend gdb_finish_hooks gdb_finish_${lib_id}
+ }
+
+ # Propagate errors.
+ if { $code == 1 } {
+ global errorInfo errorCode
+ return -code error -errorinfo $errorInfo -errorcode $errorCode $result
+ } elseif { $code > 1 } {
+ return -code $code $result
+ }
+
+ return $result
+}
+
load_lib libgloss.exp
load_lib cache.exp
load_lib gdb-utils.exp
@@ -5225,6 +5261,12 @@ proc gdb_finish { } {
}
set banned_traced 0
}
+
+ global gdb_finish_hooks
+ foreach gdb_finish_hook $gdb_finish_hooks {
+ $gdb_finish_hook
+ }
+ set gdb_finish_hooks [list]
}

global debug_format
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index 8c9f97af6e..680648564d 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -19,8 +19,7 @@
# 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.
-rename spawn builtin_spawn
-proc spawn {args} {
+proc tuiterm_spawn { args } {
set result [uplevel builtin_spawn $args]
global gdb_spawn_name
upvar spawn_out spawn_out
@@ -32,6 +31,20 @@ proc spawn {args} {
return $result
}

+# Initialize tuiterm.exp environment.
+proc gdb_init_tuiterm { } {
+ # Override spawn with tui_spawn.
+ rename spawn builtin_spawn
+ rename tuiterm_spawn spawn
+}
+
+# Finalize tuiterm.exp environment.
+proc gdb_finish_tuiterm { } {
+ # Restore spawn.
+ rename spawn tuiterm_spawn
+ rename builtin_spawn spawn
+}
+
namespace eval Term {
variable _rows
variable _cols