Re: [PING][RFC][gdb/testsuite] Fix inferior pid in gdb.server/server-kill.exp

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

On 10/25/22 14:20, Tom de Vries wrote:
> On 10/11/22 16:01, Tom de Vries wrote:
>> Hi,
>>
>> Consider this patch in gdb.server/server-kill.exp:
>> ...
>>   proc kill_server {} {
>>       global server_pid
>>
>> +    remote_exec target "pstree -a -p $server_pid"
>>       remote_exec target "kill -9 $server_pid"
>>   }
>> ...
>>
>> We have for kill_pid_of=server, as expected, the gdbserver killed:
>> ...
>> Executing on target: pstree -a -p 6969    (timeout = 300)
>> builtin_spawn -ignore SIGHUP pstree -a -p 6969^M
>> gdbserver,6969^M
>>    `-server-kill,6976^M
>> Executing on target: kill -9 6969    (timeout = 300)
>> builtin_spawn -ignore SIGHUP kill -9 6969^M
>> ...
>>
>> But for kill_pid_of=inferior, we also have the gdbserver killed:
>> ...
>> Executing on target: pstree -a -p 6805    (timeout = 300)
>> builtin_spawn -ignore SIGHUP pstree -a -p 6805^M
>> gdbserver,6805^M
>>    `-server-kill,6812^M
>> Executing on target: kill -9 6805    (timeout = 300)
>> builtin_spawn -ignore SIGHUP kill -9 6805^M
>> ...
>>
>> The proc prepare contains the pid extraction code:
>> ...
>>      if { $::kill_pid_of == "inferior" } {
>>          # Get the pid of GDBServer.
>>          set test "p server_pid"
>>          set server_pid 0
>>          gdb_test_multiple $test $test {
>>              -re " = ($decimal)\r\n$gdb_prompt $" {
>>                  set server_pid $expect_out(1,string)
>>                  pass $test
>>              }
>>          }
>>      } else {
>>          set server_pid [exp_pid -i $::server_spawn_id]
>>      }
>> ...
>>
>> The bit for $::kill_pid_of == "inferior" looks like the correct code to
>> extract the gdbserver pid (it prints the parent PID of the inferior).
>>
>> The bit for $::kill_pid_of == "server" does work for local target, but 
>> for
>> remote target using say target board remote-gdbserver-on-localhost, we 
>> have:
>> ...
>> Executing on target: pstree -a -p 10354    (timeout = 300)
>> builtin_spawn [open ...]^M
>> ssh,10354 -l vries localhost/gdbserver --once localhost:2350 s
>> XYZ0ZYX
>> Executing on target: kill -9 10354    (timeout = 300)
>> builtin_spawn [open ...]^M
>> XYZ0ZYX
>> ...
>> In other words, we're not killing the gdbserver, but the ssh session (a
>> problem which was already fixed once in commit f90183d7e31 ("Get 
>> GDBserver
>> pid on remote target")).
>>
>> Fix this by:
>> - using the $::kill_pid_of == "inferior" bit for the gdbserver pid, and
>> - using the inferior command for the the inferior pid.
>>
>> This introduces the following 4 FAILs:
>> ...
>> FAIL: gdb.server/server-kill.exp: kill_pid_of=inferior: test_tstatus: 
>> tstatus
>> FAIL: gdb.server/server-kill.exp: kill_pid_of=inferior: 
>> test_unwind_nosyms: bt
>> FAIL: gdb.server/server-kill.exp: kill_pid_of=inferior: 
>> test_unwind_syms: bt
>> FAIL: gdb.server/server-kill.exp: kill_pid_of=inferior: test_stepi: stepi
>> ...
>>
>> The first in more detail:
>> ...
>> (gdb) status-packet on
>> tstatus^M
>> No trace has been run on the target.^M
>> Collected 0 trace frames.^M
>> Trace buffer has 5242880 bytes of 5242880 bytes free (0% full).^M
>> Trace will stop if GDB disconnects.^M
>> Not looking at any trace frame.^M
>> (gdb) FAIL: gdb.server/server-kill.exp: kill_pid_of=inferior: 
>> test_tstatus: tstatus
>> ...
>>
>> The test-case contains the comment:
>> ...
>>   # When KILL_PID_OF is set to 'inferior' then the pid we kill is that
>>   # of the inferior running under gdbserver, when this process dies
>>   # gdbserver itself will exit.
>> ...
>> but that doesn't seem to be happening.
>>
>> I've added a sleep 60 after the kill to rule out any timing issues, 
>> and indeed
>> after 60 seconds the gdbserver is still running.
>>
>> I don't know gdbserver well enough to decide whether the test-case 
>> assumption
>> is wrong and we need to fix the test-case, or there's a gdbserver problem
>> which is known or for which we can file a PR.
>>
>> Tested on x86_64-linux.
>>
>> Any comments?
>>
> 
> Ping.
> 

I've ended up fixing this problem by commit 079f190d4cf 
("[gdb/testsuite] Fix gdb.server/server-kill.exp for remote target"), so 
this patch is now obsolete.

Thanks,
- Tom