[RFC/PATCH] New convenience variable $_exitsignal

[RFC/PATCH] New convenience variable $_exitsignal

[Sergio Durigan Junior]

This patch was proposed by Pedro at:

<http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>

It adds a new convenience variable called "$_exitsignal", which will
hold the signal number when the inferior terminates due to the uncaught
signal.

I've made modifications on infrun.c:handle_inferior_event such that
$_exitcode gets cleared when the inferior signalled, and vice-versa.
This assumption was made because IMO the variables are mutually
exclusive, i.e., when the inferior terminates because of an uncaught
signal it is not possible for it to return.  Anyway, Pedro's explanation
seems to follow the same logic.

The patch also adds a NEWS entry, documentation bits, and a testcase.

OK to apply?

-- 
Sergio

gdb/ChangeLog:
2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>

	* NEWS: Mention new convenience variable "$_exitsignal".
	* infrun.c (handle_inferior_event): Set internal variable
	"$_exitsignal" for TARGET_WAITKIND_SIGNALLED and clear
	"$_exitcode", vice-versa for TARGET_WAITKIND_STOPPED.

gdb/testsuite/ChangeLog:
2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.base/exitsignal.exp: New file.
	* gdb.base/segv.c: Likewise.

gdb/doc/ChangeLog:
2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.texinfo (Convenience Variables): Document "$_exitsignal".

diff --git a/gdb/NEWS b/gdb/NEWS
index eea9917..e9e81b3 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -69,6 +69,10 @@ show range-stepping
 * The exception-related catchpoints, like "catch throw", now accept a
   regular expression which can be used to filter exceptions by type.
 
+* The new convenience variable $_exitsignal is automatically set to
+  the signal number when the program being debugged dies due to an
+  uncaught signal.
+
 * MI changes
 
   ** The -trace-save MI command can optionally save trace buffer in Common
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e6ec4ff..2025f15 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -9735,7 +9735,12 @@ to match the format in which the data was printed.
 @item $_exitcode
 @vindex $_exitcode@r{, convenience variable}
 The variable @code{$_exitcode} is automatically set to the exit code when
-the program being debugged terminates.
+the program being debugged terminates normally.
+
+@item $_exitsignal
+@vindex $_exitsignal@r{, convenience variable}
+The variable @code{$_exitsignal} is automatically set to the signal
+number when the program being debugged dies due to an uncaught signal.
 
 @item $_exception
 The variable @code{$_exception} is set to the exception object being
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 51a032b..f4ed1ac 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3455,6 +3455,12 @@ handle_inferior_event (struct execution_control_state *ecs)
 	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
 				   (LONGEST) ecs->ws.value.integer);
 
+	  /* Clear the internal variable, since if we are here chances
+	     are the inferior has not been terminated by a signal.
+	     And even if it has, then GDB will get to
+	     TARGET_WAITKIND_SIGNALLED in time...  */
+	  clear_internalvar (lookup_internalvar ("_exitsignal"));
+
 	  /* Also record this in the inferior itself.  */
 	  current_inferior ()->has_exit_code = 1;
 	  current_inferior ()->exit_code = (LONGEST) ecs->ws.value.integer;
@@ -3462,7 +3468,17 @@ handle_inferior_event (struct execution_control_state *ecs)
 	  print_exited_reason (ecs->ws.value.integer);
 	}
       else
-	print_signal_exited_reason (ecs->ws.value.sig);
+	{
+	  print_signal_exited_reason (ecs->ws.value.sig);
+	  /* Set the value of the internal variable $_exitsignal,
+	     which holds the signal uncaught by the inferior.  */
+	  set_internalvar_integer (lookup_internalvar ("_exitsignal"),
+				   (LONGEST) ecs->ws.value.sig);
+
+	  /* Clear the $_exitcode internal variable, because if the
+	     inferior signalled then its return code does not exist.  */
+	  clear_internalvar (lookup_internalvar ("_exitcode"));
+	}
 
       gdb_flush (gdb_stdout);
       target_mourn_inferior ();
diff --git a/gdb/testsuite/gdb.base/exitsignal.exp b/gdb/testsuite/gdb.base/exitsignal.exp
new file mode 100644
index 0000000..aa041d5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/exitsignal.exp
@@ -0,0 +1,49 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile segv.c
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+# Run to main
+if { ![runto_main] } {
+    return -1
+}
+
+# Print $_exitsignal.  It should be void now, because nothing
+# happened.
+gdb_test "print \$_exitsignal" " = void" \
+    "\$_exitsignal is void before running"
+
+# Trigger SIGSEGV.
+gdb_test "continue" "Program received signal SIGSEGV.*" "trigger SIGSEGV"
+
+# Continue until the end.
+gdb_test "continue" "Program terminated with signal SIGSEGV.*" \
+    "program terminated with SIGSEGV"
+
+# Now, print $_exitsignal again.  It should be 11 (SIGSEGV).
+gdb_test "print \$_exitsignal" " = 11"
+
+# Re-run to main, i.e., restart the executable.
+if { ![rerun_to_main] } {
+    return -1
+}
+
+# Print the $_exitsignal again.  Even in this normal scenario, it
+# should still contain the signal triggered in the other run.
+gdb_test "print \$_exitsignal" " = 11"
diff --git a/gdb/testsuite/gdb.base/segv.c b/gdb/testsuite/gdb.base/segv.c
new file mode 100644
index 0000000..dddbd4c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/segv.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+/* This test can be used just to generate a SIGSEGV.  */
+
+#include <stdio.h>
+
+int
+main (int argc, char *argv[])
+{
+  char *p = NULL;
+
+  /* Generating a SIGSEGV.  */
+  *p = 1;
+}

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Doug Evans]

