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

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

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

Hi Tom,

>>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>
>>>> 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?
>
> Rainer> how should we proceed with this patch?  It would be a pity to release
> Rainer> GDB 14 with make check killing the whole session on Solaris...
>
> I think just adding Andrew's proposed assert to your patch should be
> good enough.
>
> The idea behind the assert is so that we can detect the bad case, if it
> ever happens, on a platform that is otherwise ok.  The noise of an extra
> pass doesn't seem so bad, we have zillions of those already.  The noise
> from the fail also shouldn't be too bad since, IIRC, you said this test
> is already not fully passing on Solaris.
>
> Anyway to sum up, the assert would be there as a "just in case" for
> other platforms, not Solaris.

fine with me.  Here's the patch as amended.

Re-tested on amd64-pc-solaris2.11 and x86_64-pc-linux-gnu.

Ok for master now?

Thanks.
	Rainer

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



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-amd64-random-kill.patch --]
[-- Type: text/x-patch, Size: 756 bytes --]

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,12 @@ 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.
+	    gdb_assert {[expr $inferior_pid > 0]} \
+		"check for a sane inferior pid"
+	    if {$inferior_pid > 0} {
+	    	remote_exec target "kill -ALRM $inferior_pid"
+	    }
 	}
 
 	gdb_test "continue" \