Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves <palves@redhat.com>]

On 02/09/2016 02:42 PM, Metzger, Markus T wrote:

>>> CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
>>> +  if (this_frame == NULL)
>>> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
>>
>> How can this happen?
> 
> One of its callers, frame_unwind_caller_pc, calls it with the result of
> skip_artificial_frames like this:
> 
> CORE_ADDR
> frame_unwind_caller_pc (struct frame_info *this_frame)
> {
>   return frame_unwind_pc (skip_artificial_frames (this_frame));
> }
> 
> Rather than handling the skip_artificial_frames() NULL return here,
> I made frame_unwind_pc handle a NULL frame argument.
> 
> I can move the check into frame_unwind_caller_pc if you prefer.

Yes, please.

Though, I think all these frame_unwind_caller_XXX methods should be
consistent in how they handle skip_artificial_frames (this_frame)
returning NULL, because they're all called together, assuming they're
referring to the same frame.  If we throw error here, then I think
we should throw in frame_unwind_caller_arch too, instead of having
that one return the arch of the next frame.

Thanks,
Pedro Alves