On Sat, Jun 15, 2013 at 11:25 PM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> This patch was proposed by Pedro at:
>
> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>
>
> It adds a new convenience variable called "$_exitsignal", which will
> hold the signal number when the inferior terminates due to the uncaught
> signal.
>
> I've made modifications on infrun.c:handle_inferior_event such that
> $_exitcode gets cleared when the inferior signalled, and vice-versa.
> This assumption was made because IMO the variables are mutually
> exclusive, i.e., when the inferior terminates because of an uncaught
> signal it is not possible for it to return.  Anyway, Pedro's explanation
> seems to follow the same logic.
>
> The patch also adds a NEWS entry, documentation bits, and a testcase.
>
> OK to apply?

Hi.
I'd like the documentation to explain how it's intended that the two
(exitcode/exitsignal) are intended to be used together.  E.g., how
does one know which one has a useful value?


> gdb/ChangeLog:
> 2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>
>
>         * NEWS: Mention new convenience variable "$_exitsignal".
>         * infrun.c (handle_inferior_event): Set internal variable
>         "$_exitsignal" for TARGET_WAITKIND_SIGNALLED and clear
>         "$_exitcode", vice-versa for TARGET_WAITKIND_STOPPED.

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Eli Zaretskii]

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: Pedro Alves <palves@redhat.com>
> Date: Sun, 16 Jun 2013 03:25:15 -0300
> 
> This patch was proposed by Pedro at:
> 
> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>
> 
> It adds a new convenience variable called "$_exitsignal", which will
> hold the signal number when the inferior terminates due to the uncaught
> signal.

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index eea9917..e9e81b3 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -69,6 +69,10 @@ show range-stepping
>  * The exception-related catchpoints, like "catch throw", now accept a
>    regular expression which can be used to filter exceptions by type.
>  
> +* The new convenience variable $_exitsignal is automatically set to
> +  the signal number when the program being debugged dies due to an
> +  uncaught signal.

I'd say "set to the terminating signal number".  Otherwise, OK.

> +@item $_exitsignal
> +@vindex $_exitsignal@r{, convenience variable}
> +The variable @code{$_exitsignal} is automatically set to the signal
> +number when the program being debugged dies due to an uncaught signal.

I'd reverse the order:

 When the program being debugged dies due to an uncaught signal,
 @value{GDBN} automatically sets this variable to that signal's
 number.

RE: [RFC/PATCH] New convenience variable $_exitsignal

[Pierre Muller]

  Hi Sergio,

  Is there a reason why you don't handle
corelow.c anymore in your new patch?

  Also, I seem to vaguely remember that on
linux, the signal that generated the program exit is
finally embedded in the status...
  See WIFEXITED and WIFSIGNALED macros in linux-nat.c

For instance if I interrupt as (launched without argumrents)
with Ctrl-C,
The exit code (at least in bash shell) is 130.

  Is this a shell feature? If not, I am not sure setting $_exitcode 
variable to zero is the right thing to do.

  Sorry to bother you again...

Pierre Muller
 

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Sergio Durigan Junior
> Envoyé : dimanche 16 juin 2013 08:25
> À : GDB Patches
> Cc : Pedro Alves
> Objet : [RFC/PATCH] New convenience variable $_exitsignal
> 
> This patch was proposed by Pedro at:
> 
> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>
> 
> It adds a new convenience variable called "$_exitsignal", which will
> hold the signal number when the inferior terminates due to the uncaught
> signal.
> 
> I've made modifications on infrun.c:handle_inferior_event such that
> $_exitcode gets cleared when the inferior signalled, and vice-versa.
> This assumption was made because IMO the variables are mutually
> exclusive, i.e., when the inferior terminates because of an uncaught
> signal it is not possible for it to return.  Anyway, Pedro's explanation
> seems to follow the same logic.
> 
> The patch also adds a NEWS entry, documentation bits, and a testcase.
> 
> OK to apply?
> 
> --
> Sergio
> 
> gdb/ChangeLog:
> 2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* NEWS: Mention new convenience variable "$_exitsignal".
> 	* infrun.c (handle_inferior_event): Set internal variable
> 	"$_exitsignal" for TARGET_WAITKIND_SIGNALLED and clear
> 	"$_exitcode", vice-versa for TARGET_WAITKIND_STOPPED.
> 
> gdb/testsuite/ChangeLog:
> 2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* gdb.base/exitsignal.exp: New file.
> 	* gdb.base/segv.c: Likewise.
> 
> gdb/doc/ChangeLog:
> 2013-06-16  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* gdb.texinfo (Convenience Variables): Document "$_exitsignal".
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index eea9917..e9e81b3 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -69,6 +69,10 @@ show range-stepping
>  * The exception-related catchpoints, like "catch throw", now accept a
>    regular expression which can be used to filter exceptions by type.
> 
> +* The new convenience variable $_exitsignal is automatically set to
> +  the signal number when the program being debugged dies due to an
> +  uncaught signal.
> +
>  * MI changes
> 
>    ** The -trace-save MI command can optionally save trace buffer in
Common
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index e6ec4ff..2025f15 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -9735,7 +9735,12 @@ to match the format in which the data was printed.
>  @item $_exitcode
>  @vindex $_exitcode@r{, convenience variable}
>  The variable @code{$_exitcode} is automatically set to the exit code when
> -the program being debugged terminates.
> +the program being debugged terminates normally.
> +
> +@item $_exitsignal
> +@vindex $_exitsignal@r{, convenience variable}
> +The variable @code{$_exitsignal} is automatically set to the signal
> +number when the program being debugged dies due to an uncaught signal.
> 
>  @item $_exception
>  The variable @code{$_exception} is set to the exception object being
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 51a032b..f4ed1ac 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3455,6 +3455,12 @@ handle_inferior_event (struct
execution_control_state
> *ecs)
>  	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
>  				   (LONGEST) ecs->ws.value.integer);
> 
> +	  /* Clear the internal variable, since if we are here chances
> +	     are the inferior has not been terminated by a signal.
> +	     And even if it has, then GDB will get to
> +	     TARGET_WAITKIND_SIGNALLED in time...  */
> +	  clear_internalvar (lookup_internalvar ("_exitsignal"));
> +
>  	  /* Also record this in the inferior itself.  */
>  	  current_inferior ()->has_exit_code = 1;
>  	  current_inferior ()->exit_code = (LONGEST) ecs->ws.value.integer;
> @@ -3462,7 +3468,17 @@ handle_inferior_event (struct
execution_control_state
> *ecs)
>  	  print_exited_reason (ecs->ws.value.integer);
>  	}
>        else
> -	print_signal_exited_reason (ecs->ws.value.sig);
> +	{
> +	  print_signal_exited_reason (ecs->ws.value.sig);
> +	  /* Set the value of the internal variable $_exitsignal,
> +	     which holds the signal uncaught by the inferior.  */
> +	  set_internalvar_integer (lookup_internalvar ("_exitsignal"),
> +				   (LONGEST) ecs->ws.value.sig);
> +
> +	  /* Clear the $_exitcode internal variable, because if the
> +	     inferior signalled then its return code does not exist.  */
> +	  clear_internalvar (lookup_internalvar ("_exitcode"));
> +	}
> 
>        gdb_flush (gdb_stdout);
>        target_mourn_inferior ();
> diff --git a/gdb/testsuite/gdb.base/exitsignal.exp
> b/gdb/testsuite/gdb.base/exitsignal.exp
> new file mode 100644
> index 0000000..aa041d5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/exitsignal.exp
> @@ -0,0 +1,49 @@
> +# Copyright 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +standard_testfile segv.c
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +# Run to main
> +if { ![runto_main] } {
> +    return -1
> +}
> +
> +# Print $_exitsignal.  It should be void now, because nothing
> +# happened.
> +gdb_test "print \$_exitsignal" " = void" \
> +    "\$_exitsignal is void before running"
> +
> +# Trigger SIGSEGV.
> +gdb_test "continue" "Program received signal SIGSEGV.*" "trigger SIGSEGV"
> +
> +# Continue until the end.
> +gdb_test "continue" "Program terminated with signal SIGSEGV.*" \
> +    "program terminated with SIGSEGV"
> +
> +# Now, print $_exitsignal again.  It should be 11 (SIGSEGV).
> +gdb_test "print \$_exitsignal" " = 11"
> +
> +# Re-run to main, i.e., restart the executable.
> +if { ![rerun_to_main] } {
> +    return -1
> +}
> +
> +# Print the $_exitsignal again.  Even in this normal scenario, it
> +# should still contain the signal triggered in the other run.
> +gdb_test "print \$_exitsignal" " = 11"
> diff --git a/gdb/testsuite/gdb.base/segv.c b/gdb/testsuite/gdb.base/segv.c
> new file mode 100644
> index 0000000..dddbd4c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/segv.c
> @@ -0,0 +1,31 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2013 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +*/
> +
> +/* This test can be used just to generate a SIGSEGV.  */
> +
> +#include <stdio.h>
> +
> +int
> +main (int argc, char *argv[])
> +{
> +  char *p = NULL;
> +
> +  /* Generating a SIGSEGV.  */
> +  *p = 1;
> +}

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Sergio Durigan Junior]

