Possible regression on PPC64 testsuite with native-{extended-}gdbserver (was: Re: [pushed] Use setjmp/longjmp for TRY/CATCH instead of sigsetjmp/siglongjmp)

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Tuesday, April 12 2016, Pedro Alves wrote:

> Now that we don't ever throw GDB exceptions from signal handlers [1],
> we can switch to have TRY/CATCH implemented in terms of plain
> setjmp/longjmp instead of sigsetjmp/siglongjmp.
>
> In https://sourceware.org/ml/gdb-patches/2015-02/msg00114.html, Yichun
> Zhang mentions a 11%/14%+ speedup in his GDB python scripts with a
> patch that did something similar to only a specific set of TRY/CATCH
> calls.
>
> [1] - https://sourceware.org/ml/gdb-patches/2016-03/msg00351.html
>
> Tested on x86_64 Fedora 23, native and gdbserver.

Hi Pedro,

Almost a month ago, a few PPC64{BE,LE} buildslaves started failing to
upload the gdb.log/gdb.sum files to the buildmaster when running
testcases related to native-{extended-}gdbserver.  This was causing the
buildmaster to be unable to analyze regressions.  I was concerned about
this, but somehow I forgot to investigate the problem for a while and
only got back to it a couple of days ago.

Edjunior (Cc'ed) and I looked at the logs and found that the actual
problem was that the testsuite step was being terminated because it was
timing out.  Edjunior kindly looked at the machine and found several
zombie processes hanging (apparently all of them belonging to
gdb.threads/process-dies-while-handling-bp.exp, but Edjunior can correct
me if I'm wrong).  Since these machines are VMs, we thought a simple
reboot would probably fix the issue.  Unfortunately it didn't.

Anyway, after some more "investigation", I found that this strange
behaviour started happening after you pushed this specific patch.  You
can see, for example, the last 155 builds on one of the builders
affected, and you'll see that the 'Failed' message only happens after
this commit:

  <http://gdb-build.sergiodj.net/builders/Fedora-ppc64be-native-extended-gdbserver-m64?numbuilds=155>

You can also look at another builder, and see that the behaviour is the
same:

  <http://gdb-build.sergiodj.net/builders/Fedora-ppc64be-native-gdbserver-m64?numbuilds=200>

Or:

  <http://gdb-build.sergiodj.net/builders/Fedora-ppc64le-native-extended-gdbserver-m64?numbuilds=200>

(This last one being a PPC64LE).

As I said, this problem doesn't happen when we're not testing gdbserver
configurations.

I haven't investigated the problem further, and it may very well be
something unrelated to this patch (notice that, although the failure
happens several times, it's not deterministic), but I decided it was
a good thing to raise awareness.

I'll try to invest more time to figure out what's happening; if I find
anything else, I'll reply to this e-mail.  If you need access to one of
the buildslaves, please let me know and I'll see if I can arrange that
for you.

Thanks,

> gdb/ChangeLog:
> 2016-04-12  Pedro Alves  <palves@redhat.com>
>
> 	* common/common-exceptions.c (struct catcher) <buf>: Now a
> 	'jmp_buf' instead of SIGJMP_BUF.
> 	(exceptions_state_mc_init): Change return type to 'jmp_buf'.
> 	(throw_exception): Use longjmp instead of SIGLONGJMP.
> 	* common/common-exceptions.h: Include <setjmp.h> instead of
> 	"gdb_setjmp.h".
> 	(exceptions_state_mc_init): Change return type to 'jmp_buf'.
> 	[GDB_XCPT == GDB_XCPT_SJMP] (TRY): Use setjmp instead of
> 	SIGSETJMP.
> 	* cp-support.c: Include "gdb_setjmp.h".
> ---
>  gdb/ChangeLog                  | 13 +++++++++++++
>  gdb/common/common-exceptions.c |  6 +++---
>  gdb/common/common-exceptions.h |  8 ++++----
>  gdb/cp-support.c               |  2 +-
>  4 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 5dfd4b0..b750266 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,18 @@
>  2016-04-12  Pedro Alves  <palves@redhat.com>
>  
> +	* common/common-exceptions.c (struct catcher) <buf>: Now a
> +	'jmp_buf' instead of SIGJMP_BUF.
> +	(exceptions_state_mc_init): Change return type to 'jmp_buf'.
> +	(throw_exception): Use longjmp instead of SIGLONGJMP.
> +	* common/common-exceptions.h: Include <setjmp.h> instead of
> +	"gdb_setjmp.h".
> +	(exceptions_state_mc_init): Change return type to 'jmp_buf'.
> +	[GDB_XCPT == GDB_XCPT_SJMP] (TRY): Use setjmp instead of
> +	SIGSETJMP.
> +	* cp-support.c: Include "gdb_setjmp.h".
> +
> +2016-04-12  Pedro Alves  <palves@redhat.com>
> +
>  	* common/common-exceptions.c (exception_rethrow): Remove
>  	prepare_to_throw_exception call.
>  	* common/common-exceptions.h (prepare_to_throw_exception): Delete
> diff --git a/gdb/common/common-exceptions.c b/gdb/common/common-exceptions.c
> index 5ea8188..2e63862 100644
> --- a/gdb/common/common-exceptions.c
> +++ b/gdb/common/common-exceptions.c
> @@ -46,7 +46,7 @@ struct catcher
>  {
>    enum catcher_state state;
>    /* Jump buffer pointing back at the exception handler.  */
> -  SIGJMP_BUF buf;
> +  jmp_buf buf;
>    /* Status buffer belonging to the exception handler.  */
>    struct gdb_exception exception;
>    struct cleanup *saved_cleanup_chain;
> @@ -73,7 +73,7 @@ catcher_list_size (void)
>    return size;
>  }
>  
> -SIGJMP_BUF *
> +jmp_buf *
>  exceptions_state_mc_init (void)
>  {
>    struct catcher *new_catcher = XCNEW (struct catcher);
> @@ -275,7 +275,7 @@ throw_exception (struct gdb_exception exception)
>       be zero, by definition in defs.h.  */
>    exceptions_state_mc (CATCH_THROWING);
>    current_catcher->exception = exception;
> -  SIGLONGJMP (current_catcher->buf, exception.reason);
> +  longjmp (current_catcher->buf, exception.reason);
>  #else
>    if (exception.reason == RETURN_QUIT)
>      {
> diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
> index 54c6249..e21713c 100644
> --- a/gdb/common/common-exceptions.h
> +++ b/gdb/common/common-exceptions.h
> @@ -20,7 +20,7 @@
>  #ifndef COMMON_EXCEPTIONS_H
>  #define COMMON_EXCEPTIONS_H
>  
> -#include "gdb_setjmp.h"
> +#include <setjmp.h>
>  
>  /* Reasons for calling throw_exceptions().  NOTE: all reason values
>     must be less than zero.  enum value 0 is reserved for internal use
> @@ -142,7 +142,7 @@ struct gdb_exception
>     macros defined below.  */
>  
>  #if GDB_XCPT == GDB_XCPT_SJMP
> -extern SIGJMP_BUF *exceptions_state_mc_init (void);
> +extern jmp_buf *exceptions_state_mc_init (void);
>  extern int exceptions_state_mc_action_iter (void);
>  extern int exceptions_state_mc_action_iter_1 (void);
>  extern int exceptions_state_mc_catch (struct gdb_exception *, int);
> @@ -181,9 +181,9 @@ extern void exception_rethrow (void);
>  
>  #define TRY \
>       { \
> -       SIGJMP_BUF *buf = \
> +       jmp_buf *buf = \
>  	 exceptions_state_mc_init (); \
> -       SIGSETJMP (*buf); \
> +       setjmp (*buf); \
>       } \
>       while (exceptions_state_mc_action_iter ()) \
>         while (exceptions_state_mc_action_iter_1 ())
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index c7f5074..5662f86 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -34,7 +34,7 @@
>  #include "cp-abi.h"
>  #include "namespace.h"
>  #include <signal.h>
> -
> +#include "gdb_setjmp.h"
>  #include "safe-ctype.h"
>  
>  #define d_left(dc) (dc)->u.s_binary.left
> -- 
> 2.5.5

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/