[Patch] Avoid deadlock in guality tests.

[Patch] Avoid deadlock in guality tests.

[Yvan Roux]

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

Hi,

we are confronted to a deadlock situation when doing native validation
on armv8l target.

When gcc/testsuite/gcc.dg/guality/example.c is executed it spawns gdb,
and makes it attach to his parent, but during the test execution, gdb
receives a SIGSEGV, which is handled as a stop signal.  Then the test
timeouts and dejagnu tries to terminate it, but doesn't manage to do
it, this was raised and discussed on dejagnu list.

https://lists.gnu.org/archive/html/dejagnu/2016-03/msg00048.html

Dejagnu cleanup mechanism needs to be enhanced, but I think that it
would also be better if guality tests don't get stuck and/or can be
killed easily.  This patch changes GDB signals handling to nostop for
SIGSEGV, SIGINT, SIGTERM and SIGBUS.  I am not sure if we need to
increase the list of signals to all the stop ones (which are not used
by GDB) or to restrict it just to SIGSEGV.

Tested without regression on native x86_64, i386, aarch64 targets and
unleash native armv8l one.  Is it OK for trunk ? (I don't know the
rules for that kind of testsuite fix during stage 4).

Cheers,
Yvan

2016-04-06  Yvan Roux  <yvan.roux@linaro.org>
            Pedro Alves  <palves@redhat.com>

        * gcc.dg/guality/guality.h (main): Avoid GDB being blocked on signals.