On Monday, June 17 2013, Pierre Muller wrote:

>   Hi Sergio,
>
>   Is there a reason why you don't handle
> corelow.c anymore in your new patch?

Hi Pierre,

Yes, corelow.c is not important to this patch because (as Pedro
explained on
<http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>)
$_exitsignal should not be set for corefiles, because the inferior has
not exited.

corelow.c will be touched in my next patch, which will add $_signo (but
with the modifications proposed by Pedro).

>   Also, I seem to vaguely remember that on
> linux, the signal that generated the program exit is
> finally embedded in the status...
>   See WIFEXITED and WIFSIGNALED macros in linux-nat.c
>
> For instance if I interrupt as (launched without argumrents)
> with Ctrl-C,
> The exit code (at least in bash shell) is 130.
>
>   Is this a shell feature? If not, I am not sure setting $_exitcode 
> variable to zero is the right thing to do.

To the extent of my knowledge, this is a shell feature.  It sets $? (the
variable which contains the exit code of the program) to 128 + signal
number.  However, if you use this simple C program (borrowed from
waitpid's manpage, and modified to print the exit code when the child is
killed by a signal):

    #include <sys/wait.h>
    #include <stdlib.h>
    #include <unistd.h>
    #include <stdio.h>

    int
    main(int argc, char *argv[])
    {
        pid_t cpid, w;
        int status;

        cpid = fork();
        if (cpid == -1) {
            perror("fork");
            exit(EXIT_FAILURE);
        }

        if (cpid == 0) {            /* Code executed by child */
            printf("Child PID is %ld\n", (long) getpid());
            if (argc == 1)
                pause();                    /* Wait for signals */
            _exit(atoi(argv[1]));

        } else {                    /* Code executed by parent */
            do {
                w = waitpid(cpid, &status, WUNTRACED | WCONTINUED);
                if (w == -1) {
                    perror("waitpid");
                    exit(EXIT_FAILURE);
                }

                if (WIFEXITED(status)) {
                    printf("exited, status=%d\n", WEXITSTATUS(status));
                } else if (WIFSIGNALED(status)) {
                    printf("killed by signal %d, exit=%d\n", WTERMSIG(status),
                           WEXITSTATUS(status));
                } else if (WIFSTOPPED(status)) {
                    printf("stopped by signal %d\n", WSTOPSIG(status));
                } else if (WIFCONTINUED(status)) {
                    printf("continued\n");
                }
            } while (!WIFEXITED(status) && !WIFSIGNALED(status));
            exit(EXIT_SUCCESS);
        }
    }

And run it:

    $ ./a.out 
    Child PID is 2448

Then, on another terminal:

    $ kill -USR1 2448

You'll see:

    killed by signal 10, exit=0

Thus, it makes sense to unset $_exitcode (note that I am not setting it
to zero, as you said).

>   Sorry to bother you again...

