Re: [PATCH] Guard against killing unrelated processes in amd64-disp-step.exp

[Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>]

Hi Andrew,

>>> diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>>> b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>>> --- a/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>>> +++ b/gdb/testsuite/gdb.arch/amd64-disp-step.exp
>>> @@ -222,7 +222,10 @@ proc rip_test { reg test_start_label tes
>>>  	    # If we use 'signal' to send the signal GDB doesn't actually do
>>>  	    # the displaced step, but instead just delivers the signal.
>>>  	    set inferior_pid [get_inferior_pid]
>>> -	    remote_exec target "kill -ALRM $inferior_pid"
>>> +	    # Ensure that $inferior_pid refers to a single process.
>>> +	    if {$inferior_pid > 0} {
>>> +		remote_exec target "kill -ALRM $inferior_pid"
>>> +	    }
>>
>> Does this not hide the fact that the test is no longer doing what it
>> expected?
>
> it does.  However, the results for this particular test were so bad
> already that I didn't think about one or more FAILs here.
>
>> I'm fine with the 'if {$inferior_pid > 0}' being added to ensure we
>> don't signal some random process(es), but I think we should probably add
>> something like:
>>
>>   gdb_assert {[expr $inferior_pid > 0]} \
>>     "check for a sane inferior pid"
>>   if {$inferior_pid > 0} {
>>     remote_exec target "kill -ALRM $inferior_pid"
>>   }
>>
>> This way you will still see a FAIL.
>
> True, but you will also see quite a bunch of PASSes in the working case
> that tell you nothing.  Seems like unnecessary noise to me.  Isn't there
> another way to convey the failure info without that noise?

how should we proceed with this patch?  It would be a pity to release
GDB 14 with make check killing the whole session on Solaris...

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University