[-- Attachment #2: guality-fix.diff --]
[-- Type: text/plain, Size: 523 bytes --]

diff --git a/gcc/testsuite/gcc.dg/guality/guality.h b/gcc/testsuite/gcc.dg/guality/guality.h
index 52fa706..d5867d8 100644
--- a/gcc/testsuite/gcc.dg/guality/guality.h
+++ b/gcc/testsuite/gcc.dg/guality/guality.h
@@ -252,6 +252,10 @@ main (int argc, char *argv[])
       if (!guality_gdb_input
 	  || fprintf (guality_gdb_input, "\
 set height 0\n\
+handle SIGINT pass nostop\n\
+handle SIGTERM pass nostop\n\
+handle SIGSEGV pass nostop\n\
+handle SIGBUS pass nostop\n\
 attach %i\n\
 set guality_attached = 1\n\
 b %i\n\

Re: [Patch] Avoid deadlock in guality tests.

[Jakub Jelinek]

On Wed, Apr 06, 2016 at 04:53:47PM +0200, Yvan Roux wrote:
> 2016-04-06  Yvan Roux  <yvan.roux@linaro.org>
>             Pedro Alves  <palves@redhat.com>
> 
>         * gcc.dg/guality/guality.h (main): Avoid GDB being blocked on signals.

Ok.

> diff --git a/gcc/testsuite/gcc.dg/guality/guality.h b/gcc/testsuite/gcc.dg/guality/guality.h
> index 52fa706..d5867d8 100644
> --- a/gcc/testsuite/gcc.dg/guality/guality.h
> +++ b/gcc/testsuite/gcc.dg/guality/guality.h
> @@ -252,6 +252,10 @@ main (int argc, char *argv[])
>        if (!guality_gdb_input
>  	  || fprintf (guality_gdb_input, "\
>  set height 0\n\
> +handle SIGINT pass nostop\n\
> +handle SIGTERM pass nostop\n\
> +handle SIGSEGV pass nostop\n\
> +handle SIGBUS pass nostop\n\
>  attach %i\n\
>  set guality_attached = 1\n\
>  b %i\n\


	Jakub

Re: [Patch] Avoid deadlock in guality tests.

[Pedro Alves]

On 04/06/2016 03:53 PM, Yvan Roux wrote:
> Dejagnu cleanup mechanism needs to be enhanced, but I think that it
> would also be better if guality tests don't get stuck and/or can be
> killed easily.  This patch changes GDB signals handling to nostop for
> SIGSEGV, SIGINT, SIGTERM and SIGBUS.  I am not sure if we need to
> increase the list of signals to all the stop ones (which are not used
> by GDB) or to restrict it just to SIGSEGV.

I'd suggest:

 handle all pass nostop
 handle SIGINT pass nostop

That would make gdb pass _all_ signals except SIGTRAP

Thanks,
Pedro Alves

Re: [Patch] Avoid deadlock in guality tests.

[Yvan Roux]

On 6 April 2016 at 17:09, Pedro Alves <palves@redhat.com> wrote:
> On 04/06/2016 03:53 PM, Yvan Roux wrote:
>> Dejagnu cleanup mechanism needs to be enhanced, but I think that it
>> would also be better if guality tests don't get stuck and/or can be
>> killed easily.  This patch changes GDB signals handling to nostop for
>> SIGSEGV, SIGINT, SIGTERM and SIGBUS.  I am not sure if we need to
>> increase the list of signals to all the stop ones (which are not used
>> by GDB) or to restrict it just to SIGSEGV.
>
> I'd suggest:
>
>  handle all pass nostop
>  handle SIGINT pass nostop
>
> That would make gdb pass _all_ signals except SIGTRAP

I've committed it already :/

I can make the change, but isn't there cases where SIGILL is used for
breakpoints in GDB (I think I've seen that somewhere).


Yvan

Re: [Patch] Avoid deadlock in guality tests.

[Pedro Alves]

On 04/06/2016 04:13 PM, Yvan Roux wrote:
> On 6 April 2016 at 17:09, Pedro Alves <palves@redhat.com> wrote:
>> On 04/06/2016 03:53 PM, Yvan Roux wrote:
>>> Dejagnu cleanup mechanism needs to be enhanced, but I think that it
>>> would also be better if guality tests don't get stuck and/or can be
>>> killed easily.  This patch changes GDB signals handling to nostop for
>>> SIGSEGV, SIGINT, SIGTERM and SIGBUS.  I am not sure if we need to
>>> increase the list of signals to all the stop ones (which are not used
>>> by GDB) or to restrict it just to SIGSEGV.
>>
>> I'd suggest:
>>
>>  handle all pass nostop
>>  handle SIGINT pass nostop
>>
>> That would make gdb pass _all_ signals except SIGTRAP
> 
> I've committed it already :/
> 
> I can make the change, but isn't there cases where SIGILL is used for
> breakpoints in GDB (I think I've seen that somewhere).

True, and SIGSEGV and SIGEMT too.  But GDB handles that transparently
and won't pass such a breakpoint signal to the program, even with
"handle pass".  Only "handle SIGTRAP pass" passes a
breakpoint/step/etc. trap to the program.

Thanks,
Pedro Alves

Re: [Patch] Avoid deadlock in guality tests.

[Yvan Roux]

On 6 April 2016 at 17:24, Pedro Alves <palves@redhat.com> wrote:
> On 04/06/2016 04:13 PM, Yvan Roux wrote:
>> On 6 April 2016 at 17:09, Pedro Alves <palves@redhat.com> wrote:
>>> On 04/06/2016 03:53 PM, Yvan Roux wrote:
>>>> Dejagnu cleanup mechanism needs to be enhanced, but I think that it
>>>> would also be better if guality tests don't get stuck and/or can be
>>>> killed easily.  This patch changes GDB signals handling to nostop for
>>>> SIGSEGV, SIGINT, SIGTERM and SIGBUS.  I am not sure if we need to
>>>> increase the list of signals to all the stop ones (which are not used
>>>> by GDB) or to restrict it just to SIGSEGV.
>>>
>>> I'd suggest:
>>>
>>>  handle all pass nostop
>>>  handle SIGINT pass nostop
>>>
>>> That would make gdb pass _all_ signals except SIGTRAP
>>
>> I've committed it already :/
>>
>> I can make the change, but isn't there cases where SIGILL is used for
>> breakpoints in GDB (I think I've seen that somewhere).
>
> True, and SIGSEGV and SIGEMT too.  But GDB handles that transparently
> and won't pass such a breakpoint signal to the program, even with
> "handle pass".  Only "handle SIGTRAP pass" passes a
> breakpoint/step/etc. trap to the program.

Ah ok, thanks for the explanations Pedro, I'll prepare a new patch and
validate it.

Cheers,
Yvan

Re: [Patch] Avoid deadlock in guality tests.

[Yvan Roux]

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

Hi,

On 6 April 2016 at 17:29, Yvan Roux <yvan.roux@linaro.org> wrote:
> On 6 April 2016 at 17:24, Pedro Alves <palves@redhat.com> wrote:
>> On 04/06/2016 04:13 PM, Yvan Roux wrote:
>>> On 6 April 2016 at 17:09, Pedro Alves <palves@redhat.com> wrote:
>>>> On 04/06/2016 03:53 PM, Yvan Roux wrote:
>>>>> Dejagnu cleanup mechanism needs to be enhanced, but I think that it
>>>>> would also be better if guality tests don't get stuck and/or can be
>>>>> killed easily.  This patch changes GDB signals handling to nostop for
>>>>> SIGSEGV, SIGINT, SIGTERM and SIGBUS.  I am not sure if we need to
>>>>> increase the list of signals to all the stop ones (which are not used
>>>>> by GDB) or to restrict it just to SIGSEGV.
>>>>
>>>> I'd suggest:
>>>>
>>>>  handle all pass nostop
>>>>  handle SIGINT pass nostop
>>>>
>>>> That would make gdb pass _all_ signals except SIGTRAP
>>>
>>> I've committed it already :/
>>>
>>> I can make the change, but isn't there cases where SIGILL is used for
>>> breakpoints in GDB (I think I've seen that somewhere).
>>
>> True, and SIGSEGV and SIGEMT too.  But GDB handles that transparently
>> and won't pass such a breakpoint signal to the program, even with
>> "handle pass".  Only "handle SIGTRAP pass" passes a
>> breakpoint/step/etc. trap to the program.
>
> Ah ok, thanks for the explanations Pedro, I'll prepare a new patch and
> validate it.

Here is the new patch which makes GDB pass all signals except SIGTRAP.
validated on native armv8l target without regessions. ok for trunk ?

Cheers,
Yvan

2016-04-07  Yvan Roux  <yvan.roux@linaro.org>
            Pedro Alves  <palves@redhat.com>

        * gcc.dg/guality/guality.h (main): Make GDB pass all signals except
        SIGTRAP.

[-- Attachment #2: guality-fix.diff --]
[-- Type: text/plain, Size: 550 bytes --]

diff --git a/gcc/testsuite/gcc.dg/guality/guality.h b/gcc/testsuite/gcc.dg/guality/guality.h
index d5867d8..acf977f 100644
--- a/gcc/testsuite/gcc.dg/guality/guality.h
+++ b/gcc/testsuite/gcc.dg/guality/guality.h
@@ -252,10 +252,8 @@ main (int argc, char *argv[])
       if (!guality_gdb_input
 	  || fprintf (guality_gdb_input, "\
 set height 0\n\
+handle all pass nostop\n\
 handle SIGINT pass nostop\n\
-handle SIGTERM pass nostop\n\
-handle SIGSEGV pass nostop\n\
-handle SIGBUS pass nostop\n\
 attach %i\n\
 set guality_attached = 1\n\
 b %i\n\

Re: [Patch] Avoid deadlock in guality tests.

[Pedro Alves]

On 04/07/2016 02:41 PM, Yvan Roux wrote:

> Here is the new patch which makes GDB pass all signals except SIGTRAP.
> validated on native armv8l target without regessions. ok for trunk ?

LGTM, FWIW.
Thanks,
Pedro Alves