Hey, no problem, your question was actually very pertinent.

Thanks,

-- 
Sergio

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Sergio Durigan Junior]

On Monday, June 17 2013, I wrote:

> On Monday, June 17 2013, Pierre Muller wrote:
>
>>   Hi Sergio,
>>
>>   Is there a reason why you don't handle
>> corelow.c anymore in your new patch?
>
> Hi Pierre,
>
> Yes, corelow.c is not important to this patch because (as Pedro
> explained on
> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>)
> $_exitsignal should not be set for corefiles, because the inferior has
> not exited.
>
> corelow.c will be touched in my next patch, which will add $_signo (but
> with the modifications proposed by Pedro).

I've been thinking about this answer I gave to Pierre.  After
investigating how corefiles handle the signal, I guess the right choice
would indeed be to set $_exitsignal in corelow.c as well.  This is my
rationale.

1) Single-threaded program + generate-core-file

In this case, NT_SIGINFO will not be filled by GDB's generate-core-file
(bug) because PRSTATUS generation does not contemplate that yet (which
reminds me of the PRPSINFO work I did few months ago, and the PRSTATUS
work I still need to do, which will fix this bug).  So, in this case,
"print $_siginfo.si_signo" will not display the correct signal, and we
can only rely on "bfd_core_file_failing_signal" (called inside
corelow.c).  Thus, setting $_signo to "bfd_core_file_failing_signal" is
the logical choice (of course, if we want to avoid having to use
NT_SIGINFO, that is the *only* choice).

2) Single-threaded program + SIGSEGV (or another "Core" signal)

In this case, the Linux kernel correctly generates the NT_SIGINFO, which
can be displayed by $_siginfo.  However, we don't want to use
NT_SIGINFO, so "bfd_core_file_failing_signal" is the only choice again.

3) Multi-threaded program + generate-core-file

Again, NT_SIGINFO is not generated by GDB.  Again,
"bfd_core_file_failing_signal" is the only choice.  (Back to this case
later)

4) Multi-threaded program + SIGSEGV (or another "Core" signal)

Linux kernel generated NT_SIGINFO, but we don't want to use it.
However, the kernel put in NT_SIGINFO the same signal number (which
killed the process) for all threads.  Thus, using
"bfd_core_file_failing_signal" is OK since there is no concept of "this
signal number killed only this thread".


Case (3) is the most difficult IMO.  I don't know how we are going to
handle it when I/we implement NT_SIGINFO generation on PRSTATUS.  My
first reaction is to do it using the same logic as the Linux kernel,
i.e., putting the same signal number in every thread's siginfo.  But I
don't think we should bikeshed too much now, so I'm stopping my e-mail
here.

I'd like to hear opinions.

Thanks,

-- 
Sergio

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Pedro Alves]

On 06/19/2013 05:59 AM, Sergio Durigan Junior wrote:
> On Monday, June 17 2013, I wrote:
> 
>> On Monday, June 17 2013, Pierre Muller wrote:
>>
>>>   Hi Sergio,
>>>
>>>   Is there a reason why you don't handle
>>> corelow.c anymore in your new patch?
>>
>> Hi Pierre,
>>
>> Yes, corelow.c is not important to this patch because (as Pedro
>> explained on
>> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>)
>> $_exitsignal should not be set for corefiles, because the inferior has
>> not exited.
>>
>> corelow.c will be touched in my next patch, which will add $_signo (but
>> with the modifications proposed by Pedro).
> 
> I've been thinking about this answer I gave to Pierre.  After
> investigating how corefiles handle the signal, I guess the right choice
> would indeed be to set $_exitsignal in corelow.c as well.  This is my
> rationale.

Looks like I completely missed this email.  Sorry about that.

> 1) Single-threaded program + generate-core-file
> 
> In this case, NT_SIGINFO will not be filled by GDB's generate-core-file
> (bug) because PRSTATUS generation does not contemplate that yet (which
> reminds me of the PRPSINFO work I did few months ago, and the PRSTATUS
> work I still need to do, which will fix this bug).  So, in this case,
> "print $_siginfo.si_signo" will not display the correct signal, and we
> can only rely on "bfd_core_file_failing_signal" (called inside
> corelow.c).  Thus, setting $_signo to "bfd_core_file_failing_signal" is
> the logical choice (of course, if we want to avoid having to use
> NT_SIGINFO, that is the *only* choice).
> 
> 2) Single-threaded program + SIGSEGV (or another "Core" signal)
> 
> In this case, the Linux kernel correctly generates the NT_SIGINFO, which
> can be displayed by $_siginfo.  However, we don't want to use
> NT_SIGINFO, so "bfd_core_file_failing_signal" is the only choice again.
> 
> 3) Multi-threaded program + generate-core-file
> 
> Again, NT_SIGINFO is not generated by GDB.  Again,
> "bfd_core_file_failing_signal" is the only choice.  (Back to this case
> later)
> 
> 4) Multi-threaded program + SIGSEGV (or another "Core" signal)
> 
> Linux kernel generated NT_SIGINFO, but we don't want to use it.
> However, the kernel put in NT_SIGINFO the same signal number (which
> killed the process) for all threads.

Really?  That's ......, to say the least.  ;-)

Actually, in my 3.9.10-100.fc17.x86_64 kernel (Fedora 17),
what I see is that the kernel only generates the NT_SIGINFO
note for the thread that actually crashed.

Hmm, actually, for a program with 3 threads, that has one thread
call abort, I get:

$ readelf -n ~/gdb/tests/threads-crash.core.32195

Displaying notes found at file offset 0x000005f0 with length 0x00001998:
  Owner                 Data size       Description
  CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000088       NT_PRPSINFO (prpsinfo structure)
  CORE                 0x00000080       NT_SIGINFO (siginfo_t data)
  CORE                 0x00000130       NT_AUXV (auxiliary vector)
  CORE                 0x000002aa       NT_FILE (mapped files)
