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

[Tom de Vries <tdevries@suse.de>]

[-- 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