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

[Pedro Alves <pedro@palves.net>]

On 2023-07-13 18:59, Rainer Orth wrote:
> Hi Tom,
> 
>>>>>>> "Rainer" == Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>>
>> Rainer> When testing current gdb trunk on Solaris/amd64, the whole session was
>> Rainer> reliably terminated by make check.  I could trace this to the following
>> Rainer> entry in gdb.arch/amd64-disp-step/gdb.log:
>>
>> Thank you for the patch.
>>
>> Rainer> If $inferior_pid doesn't refer a single process for some reason, this
>> Rainer> kill would terminate either a process group or the whole session.
>>
>> I don't mind the patch, it seems like an improvement -- but I wonder why

Agreed.  This is the second time all these years that I'm seeing something like this.
ISTR some test killing everything on OpenBSD many years ago.  :-)

> 
> That's what I thought: if for whatever reason the pid turns
> non-positive, hell breaks lose if that's passed to kill unchecked.
> 
>> this ends up as -1, and whether a fix belongs elsewhere.
> 
> gdb.log shows
> 
> (gdb) PASS: gdb.arch/amd64-disp-step.exp: add into rcx: send_signal=off: verify_regs: rdi expected value
> jump test_rip_rcx
> Continuing at 0x4015b2.
> 
> Program terminated with signal SIGALRM, Alarm clock.
> The program no longer exists.
> [...]
> [Current inferior is 1 [<null>] (/vol/obj/gnu/gdb/gdb/11.4-amd64-dist/gdb/testsuite/outputs/gdb.arch/amd64-disp-step/amd64-disp-step)]
> 
> lib/gdb.exp (get_inferior_pid) turns this <null> into -1:
> 

I'm not exactly sure what is the particular problem triggering what you're seeing, but I observe a couple things:

#1 - Solaris doesn't support displaced stepping.  It could, the x86-64 gdbarch displaced step implementation is pretty generic.
But they're only installed on Linux today.  That's because displaced stepping is more useful with non-stop, and Solaris doesn't
support that.

However, I tweaked the testcase to force displaced-stepping off, with:

    -gdb_test "set displaced-stepping on" ""
    +gdb_test "set displaced-stepping off" ""

and the test still passes cleanly on Linux.  So that shouldn't itself be a problem.  GDB will just do the
regular remove-breakpoint, step, re-insert breakpoint dance on Solaris.

#2 - I did notice however something else.  The .S file has this:

 /* test syscall */

	.global test_syscall
	mov $0x27,%eax /* getpid */
 test_syscall:
	syscall
	nop
 test_syscall_end:
	nop

That seems like it is assuming Linux syscalls?  Or is 0x27 getpid on Solaris as well?  If not, I wouldn't
be surprised if that syscall is doing something undefined.  Wonder what happens if you comment out that
code and the corresponding test in the .exp file.

Pedro Alves