Re: [PATCH v8 6/6] gdb/infrun: handle already-exited threads when attempting to stop

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

On 14-05-2020 20:58, Pedro Alves wrote:
> On 5/14/20 7:54 PM, Aktemur, Tankut Baris wrote:
>> On Thursday, May 14, 2020 8:00 PM, Tom de Vries wrote:
>>> On 13-05-2020 22:53, Pedro Alves via Gdb-patches wrote:
>>>> +	set testpid($num) [get_integer_valueof "pid" -1]
>>>
>>> I'm running into:
>>> ...
>>> ERROR: tcl error sourcing
>>> /data/gdb_versions/devel/src/gdb/testsuite/gdb.multi/multi-kill.exp.
>>> ERROR: can't set "testpid(1)": variable isn't array
>>>     while executing
>>> "set testpid($num) [get_integer_valueof "pid" -1]"
>>
>> I'm not able to reproduce this problem.  The test runs fine on my system
>> with 58 expected passes and no failures.  Could it be related to the tcl
>> version?
> 
> It's related to testcase run order, and the fact that global variables
> can leak between testcases.
> 
>>
>> As a rough guess, could you try the patch below to initialize the array
>> variable?
>>
>> diff --git a/gdb/testsuite/gdb.multi/multi-kill.exp b/gdb/testsuite/gdb.multi/multi-kill.exp
>> index ce6075045fc..a21f8a78b0d 100644
>> --- a/gdb/testsuite/gdb.multi/multi-kill.exp
>> +++ b/gdb/testsuite/gdb.multi/multi-kill.exp
>> @@ -70,6 +70,7 @@ proc start_inferior {num} {
>>  # Sufficient inferiors to make sure that at least some other inferior
>>  # is killed while we're handling a killed event.
>>  set NUM_INFS 10
>> +array set testpid {}
> 
> That's not sufficient, because the variable can leak to other tests.
> For example:
> 
>  $ runtest gdb.threads/check-libthread-db.exp gdb.multi/multi-kill.exp
> 
>  Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.multi/multi-kill.exp ...
>  Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.threads/check-libthread-db.exp ...
>  ERROR: tcl error sourcing /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.threads/check-libthread-db.exp.
>  ERROR: can't set "testpid": variable is array
>      while executing
>  "set testpid [spawn_id_get_pid $test_spawn_id]"
>      ("uplevel" body line 8)
> 
> This works for me.
> 
> From 96aa00638e9a1201874e316ad53c38614da8cc04 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 14 May 2020 19:07:35 +0100
> Subject: [PATCH] Fix global variable collision in gdb.multi/multi-kill.exp
> 
> gdb/testsuite/ChangeLog:
> 2020-05-14  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.multi/multi-kill.exp: Wrap in namespace.
> 	(start_inferior): Add TESTPID parameter.  Use it instead of the
> 	testpid global.
> 	(top level): Define empty TESTPID array, and pass it down to
> 	start_inferior.
> ---
>  gdb/testsuite/gdb.multi/multi-kill.exp | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.multi/multi-kill.exp b/gdb/testsuite/gdb.multi/multi-kill.exp
> index ce6075045fc..03bf8449cf8 100644
> --- a/gdb/testsuite/gdb.multi/multi-kill.exp
> +++ b/gdb/testsuite/gdb.multi/multi-kill.exp
> @@ -39,11 +39,15 @@ save_vars { GDBFLAGS } {
>      clean_restart ${binfile}
>  }
>  
> +# Wrap the entire test in a namespace to avoid contaminating other tests.
> +namespace eval $testfile {
> +
>  # Start inferior NUM and record its PID in the TESTPID array.
>  
> -proc start_inferior {num} {
> +proc start_inferior {num testpid} {
>      with_test_prefix "start_inferior $num" {
> -	global testpid binfile srcfile
> +	upvar $testpid tpid
> +	global binfile srcfile
>  
>  	if {$num != 1} {
>  	    gdb_test "add-inferior" "Added inferior .*" \
> @@ -58,8 +62,8 @@ proc start_inferior {num} {
>  	gdb_run_cmd
>  	gdb_test "" ".*reakpoint .*, initialized .*${srcfile}.*" "run"
>  
> -	set testpid($num) [get_integer_valueof "pid" -1]
> -	if {$testpid($num) == -1} {
> +	set tpid($num) [get_integer_valueof "pid" -1]
> +	if {$tpid($num) == -1} {
>  	    return -1
>  	}
>  
> @@ -71,8 +75,11 @@ proc start_inferior {num} {
>  # is killed while we're handling a killed event.
>  set NUM_INFS 10
>  
> +# The array holding each inferior's PID, indexed by inferior number.
> +array set testpid {}
> +

Actually, we still need to declare the variable as a namespace variable
using:
...
variable testpid
array set testpid {}
...
otherwise we run into:
...
ERROR: tcl error sourcing
/data/gdb_versions/devel/src/gdb/testsuite/gdb.multi/multi-kill.exp.
ERROR: can't array set "testpid": variable isn't array
    while executing
"array set testpid {}"
    (in namespace eval "::multi-kill" script line 36)
    invoked from within
"namespace eval $testfile {
...

Thanks,
- Tom