...
  CORE                 0x00000200       NT_FPREGSET (floating point registers)
  LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
  CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000200       NT_FPREGSET (floating point registers)
  LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
  CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
  CORE                 0x00000200       NT_FPREGSET (floating point registers)
  LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)

Which kind of makes sense, given the other threads didn't actually get
any signal.

> Thus, using
> "bfd_core_file_failing_signal" is OK since there is no concept of "this
> signal number killed only this thread".
> 
> 
> Case (3) is the most difficult IMO.  I don't know how we are going to
> handle it when I/we implement NT_SIGINFO generation on PRSTATUS.  My
> first reaction is to do it using the same logic as the Linux kernel,
> i.e., putting the same signal number in every thread's siginfo.  But I
> don't think we should bikeshed too much now, so I'm stopping my e-mail
> here.
> 
> I'd like to hear opinions.

I can't say I really understand how any of that argues against my
original rationale for not setting $_exitsignal on corefiles (because
the inferior has not really exited at the point the core has been
generated), rather than point at implementation choices.

Now, if one were to instead argue that _user interface_ -wise, it'd
make sense to set $_exitsignal, because we also print
"Program terminated with signal", (emphasis on "terminated"), then
I'd agree:

  siggy = bfd_core_file_failing_signal (core_bfd);
  if (siggy > 0)
    {
...
      printf_filtered (_("Program terminated with signal %s, %s.\n"),
		       gdb_signal_to_name (sig), gdb_signal_to_string (sig));
    }

-- 
Pedro Alves

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Sergio Durigan Junior]

On Monday, September 16 2013, Pedro Alves wrote:

> On 06/19/2013 05:59 AM, Sergio Durigan Junior wrote:
>> On Monday, June 17 2013, I wrote:
>> 
>>> On Monday, June 17 2013, Pierre Muller wrote:
>>>
>>>>   Hi Sergio,
>>>>
>>>>   Is there a reason why you don't handle
>>>> corelow.c anymore in your new patch?
>>>
>>> Hi Pierre,
>>>
>>> Yes, corelow.c is not important to this patch because (as Pedro
>>> explained on
>>> <http://sourceware.org/ml/gdb-patches/2013-06/msg00337.html>)
>>> $_exitsignal should not be set for corefiles, because the inferior has
>>> not exited.
>>>
>>> corelow.c will be touched in my next patch, which will add $_signo (but
>>> with the modifications proposed by Pedro).
>> 
>> I've been thinking about this answer I gave to Pierre.  After
>> investigating how corefiles handle the signal, I guess the right choice
>> would indeed be to set $_exitsignal in corelow.c as well.  This is my
>> rationale.
>
> Looks like I completely missed this email.  Sorry about that.

No problem, I forgot about it as well, and I wrote it mostly to keep my
opinions recorded.  Glad you replied to it, though!

>> 1) Single-threaded program + generate-core-file
>> 
>> In this case, NT_SIGINFO will not be filled by GDB's generate-core-file
>> (bug) because PRSTATUS generation does not contemplate that yet (which
>> reminds me of the PRPSINFO work I did few months ago, and the PRSTATUS
>> work I still need to do, which will fix this bug).  So, in this case,
>> "print $_siginfo.si_signo" will not display the correct signal, and we
>> can only rely on "bfd_core_file_failing_signal" (called inside
>> corelow.c).  Thus, setting $_signo to "bfd_core_file_failing_signal" is
>> the logical choice (of course, if we want to avoid having to use
>> NT_SIGINFO, that is the *only* choice).
>> 
>> 2) Single-threaded program + SIGSEGV (or another "Core" signal)
>> 
>> In this case, the Linux kernel correctly generates the NT_SIGINFO, which
>> can be displayed by $_siginfo.  However, we don't want to use
>> NT_SIGINFO, so "bfd_core_file_failing_signal" is the only choice again.
>> 
>> 3) Multi-threaded program + generate-core-file
>> 
>> Again, NT_SIGINFO is not generated by GDB.  Again,
>> "bfd_core_file_failing_signal" is the only choice.  (Back to this case
>> later)
>> 
>> 4) Multi-threaded program + SIGSEGV (or another "Core" signal)
>> 
>> Linux kernel generated NT_SIGINFO, but we don't want to use it.
>> However, the kernel put in NT_SIGINFO the same signal number (which
>> killed the process) for all threads.
>
> Really?  That's ......, to say the least.  ;-)
>
> Actually, in my 3.9.10-100.fc17.x86_64 kernel (Fedora 17),
> what I see is that the kernel only generates the NT_SIGINFO
> note for the thread that actually crashed.
>
> Hmm, actually, for a program with 3 threads, that has one thread
> call abort, I get:
>
> $ readelf -n ~/gdb/tests/threads-crash.core.32195
>
> Displaying notes found at file offset 0x000005f0 with length 0x00001998:
>   Owner                 Data size       Description
>   CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
>   CORE                 0x00000088       NT_PRPSINFO (prpsinfo structure)
>   CORE                 0x00000080       NT_SIGINFO (siginfo_t data)
>   CORE                 0x00000130       NT_AUXV (auxiliary vector)
>   CORE                 0x000002aa       NT_FILE (mapped files)
> ...
>   CORE                 0x00000200       NT_FPREGSET (floating point registers)
>   LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
>   CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
>   CORE                 0x00000200       NT_FPREGSET (floating point registers)
>   LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
>   CORE                 0x00000150       NT_PRSTATUS (prstatus structure)
>   CORE                 0x00000200       NT_FPREGSET (floating point registers)
>   LINUX                0x00000340       NT_X86_XSTATE (x86 XSAVE extended state)
>
> Which kind of makes sense, given the other threads didn't actually get
> any signal.

Yes, it does make sense.  And it was what I was expecting to see.
However, I cannot really remember how I came to the conclusion I wrote
above, since I did the same things that you did.

