This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
- From: Pedro Alves <palves at redhat dot com>
- To: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 09 Feb 2016 22:01:42 +0000
- Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
- Authentication-results: sourceware.org; auth=none
- References: <1454681922-2228-1-git-send-email-markus dot t dot metzger at intel dot com> <1454681922-2228-3-git-send-email-markus dot t dot metzger at intel dot com> <56B9D620 dot 2020104 at redhat dot com> <A78C989F6D9628469189715575E55B233325FC44 at IRSMSX104 dot ger dot corp dot intel dot 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