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

[Pedro Alves <palves@redhat.com>]

On 6/2/20 4:38 PM, Andrew Burgess wrote:

> If we're going to add code to loop over all globals anyway, then why
> not, instead of warning about bad cases, and then wrapping tests in a
> namespace, just have this code "fix" the leaked globals by deleting
> them?
> 
> My thinking is that any global that exists when we start a test should
> continue to exist at the end of the test.  Any other global should
> just be unset when the test script finishes.
> 
> If there really is some global state that is lazily created by a
> particular test script then (a) this seems like a bug anyway, and (b)
> this is easy to fix by giving it an earlier creation / initialisation.
> 
> In this way, folk can just write test scripts, dump their junk all
> over the global namespace as they like, and we'll just clean up for
> them.
> 
> Thoughts?

Hmm, I thought of this when I saw Tom's detection patch the first time,
but dismissed it, perhaps too quickly, thinking that it wouldn't
be safe.  You mention global state lazily created by test scripts, but I was
thinking of lazy initialized globals within Dejagnu itself, or whatever Tcl
library we might end up using the future, which might need to be carried
over from testcase to testcase.  Imagine, this in a Dejagnu function:

proc fail {} {
   global number_of_fails

   # Hadn't been set before.  (valid since Tcl 8.5; otherwise, imagine it wrapped in 'info exits')
   incr number_of_fails
}

This is obviously not the real "fail", it is just to
show the point, or the risk.  I imagined there might be similar
functions in random Dejagnu boards.

(
Now that I look, it seems like Dejagnu wants to be sure to initialize
all global variables it uses in runtest.exp:

 # Initialize a few global variables used by all tests.
 # `reset_vars' resets several of these, we define them here to document their
 # existence.  In fact, it would be nice if all globals used by some interface
 # of dejagnu proper were documented here.

And I guess we can always initialize some ourselves, if we find that missing.
)

So if people think the benefits outweigh the risks, then I guess
we should give it a go.

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.

Thanks,
Pedro Alves