>> Thus, using
>> "bfd_core_file_failing_signal" is OK since there is no concept of "this
>> signal number killed only this thread".
>> 
>> 
>> Case (3) is the most difficult IMO.  I don't know how we are going to
>> handle it when I/we implement NT_SIGINFO generation on PRSTATUS.  My
>> first reaction is to do it using the same logic as the Linux kernel,
>> i.e., putting the same signal number in every thread's siginfo.  But I
>> don't think we should bikeshed too much now, so I'm stopping my e-mail
>> here.
>> 
>> I'd like to hear opinions.
>
> I can't say I really understand how any of that argues against my
> original rationale for not setting $_exitsignal on corefiles (because
> the inferior has not really exited at the point the core has been
> generated), rather than point at implementation choices.

Interesting.  I thought setting it made sense because it seems to me
that the inferior has exited when the corefile has been generated.  I am
clearly missing some knowledge here, then...

> Now, if one were to instead argue that _user interface_ -wise, it'd
> make sense to set $_exitsignal, because we also print
> "Program terminated with signal", (emphasis on "terminated"), then
> I'd agree:
>
>   siggy = bfd_core_file_failing_signal (core_bfd);
>   if (siggy > 0)
>     {
> ...
>       printf_filtered (_("Program terminated with signal %s, %s.\n"),
> 		       gdb_signal_to_name (sig), gdb_signal_to_string (sig));
>     }

...or not.  Apparently, you are differentiating between "exited" and
"terminated", right?  Could you expand a little more on this?

And BTW, I guess my reasoning for setting $_exitsignal here is indeed
because we already assume that the inferior has been terminated (or
exited?) indeed.

Thanks,

-- 
Sergio

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Pedro Alves]

On 09/17/2013 01:11 AM, Sergio Durigan Junior wrote:

>> I can't say I really understand how any of that argues against my
>> original rationale for not setting $_exitsignal on corefiles (because
>> the inferior has not really exited at the point the core has been
>> generated), rather than point at implementation choices.
> 
> Interesting.  I thought setting it made sense because it seems to me
> that the inferior has exited when the corefile has been generated.  I am
> clearly missing some knowledge here, then...

Then that's what you need to argue.  ;-) My point of view was that
when inspecting a core dump, you're seeing the state of the
program _before_ it exited.  E.g, if you stop the program (with a
breakpoint, ctrl-c, or even "interrupt -a"/non-stop), do "gcore", and
then debug the resulting core, the program never exited.

>> Now, if one were to instead argue that _user interface_ -wise, it'd
>> make sense to set $_exitsignal, because we also print
>> "Program terminated with signal", (emphasis on "terminated"), then
>> I'd agree:
>>
>>   siggy = bfd_core_file_failing_signal (core_bfd);
>>   if (siggy > 0)
>>     {
>> ...
>>       printf_filtered (_("Program terminated with signal %s, %s.\n"),
>> 		       gdb_signal_to_name (sig), gdb_signal_to_string (sig));
>>     }
> 
> ...or not.  Apparently, you are differentiating between "exited" and
> "terminated", right?  Could you expand a little more on this?

No, not differentiating.  The emphasis was for pointing out that GDB already
claims the program has indeed terminated, so from that perspective, it does
indeed make sense to set $_exitsignal, otherwise, to make things consistent,
_I_, in turn, would need to argue for removing that "Program terminated with
signal" message.  I don't plan to, so I get to agree with setting $_exitsignal.
See, this is all about _user interface_, not random internal implementation
choices, which will naturally change over time, but the user interface design
decision will most probably outlast that for a long while.

> And BTW, I guess my reasoning for setting $_exitsignal here is indeed
> because we already assume that the inferior has been terminated (or
> exited?) indeed.

Thanks,
-- 
Pedro Alves

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I can't say I really understand how any of that argues against my
Pedro> original rationale for not setting $_exitsignal on corefiles (because
Pedro> the inferior has not really exited at the point the core has been
Pedro> generated), rather than point at implementation choices.

Pedro> Now, if one were to instead argue that _user interface_ -wise, it'd
Pedro> make sense to set $_exitsignal, because we also print
Pedro> "Program terminated with signal", (emphasis on "terminated"), then
Pedro> I'd agree:

I may have missed part of the thread, but what is the rationale for
being so precise here?  It seems a bit pedantic to me -- which is fine
if there is a purpose to it, but I couldn't think of one in this case.

That is, gdb has a bit of information that is relevant to the user.  It
is useful to users if we expose it to them in a script-friendly way.  We
already have $_exitsignal, and differentiating between "the program
exited with signal X" and "the program is about to exit with signal X"
doesn't seem particularly handy.

Another consideration along these lines is that I have a branch in
progress for "catch exit" -- it's been waiting for Sergio's work on
these convenience variables.  I think here as well $_exitsignal seems
like a natural fit, even though the process has not technically exited
at the catchpoint.

Tom

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Philippe Waroquiers]

On Tue, 2013-09-17 at 12:36 -0600, Tom Tromey wrote:

> Another consideration along these lines is that I have a branch in
> progress for "catch exit" -- it's been waiting for Sergio's work on
> these convenience variables.  I think here as well $_exitsignal seems
> like a natural fit, even though the process has not technically exited
> at the catchpoint.
Will there be (significant) functional differences between
    "catch exit"
and
    "catch syscall exit exit_group" ?

Philippe

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> On Tue, 2013-09-17 at 12:36 -0600, Tom Tromey wrote:
>> Another consideration along these lines is that I have a branch in
>> progress for "catch exit" -- it's been waiting for Sergio's work on
>> these convenience variables.  I think here as well $_exitsignal seems
>> like a natural fit, even though the process has not technically exited
>> at the catchpoint.

Philippe> Will there be (significant) functional differences between
Philippe>     "catch exit"
Philippe> and
Philippe>     "catch syscall exit exit_group" ?

Does the "catch syscall" approach work when the exit is due to a signal?
I would have thought not, but I did not test it.
"catch exit" works via PTRACE_O_TRACEEXIT.

