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

Andrew Burgess
2020-06-03T12:54:15Z

* 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
> }
> }
>