Re: [PATCH v2] Add Guile frame unwinder interface

[Andy Wingo <wingo@igalia.com>]

Hi,

On Mon 09 Mar 2015 16:41, Pedro Alves <palves@redhat.com> writes:

> On 03/09/2015 10:34 AM, Andy Wingo wrote:
>> +@var{register} names a register, and should be a string, for example
>> +@samp{rip}.  @var{value} is the register value, as a @value{GDBN}
>> +value.  Alternately, passing @code{#f} as the value will mark the
>> +register as unavailable.
>
> From a glimpse over the code, I think this actually marks it as
> "<not saved>" (optimized out), right?  That would be the correct
> thing to do.  Marking a register as "<unavailable>" is also possible,
> but it is a different thing -- it means the value exists, but gdb
> couln't get to it, because e.g., the core file is trimmed, or the
> ptrace interface is missing access to some registers.
>
> That said, you may want to consider how you'd expand the API
> to allow marking registers as unavailable.

I didn't realize that "unavailable" and "not saved" were different
things, thanks for the pointer.  I guess given that the default is a
"not saved" result, I can just document this default state, and that
ephemeral-frame-add-saved-value! adds a value.  We remove the #f case.

If we need to support other states like "unavailable", we can add other
API like ephemeral-frame-mark-unavailable! or similar.

>> +  is_unwinding = 1;
>>    TRY_CATCH (ex, RETURN_MASK_ERROR)
>>      {
>>        res = unwinder->sniffer (unwinder, this_frame, this_cache);
>>      }
>> +  is_unwinding = 0;
>
> Note that RETURN_MASK_ERROR does not catch all exceptions (notably,
> a ctrl-c/QUIT passes right through uncaught).

Ah indeed.  I'll move the is_unwinding=1 to a cleanup inside the TRY_CATCH.

Thanks for the review,

Andy