Re: [RFC/PATCH] Add new internal variable $_signo

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

On Friday, June 14 2013, Pedro Alves wrote:

> On 06/14/2013 01:37 AM, Sergio Durigan Junior wrote:
>> -	print_signal_exited_reason (ecs->ws.value.sig);
>> +	{
>> +	  print_signal_exited_reason (ecs->ws.value.sig);
>> +	  /* Set the value of the internal variable $_signo to hold the
>> +	     signal number received by the inferior.  */
>> +	  set_internalvar_integer (lookup_internalvar ("_signo"),
>> +				   (LONGEST) ecs->ws.value.sig);
>> +	}
>
> This bit is actually implementing a counterpart of the existing
> $_exitcode.  At this point the inferior is gone already,
> and $_siginfo couldn't ever work, even if the backend supports it.
>
> This case is actually a different scenario from $_siginfo.  I think
> it'd make more sense to call this one $_exitsignal, as sibling
> of the existing $_exitcode.  $_exitcode records the inferior's exit
> code for normal exits, $_exitsignal would record the signal number
> for signal exits.  Like $_exitcode, $_exitsignal should print the
> same signal number until the inferior exits again, no matter which
> thread you're looking at.  Looked like that, $_exitsignal just
> seems like an obvious omission.

OK, I have a patch for $_exitcode now, will send soon.  I've made some
assumptions, will explain them when I send the patch.

> Since when debugging a core file you're seeing a snapshot of the
> inferior _before_ it exited, I think pedantically it makes more
> sense for corelow to _not_ set $_exitsignal.  However, should
> still be able to get the info from the $_signo of the thread that
> exited (the one that gets selected when you load the core).

Yes, given the separation you proposed, I agree.

> So in sum, here are my suggestions:
>
> Add $_signo, but make it print the signal of the last event the
> thread received.  I'd add a last_status field in struct thread_info
> to record that info.  Don't implement $_signo as a regular
> integer convenience variable that gets set whenever a thread
> reports an event.  That won't work correctly in non-stop mode,
> where you can be looking at thread #1, while other threads
> could be hitting events (but the user remains with thread #1
> selected).  See how the $_thread variable is implemented as lazy
> convenience, very similar to what you need.

Thanks, I will work on it this Sunday, hope to have something work soon.

> If you're willing and I think it'd be a nice addition too,
> add $_exitsignal, and make it print the uncaught signal that made
> the process die.  Document it with something like:
>
> @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.
>
> I suggest making $_signo and $_exitsignal separate patches,
> even.

Yeah, a patch's coming.

> BTW, IMO, $_exitcode (and $_exitsignal) should print the exit
> code/signal of the selected inferior.  It was never adjusted
> for multi-inferior, so it always prints the exit code of the
> inferior that happened to exit last.  Simon's patch for
> displaying the inferior's exit-code in MI's -list-thread-groups records
> the info in the inferior struct.  Once that is in, we'd just need to
> make $_exitcode a lazy convenience var too.

OK, this paragraph mades sense but made me feel a bit confused.  Anyway,
I will send the $_exitsignal patch implemented as $_exitcode is
implemented now, and if you feel it should be different, you can comment
there.

>> The patch also contains a testcase and an update to the documentation in
>> order to mention the new convenience variable.
>
> The patch adds code to handle signalled live inferiors, but the
> test didn't exercise that.

I will add that in the next iteration.

>> +@item $_signo
>> +@vindex $_signo@r{, convenience variable}
>> +The variable @code{$_signo} contains the signal number received by the
>> +inferior.
>
>> +Its purpose is to serve as an auxiliary way of retrieving
>> +such information for when the kernel does not provide the necessary
>> +support for @code{$_siginfo} to be used.
>
> IMO, this whole sentence would be better reduced to: "Unlike $_siginfo
> below, $_signo is always available in all supported targets."
>
> $_signo should work on Windows, for example, where there's no
> concept of $_siginfo.

OK, fair enough.  I will update the docs as well.

Thanks,

-- 
Sergio