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

[Pedro Alves <palves@redhat.com>]

On 5/18/20 7:18 AM, Tom de Vries wrote:

> I managed to find a better way of dealing with this, using "upvar #0".
> I've also added more comments to the patch.

Cool.

> +# Check global variables not in gdb_global_vars.
> +
> +proc check_global_vars { } {
> +    # Sample state after running test.
> +    global gdb_global_vars
> +    set vars [info globals]
> +
> +    # I'm not sure these two should actually be global, but at least there
> +    # seems to be no harm in having these as globals, given that we don't
> +    # expect to reuse these names as scalars.
> +    set skip [list "expect_out" "spawn_out"]

They can be local or global.  See man expect:

 Expect takes a rather liberal view of scoping. In particular, variables read 
 by commands specific to the Expect program will be sought first from the local
 scope, and if not found, in the global scope. For example, this obviates the need to
 place "global timeout" in every procedure you   write that uses expect. On the other hand,
 variables written are always in the local scope (unless a "global" command has been issued).
 The most common problem this causes is when spawn is executed in a procedure.
 Outside the procedure, spawn_id no longer exists, so the spawned process is no
 longer accessible simply because of scoping. Add a "global spawn_id" to such a procedure. 

For example, mi_gdb_test does:

proc mi_gdb_test { args } {
    global verbose
    global mi_gdb_prompt
    global GDB expect_out
               ^^^^^^^^^^
    global inferior_exited_re async
    upvar timeout timeout

and then gdb.mi/mi-basics.exp does:

proc test_path_specification {} {
...
    global expect_out

...
    mi_gdb_test "-environment-path" "\\\^done,path=\"(.*)\"" "environment-path"
    set orig_path $expect_out(3,string)
...

So making expect_out global allows gdb.mi/mi-basics.exp to reference it.

We could alternatively switch mi_gdb_test (and whatever other procedure
does a similar thing) to doing "upvar expect_out expect_out" and
get rid of "global expect_out".  Not sure it's worth the bother.

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

Tabs / spaces above.

Thanks,
Pedro Alves