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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom de Vries <tdevries@suse.de> [2020-06-03 12:24:09 +0200]:

> On 03-06-2020 12:09, Tom de Vries wrote:
> > On 03-06-2020 11:38, Tom de Vries wrote:
> >> On 03-06-2020 10:47, Tom de Vries wrote:
> >>> ERROR: can't read "mi_gdb_prompt": no such variable
> >>>     while executing
> >>> "expect {
> >>> -i exp95 -timeout 10
> >>>         -re "~\"GNU.*\r\n~\".*$mi_gdb_prompt$" {
> >>>             # We have a new format mi startup prompt.  If we are
> >>>             # running mi1,..."
> >>>     ("uplevel" body line 1)
> >>>     invoked from within
> >>> "uplevel $body" TCL READ VARNAME can't read "mi_gdb_prompt": no such
> >>> variable
> >>
> >> So, the following happens:
> >> - a test-case imports mi-support.exp using load_lib
> >> - mi-support.exp sets mi_gdb_prompt
> >> - the test-case finishes and the new global mi_gdb_prompt is unset,
> >>   because it was not set before the test-case
> >> - a next test-case imports mi-support.exp using load_lib
> >> - load_lib sees that the file already has been loaded, so it skips it
> >> - mi_gdb_prompt remains unset
> >> - the test-case uses mi_gdb_prompt, and we have an error.

I couldn't work out why my testing was going fine if this problem
exists... then I realised that my testing script was causing the tests
to run in parallel!

I like the load_lib override solution.  I think a solution that avoids
having to place each test into a new namespace would be a good thing
if we can pull it off.

Thanks,
Andrew

> > 
> > This naive solution seems to work.
> > 
> > Perhaps we can override load_lib to do the same, automatically.
> > 
> 
> That seems to work, I'll test this through.
> 
> Thanks,
> - Tom
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 66de9d6be3..bca47b96f2 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -25,6 +25,39 @@ if {$tool == ""} {
>      exit 2
>  }
> 
> +rename load_lib saved_load_lib
> +
> +array set gdb_persistent_globals {}
> +
> +proc gdb_persistent_global { varname } {
> +    global gdb_persistent_globals
> +    set gdb_persistent_globals($varname) 1
> +}
> +
> +proc load_lib { file } {
> +    array set known_global {}
> +    foreach varname [info globals] {
> +       set known_globals($varname) 1
> +    }
> +
> +    set code [catch "saved_load_lib $file" result]
> +
> +    foreach varname [info globals] {
> +       if { ![info exists known_globals($varname)] } {
> +           gdb_persistent_global $varname
> +       }
> +    }
> +
> +    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
> @@ -5114,10 +5147,14 @@ proc gdb_setup_known_globals {} {
>  # GDB_KNOWN_GLOBALS array is unset, this ensures we don't "leak"
>  # globals from one test script to another.
>  proc gdb_cleanup_globals {} {
> -    global gdb_known_globals
> +    global gdb_known_globals gdb_persistent_globals
> 
>      foreach varname [info globals] {
>         if {![info exists gdb_known_globals($varname)]} {
> +           if { [info exists gdb_persistent_globals($varname)] } {
> +               continue
> +           }
> +           verbose -log "gdb_cleanup_globals: deleting $varname"
>             uplevel #0 unset $varname
>         }
>      }
>