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

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

Hi,

A variable name cannot be used both as scalar and array without an
intermediate unset.  Trying to do so will result in tcl errors, for
example, for:
...
  set var "bla"
  set var(1) "bla"
...
we get:
...
  can't set "var(1)": variable isn't array
...
and for the reverse statement order we get:
...
  can't set "var": variable is array
...

So, since a global name in one test-case can leak to another
test-case, setting a global name in one test-case can result in
a tcl error in another test-case that reuses the name in a different
way.

Warn about leaking a global array from a test-case.

Also, add a possibility to skip the warning in a given test-case using
variable gdb_skip_check_global_vars, and use it in gdb.mi/mi2-var-child.exp
and gdb.mi/mi-var-cp.exp.

Tested on x86_64-linux.

Any comments?

Thanks,
- Tom

[gdb/testsuite] Warn about leaked global array

gdb/testsuite/ChangeLog:

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

	PR testsuite/25996
	* lib/gdb.exp (global_array_exists, global_unset, save_global_vars)
	(check_global_vars): New proc.
	(gdb_init): Call save_global_vars.
	(gdb_finish): Call check_global_vars.
	* gdb.mi/mi-var-cp.exp: Set gdb_skip_check_global_vars.
	* gdb.mi/mi2-var-child.exp: Same.

---
 gdb/testsuite/gdb.mi/mi-var-cp.exp     |  3 ++
 gdb/testsuite/gdb.mi/mi2-var-child.exp |  3 ++
 gdb/testsuite/lib/gdb.exp              | 94 ++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)

diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp
index 8c6a624f80..dc32ddc346 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp
@@ -23,6 +23,9 @@ if [mi_gdb_start] {
     continue
 }
 
+# FIXME.  See check_global_vars in lib/gdb.exp.
+set gdb_skip_check_global_vars 1
+
 standard_testfile .cc
 
 if [get_compiler_info "c++"] {
diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp
index e32858fbd3..b96ac41815 100644
--- a/gdb/testsuite/gdb.mi/mi2-var-child.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp
@@ -27,6 +27,9 @@ if [mi_gdb_start] {
     continue
 }
 
+# FIXME.  See check_global_vars in lib/gdb.exp.
+set gdb_skip_check_global_vars 1
+
 standard_testfile var-cmd.c
 
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f7d20bd94f..a0d558c943 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5048,6 +5048,98 @@ proc standard_testfile {args} {
     }
 }
 
+# Returns 1 if GLOBALVARNAME is a global array.
+
+proc global_array_exists { globalvarname } {
+    # Introduce local alias of global variable $globalvarname.  Using
+    # "global $globalvarname" instead is simpler, but this may result in a
+    # clash with local name "globalvarname".
+    upvar #0 $globalvarname globalvar
+    return [array exists globalvar]
+}
+
+# Unset global variable GLOBALVARNAME.
+
+proc global_unset { globalvarname } {
+    # Introduce local alias of global variable $globalvarname.
+    upvar #0 $globalvarname globalvar
+    unset globalvar
+}
+
+# Save global vars to variable gdb_global_vars.
+
+proc save_global_vars { test_file_name } {
+    # Save for the warning.
+    global gdb_test_file_name
+    set gdb_test_file_name $test_file_name
+
+    # Sample state before running test.
+    global gdb_global_vars
+    set gdb_global_vars [info globals]
+
+    global gdb_skip_check_global_vars
+    if { [info exists gdb_skip_check_global_vars]} {
+	unset gdb_skip_check_global_vars
+    }
+}
+
+# Check global variables not in gdb_global_vars.
+
+proc check_global_vars { } {
+    global gdb_skip_check_global_vars
+    if { [info exists gdb_skip_check_global_vars]} {
+	return
+    }
+    # Sample state after running test.
+    global gdb_global_vars
+    set vars [info globals]
+
+    set skip [list "expect_out" "spawn_out" "auto_index"]
+
+    foreach var $vars {
+	if { ![global_array_exists $var] } {
+	    continue
+	}
+
+	set found [lsearch -exact $gdb_global_vars $var]
+	if { $found != -1 } {
+	    # Already present before running test.
+	    continue
+	}
+
+	set found [lsearch -exact $skip $var]
+	if { $found != -1 } {
+	    continue
+	}
+
+	# A variable name cannot be used both as scalar and array without an
+	# intermediate unset.  Trying to do so will result in tcl errors, for
+	# example, for:
+	#   set var "bla"
+	#   set var(1) "bla"
+	# we get:
+	#   can't set "var(1)": variable isn't array
+	# and for the reverse statement order we get:
+	#   can't set "var": variable is array
+	#
+	# So, since a global name in one test-case can leak to another
+	# test-case, setting a global name in one test-case can result in
+	# a tcl error in another test-case that reuses the name in a different
+	# way.
+	#
+	# Warn about leaking a global array from the test-case.
+	#
+	# For a description on how to fix this, see "Wrapping test-cases in
+	# Tcl namespaces" in gdb/testsuite/README.
+	global gdb_test_file_name
+	warning "$gdb_test_file_name.exp defined global array $var"
+
+	# If the variable remains set, we won't warn for the next test where
+	# it's leaked, so unset.
+	global_unset $var
+    }
+}
+
 # The default timeout used when testing GDB commands.  We want to use
 # the same timeout as the default dejagnu timeout, unless the user has
 # already provided a specific value (probably through a site.exp file).
@@ -5177,10 +5269,12 @@ proc gdb_init { test_file_name } {
     global gdb_instances
     set gdb_instances 0
 
+    save_global_vars $test_file_name
     return [default_gdb_init $test_file_name]
 }
 
 proc gdb_finish { } {
+    check_global_vars
     global gdbserver_reconnect_p
     global gdb_prompt
     global cleanfiles