We go back and forth about the utility of it, FWIW.
I wrote most of what I did just to learn more about infrun and the
remote protocol; then one day we found some actual use for it :-)

Tom

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Sergio Durigan Junior]

On Tuesday, September 17 2013, Philippe Waroquiers wrote:

> On Tue, 2013-09-17 at 12:36 -0600, Tom Tromey wrote:
>
>> Another consideration along these lines is that I have a branch in
>> progress for "catch exit" -- it's been waiting for Sergio's work on
>> these convenience variables.  I think here as well $_exitsignal seems
>> like a natural fit, even though the process has not technically exited
>> at the catchpoint.
> Will there be (significant) functional differences between
>     "catch exit"
> and
>     "catch syscall exit exit_group" ?

I think this is more like "catch fork" and "catch syscall fork clone".
ptrace already offers PTRACE_O_TRACEEXIT so we could be using that even
when "catch syscall" is not fully functional on the target (i.e.,
there's no XML file to translate syscalls names to numbers).  But Tom
may correct me here.

-- 
Sergio

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Pedro Alves]

On 09/17/2013 07:53 PM, Philippe Waroquiers wrote:
> On Tue, 2013-09-17 at 12:36 -0600, Tom Tromey wrote:
> 
>> Another consideration along these lines is that I have a branch in
>> progress for "catch exit" -- it's been waiting for Sergio's work on
>> these convenience variables.  I think here as well $_exitsignal seems
>> like a natural fit, even though the process has not technically exited
>> at the catchpoint.
> Will there be (significant) functional differences between
>     "catch exit"
> and
>     "catch syscall exit exit_group" ?

"catch exit" implemented with PTRACE_O_TRACEEXIT allows catching
SIGKILLs.  "catch syscall" doesn't.  See:

  [PR 15812 - catch death by SIGKILL]
  https://sourceware.org/bugzilla/show_bug.cgi?id=15812

-- 
Pedro Alves

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Pedro Alves]

On 09/17/2013 07:36 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I can't say I really understand how any of that argues against my
> Pedro> original rationale for not setting $_exitsignal on corefiles (because
> Pedro> the inferior has not really exited at the point the core has been
> Pedro> generated), rather than point at implementation choices.
> 
> Pedro> Now, if one were to instead argue that _user interface_ -wise, it'd
> Pedro> make sense to set $_exitsignal, because we also print
> Pedro> "Program terminated with signal", (emphasis on "terminated"), then
> Pedro> I'd agree:
> 
> I may have missed part of the thread, but what is the rationale for
> being so precise here?  It seems a bit pedantic to me -- which is fine
> if there is a purpose to it, but I couldn't think of one in this case.

Well, guess you've missed it, because from the beginning I had already
introduced that as pedantic:

 https://sourceware.org/ml/gdb-patches/2013-06/msg00337.html

Also, if you read downthread, you'll notice that I have agreed to being lax
here, just not the presented rationale for it.  If we're deciding to set the
flag for cores, then IMO, it's important such decisions are conscientiously
based on a user interface decisions, not of the style "I think it'd make
sense, but I'd have to do such and such".

> That is, gdb has a bit of information that is relevant to the user.  It
> is useful to users if we expose it to them in a script-friendly way.  We
> already have $_exitsignal, and differentiating between "the program
> exited with signal X" and "the program is about to exit with signal X"
> doesn't seem particularly handy.

Consider non-stop, and using gcore for snapshotting the program state.
There's no termination/exit at all.  In fact, there's are potentially many
threads in the program, and each of them, if stopped, should have its own
signal number.  You should be able to get at all of them with
$_siginfo.si_signo, but it's just that older kernels don't have that info.
Let's pretend kernels did always write NT_SIGINFO.  Would we be arguing
for making $_exitsignal work for cores, given that $si_siginfo.si_signo would
work?  It's plausible.  And that's why I'm not against this at all.  I just
wanted to make sure that design decision is use-case driven, rather than
because on Linux's current implementation such-and-such happens.  IOW, I wanted
that to be recorded on the archives, so if even if the core formats change
in the future, we can refer back to today's decisions.

> Another consideration along these lines is that I have a branch in
> progress for "catch exit" -- it's been waiting for Sergio's work on
> these convenience variables.  I think here as well $_exitsignal seems
> like a natural fit, even though the process has not technically exited
> at the catchpoint.

I see, thanks.  Sounds reasonable.

-- 
Pedro Alves

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Tom Tromey]

Pedro> Well, guess you've missed it, because from the beginning I had already
Pedro> introduced that as pedantic:

Thanks.

Pedro> Consider non-stop, and using gcore for snapshotting the program
Pedro> state.  There's no termination/exit at all.  In fact, there's are
Pedro> potentially many threads in the program, and each of them, if
Pedro> stopped, should have its own signal number.  You should be able
Pedro> to get at all of them with $_siginfo.si_signo, but it's just that
Pedro> older kernels don't have that info.  Let's pretend kernels did
Pedro> always write NT_SIGINFO.  Would we be arguing for making
Pedro> $_exitsignal work for cores, given that $si_siginfo.si_signo
Pedro> would work?  It's plausible.  And that's why I'm not against this
Pedro> at all.  I just wanted to make sure that design decision is
Pedro> use-case driven, rather than because on Linux's current
Pedro> implementation such-and-such happens.  IOW, I wanted that to be
Pedro> recorded on the archives, so if even if the core formats change
Pedro> in the future, we can refer back to today's decisions.

Thanks, that is a nice summary for those of us who lost track.  Which
may just be me, but still.  Also, let me say that I continue to find it
delightful how you have all the hard cases at hand, ready to roll out
over my simplistic arguments :-)

>> Another consideration along these lines is that I have a branch in
>> progress for "catch exit" -- it's been waiting for Sergio's work on
>> these convenience variables.  I think here as well $_exitsignal seems
>> like a natural fit, even though the process has not technically exited
>> at the catchpoint.

Pedro> I see, thanks.  Sounds reasonable.

