Re: [PATCH v4] Implement $_exitsignal

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

On Monday, September 23 2013, I wrote:

> On Tuesday, September 17 2013, I wrote:
>
>> Hi,
>>
>> This is the fourth attempt to implement $_exitsignal.  You can read the
>> last 3 at:
>>
>> - First attempt:
>>
>> <http://sourceware.org/ml/gdb-patches/2013-06/msg00352.html>
>>
>> - Second attempt:
>>
>> <http://sourceware.org/ml/gdb-patches/2013-06/msg00452.html>
>>
>> - Third attempt:
>>
>> <https://sourceware.org/ml/gdb-patches/2013-08/msg00727.html>
>
> Ping.

Ping^2.

> The documentation part has been already reviewed and approved.
>
> Thanks,
>
>> The patch has suffered some changes since the third attempt.  In the
>> code, I basically addressed Pedro's requests of clearing both
>> $_exit{signal,code} when there is no signal recognized at the corefile,
>> and (most important) to drop the process of filling $_exitsignal if the
>> target doesn't provide a way of converting between GDB's internal signal
>> numbers and the target's.
>>
>> In the documentation field, I wrote a more extensive section explaining
>> how to use $_exit{code,signal} together in a GDB script, by making use
>> of the new convenience function $_isvoid.  This was Doug's request.
>>
>> I have also obviously added some testcases, but they are unchanged since
>> the last attempt.
>>
>> OK to apply?  Thanks,
>>
>> -- 
>> Sergio
>>
>> gdb/
>> 2013-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* NEWS: Mention new convenience variable $_exitsignal.
>> 	* corelow.c (core_open): Set $_exitsignal to the uncaught signal
>> 	which generated the corefile; reset $_exitcode to void.
>> 	* infrun.c (handle_inferior_event): Reset $_exitsignal for
>> 	TARGET_WAITKIND_EXITED; set $_exitsignal and reset $_exitcode for
>> 	TARGET_WAITKIND_SIGNALLED.
>>
>> gdb/testsuite/
>> 2013-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* gdb.base/corefile.exp: Test whether $_exitsignal is set and
>> 	$_exitcode is void when opening a corefile.
>> 	* gdb.base/exitsignal.exp: New file.
>> 	* gdb.base/segv.c: Likewise.
>> 	* gdb.base/normal.c: Likewise.
>>
>> gdb/doc/
>> 2013-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* gdb.texinfo (Convenience Variables): Document $_exitsignal.
>> 	Update entry for $_exitcode.
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index af06a21..4c38358 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -87,6 +87,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 terminating 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/corelow.c b/gdb/corelow.c
>> index 8371b58..9088afb 100644
>> --- a/gdb/corelow.c
>> +++ b/gdb/corelow.c
>> @@ -446,7 +446,22 @@ core_open (char *filename, int from_tty)
>>  
>>        printf_filtered (_("Program terminated with signal %s, %s.\n"),
>>  		       gdb_signal_to_name (sig), gdb_signal_to_string (sig));
>> +
>> +      /* Set the value of the internal variable $_exitsignal,
>> +	 which holds the signal uncaught by the inferior.  */
>> +      set_internalvar_integer (lookup_internalvar ("_exitsignal"),
>> +			       siggy);
>>      }
>> +  else
>> +    {
>> +      /* The inferior did not die because of an uncaught signal,
>> +	 therefore we must clear $_exitsignal.  */
>> +      clear_internalvar (lookup_internalvar ("_exitsignal"));
>> +    }
>> +
>> +  /* Clear the $_exitcode internal variable, because there is no way
>> +     the inferior could have exited and generated a corefile.  */
>> +  clear_internalvar (lookup_internalvar ("_exitcode"));
>>  
>>    /* Fetch all registers from core file.  */
>>    target_fetch_registers (get_current_regcache (), -1);
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 60d2877..104fe2a 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -9751,8 +9751,64 @@ 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.
>> +When the program being debugged terminates normally, @value{GDBN}
>> +automatically sets this variable to the exit code of the program, and
>> +resets @code{$_exitsignal} to @code{void}.
>> +
>> +@item $_exitsignal
>> +@vindex $_exitsignal@r{, convenience variable}
>> +When the program being debugged dies due to an uncaught signal,
>> +@value{GDBN} automatically sets this variable to that signal's number,
>> +and resets @code{$_exitcode} to @code{void}.
>> +
>> +To distinguish between whether the program being debugged has exited
>> +(i.e., @code{$_exitcode} is not @code{void}) or signalled (i.e.,
>> +@code{$_exitsignal} is not @code{void}), the convenience function
>> +@code{$_isvoid} can be used (see @ref{Convenience Funs,, Convenience
>> +Functions}).  For example, considering the following source code:
>> +
>> +@smallexample
>> +#include <signal.h>
>> +
>> +int
>> +main (int argc, char *argv[])
>> +@{
>> +  raise (SIGALRM);
>> +  return 0;
>> +@}
>> +@end smallexample
>> +
>> +A valid way of telling whether the program being debugged has exited
>> +or signalled would be:
>> +
>> +@smallexample
>> +(@value{GDBP}) define has_exited_or_signalled
>> +Type commands for definition of ``has_exited_or_signalled''.
>> +End with a line saying just ``end''.
>> +>if $_isvoid ($_exitsignal)
>> + >echo The program has exited\n
>> + >else
>> + >echo The program has signalled\n
>> + >end
>> +>end
>> +(@value{GDBP}) run
>> +Starting program:
>> +
>> +Program terminated with signal SIGALRM, Alarm clock.
>> +The program no longer exists.
>> +(@value{GDBP}) has_exited_or_signalled
>> +The program has signalled
>> +@end smallexample
>> +
>> +As can be seen, @value{GDBN} correctly informs that the program being
>> +debugged has signalled, since it calls @code{raise} and raises a
>> +@code{SIGALRM} signal.  If the program being debugged had not called
>> +@code{raise}, then @value{GDBN} would report a normal exit:
>> +
>> +@smallexample
>> +(@value{GDBP}) has_exited_or_signalled
>> +The program has exited
>> +@end smallexample
>>  
>>  @item $_exception
>>  The variable @code{$_exception} is set to the exception object being
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 57618ae..e356f8d 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -3432,6 +3432,10 @@ handle_inferior_event (struct execution_control_state *ecs)
>>  	  set_internalvar_integer (lookup_internalvar ("_exitcode"),
>>  				   (LONGEST) ecs->ws.value.integer);
>>  
>> +	  /* Clear the $_exitsignal internal variable, since if we are
>> +	     here the inferior has not been terminated by a signal.  */
>> +	  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;
>> @@ -3439,7 +3443,41 @@ handle_inferior_event (struct execution_control_state *ecs)
>>  	  print_exited_reason (ecs->ws.value.integer);
>>  	}
>>        else
>> -	print_signal_exited_reason (ecs->ws.value.sig);
>> +	{
>> +	  struct regcache *regcache = get_thread_regcache (ecs->ptid);
>> +	  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> +
>> +	  if (gdbarch_gdb_signal_to_target_p (gdbarch))
>> +	    {
>> +	      /* Set the value of the internal variable $_exitsignal,
>> +		 which holds the signal uncaught by the inferior.  */
>> +	      set_internalvar_integer (lookup_internalvar ("_exitsignal"),
>> +				       gdbarch_gdb_signal_to_target (gdbarch,
>> +							  ecs->ws.value.sig));
>> +	    }
>> +	  else
>> +	    {
>> +	      static int have_warned_signal_to_target = 0;
>> +
>> +	      if (!have_warned_signal_to_target)
>> +		{
>> +		  have_warned_signal_to_target = 1;
>> +		  warning (_("\
>> +Cannot fill $_exitsignal with the correct signal number."));
>> +		}
>> +
>> +	      /* Clearing the $_exitsignal convenience variable, since
>> +		 it is not possible to fill it correctly.  */
>> +	      clear_internalvar (lookup_internalvar ("_exitsignal"));
>> +	    }
>> +
>> +	  print_signal_exited_reason (ecs->ws.value.sig);
>> +
>> +	  /* Clear the $_exitcode internal variable, because if the
>> +	     inferior died with a signal 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/corefile.exp b/gdb/testsuite/gdb.base/corefile.exp
>> index 24a33a3..77ef954 100644
>> --- a/gdb/testsuite/gdb.base/corefile.exp
>> +++ b/gdb/testsuite/gdb.base/corefile.exp
>> @@ -142,6 +142,16 @@ gdb_test "print coremaker_ro" "\\\$$decimal = 201"
>>  
>>  gdb_test "print func2::coremaker_local" "\\\$$decimal = \\{0, 1, 2, 3, 4\\}"
>>  
>> +# Test the presence and the correct values of $_exitsignal and
>> +# $_exitcode variables.  The corefile is generated with a SIGABRT,
>> +# which is "6" in the Linux kernel.
>> +
>> +gdb_test "print \$_exitsignal" " = 6" \
>> +    "\$_exitsignal prints SIGABRT (6)"
>> +
>> +gdb_test "print \$_exitcode" " = void" \
>> +    "\$_exitcode is void"
>> +
>>  # Somehow we better test the ability to read the registers out of the core
>>  # file correctly.  I don't think the other tests do this.
>>  
>> diff --git a/gdb/testsuite/gdb.base/exitsignal.exp b/gdb/testsuite/gdb.base/exitsignal.exp
>> new file mode 100644
>> index 0000000..5ab2fee
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/exitsignal.exp
>> @@ -0,0 +1,97 @@
>> +# 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 checks both $_exitcode and $_exitsignal variables.  The
>> +# purpose of this checking is to ensure that the variables are
>> +# mutually exclusive, i.e., that when $_exitsignal is set, $_exitcode
>> +# is not, and vice-versa.  This mutual exclusion happens because if an
>> +# inferior exited (either successfuly or not), it certainly was not
>> +# killed by a signal.  However, if it was killed by an uncaught
>> +# signal, then there is no way for it to have exited.
>> +
>> +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"
>> +
>> +# Just to guarantee, making sure that $_exitcode is also void.
>> +gdb_test "print \$_exitcode" " = void" \
>> +    "\$_exitcode 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" \
>> +    "\$_exitsignal is 11 (SIGSEGV) after SIGSEGV."
>> +
>> +# And $_exitcode should still be void, since the inferior died because
>> +# of a signal, and did not return.
>> +gdb_test "print \$_exitcode" " = void" \
>> +    "\$_exitcode is still void after SIGSEGV"
>> +
>> +# Re-run to main, i.e., restart the executable.
>> +rerun_to_main
>> +
>> +# 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" \
>> +    "\$_exitsignal is 11 (SIGSEGV) after restarting the inferior"
>> +
>> +# And, again, print $_exitcode.
>> +gdb_test "print \$_exitcode" " = void" \
>> +    "\$_exitcode is still void after restarting the inferior"
>> +
>> +# Now we test the behaviour of $_exit{code,signal} during a normal
>> +# inferior execution.
>> +standard_testfile normal.c
>> +
>> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
>> +    return -1
>> +}
>> +
>> +# Checking $_exitsignal and $_exitcode, both should be void before the
>> +# inferior is executed.
>> +gdb_test "print \$_exitsignal" " = void" \
>> +    "\$_exitsignal is void before normal inferior is executed"
>> +gdb_test "print \$_exitcode" " = void" \
>> +    "\$_exitcode is void before normal inferior is executed"
>> +
>> +# Running until the end.
>> +gdb_test "run" "\\\[Inferior .* exited normally\\\]" "Run inferior until the end"
>> +
>> +# Checking $_exitcode.  It should be 0.
>> +gdb_test "print \$_exitcode" " = 0" \
>> +    "\$_exitcode is zero after normal inferior is executed"
>> +
>> +# Checking $_exitsignal.  It should still be void, since the inferior
>> +# has not received any signal.
>> +gdb_test "print \$_exitsignal" " = void" \
>> +    "\$_exitsignal is still void after normal inferior is executed"
>> diff --git a/gdb/testsuite/gdb.base/normal.c b/gdb/testsuite/gdb.base/normal.c
>> new file mode 100644
>> index 0000000..4aa7c45
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/normal.c
>> @@ -0,0 +1,24 @@
>> +/* 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 is just a normal return 0.  */
>> +
>> +int
>> +main (int argc, char *argv[])
>> +{
>> +  return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.base/segv.c b/gdb/testsuite/gdb.base/segv.c
>> new file mode 100644
>> index 0000000..8991f4d
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/segv.c
>> @@ -0,0 +1,29 @@
>> +/* 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 <signal.h>
>> +
>> +int
>> +main (int argc, char *argv[])
>> +{
>> +  /* Generating a SIGSEGV.  */
>> +  raise (SIGSEGV);
>> +
>> +  return 0;
>> +}
>
> -- 
> Sergio

-- 
Sergio