I see now from your explanation that the cases are in fact different.
The "catch exit" case is more clearly a direct mapping.

Tom

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> To the extent of my knowledge, this is a shell feature.  It sets $? (the
Sergio> variable which contains the exit code of the program) to 128 + signal
Sergio> number.

No, it's actually how things work under the hood.
When you call 'wait' (or one of its many equivalent APIs), the result
you get is an integer that encodes whether the program exited normally
or via signal; if normally, the exit code; or if via signal, which signal.

This touches on Doug's question of how to use these variables
generically.  There's no nice way for a script to use this right now,
partly because gdb's scripting is so limited, but also because the two
values are put into two separate variables.

Just having a variable that is "the exit status as the OS reports it"
seems nice, but then it means reimplementing WIFEXITED and friends.  Not
perhaps the friendliest.

Tom

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Pedro Alves]

On 06/16/2013 07:25 AM, Sergio Durigan Junior wrote:
> @@ -3455,6 +3455,12 @@ handle_inferior_event (struct execution_control_state *ecs)
>  	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
>  				   (LONGEST) ecs->ws.value.integer);
>  
> +	  /* Clear the internal variable, since if we are here chances
> +	     are the inferior has not been terminated by a signal.
> +	     And even if it has, then GDB will get to
> +	     TARGET_WAITKIND_SIGNALLED in time...  */

No it won't.  The target either returns TARGET_WAITKIND_EXITED
or TARGET_WAITKIND_SIGNALLED.  They're mutually exclusive.

> +	  clear_internalvar (lookup_internalvar ("_exitsignal"));
> +
>  	  /* Also record this in the inferior itself.  */
>  	  current_inferior ()->has_exit_code = 1;
>  	  current_inferior ()->exit_code = (LONGEST) ecs->ws.value.integer;
> @@ -3462,7 +3468,17 @@ handle_inferior_event (struct execution_control_state *ecs)
>  	  print_exited_reason (ecs->ws.value.integer);
>  	}
>        else
> -	print_signal_exited_reason (ecs->ws.value.sig);
> +	{
> +	  print_signal_exited_reason (ecs->ws.value.sig);
> +	  /* Set the value of the internal variable $_exitsignal,
> +	     which holds the signal uncaught by the inferior.  */
> +	  set_internalvar_integer (lookup_internalvar ("_exitsignal"),
> +				   (LONGEST) ecs->ws.value.sig);
> +
> +	  /* Clear the $_exitcode internal variable, because if the
> +	     inferior signalled then its return code does not exist.  */

s/the inferior signalled/the inferior died with a signal/ ?

> +	  clear_internalvar (lookup_internalvar ("_exitcode"));
> +	}

-- 
Pedro Alves

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Pedro Alves]

On 06/17/2013 06:20 PM, Pedro Alves wrote:
> On 06/16/2013 07:25 AM, Sergio Durigan Junior wrote:
>> @@ -3455,6 +3455,12 @@ handle_inferior_event (struct execution_control_state *ecs)
>>  	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
>>  				   (LONGEST) ecs->ws.value.integer);
>>  
>> +	  /* Clear the internal variable, since if we are here chances
>> +	     are the inferior has not been terminated by a signal.
>> +	     And even if it has, then GDB will get to
>> +	     TARGET_WAITKIND_SIGNALLED in time...  */
> 
> No it won't.  The target either returns TARGET_WAITKIND_EXITED
> or TARGET_WAITKIND_SIGNALLED.  They're mutually exclusive.

BTW, it'd be thorough if the test also exercised these clearings.
That is, check that $_exitcode is cleared when the inferior
terminates with a signal.  Check that $_exitsignal is cleared
when the inferior exits normally.

-- 
Pedro Alves

Re: [RFC/PATCH] New convenience variable $_exitsignal

[Sergio Durigan Junior]

On Monday, June 17 2013, Pedro Alves wrote:

> On 06/16/2013 07:25 AM, Sergio Durigan Junior wrote:
>> @@ -3455,6 +3455,12 @@ handle_inferior_event (struct execution_control_state *ecs)
>>  	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
>>  				   (LONGEST) ecs->ws.value.integer);
>>  
>> +	  /* Clear the internal variable, since if we are here chances
>> +	     are the inferior has not been terminated by a signal.
>> +	     And even if it has, then GDB will get to
>> +	     TARGET_WAITKIND_SIGNALLED in time...  */
>
> No it won't.  The target either returns TARGET_WAITKIND_EXITED
> or TARGET_WAITKIND_SIGNALLED.  They're mutually exclusive.

You're right, I confused the matters here, I was debugging with "debug
infrun" and saw a _STOPPED then a _SIGNALLED, and my mind twisted when I
wrote that comment.

>> +	  clear_internalvar (lookup_internalvar ("_exitsignal"));
>> +
>>  	  /* Also record this in the inferior itself.  */
>>  	  current_inferior ()->has_exit_code = 1;
>>  	  current_inferior ()->exit_code = (LONGEST) ecs->ws.value.integer;
>> @@ -3462,7 +3468,17 @@ handle_inferior_event (struct execution_control_state *ecs)
>>  	  print_exited_reason (ecs->ws.value.integer);
>>  	}
>>        else
>> -	print_signal_exited_reason (ecs->ws.value.sig);
>> +	{
>> +	  print_signal_exited_reason (ecs->ws.value.sig);
>> +	  /* Set the value of the internal variable $_exitsignal,
>> +	     which holds the signal uncaught by the inferior.  */
>> +	  set_internalvar_integer (lookup_internalvar ("_exitsignal"),
>> +				   (LONGEST) ecs->ws.value.sig);
>> +
>> +	  /* Clear the $_exitcode internal variable, because if the
>> +	     inferior signalled then its return code does not exist.  */
>
> s/the inferior signalled/the inferior died with a signal/ ?

Yes, will change.  I will resubmit the patch with all the modifications
(by Eli, Doug and you) next.

-- 
Sergio