Re: [PATCHv3 0/2] Fix for an assertion when unwinding with inline frames

Simon Marchi via Gdb-patches
2021-07-26T13:58:07Z

On 2021-07-26 7:11 a.m., Andrew Burgess wrote:
> * Simon Marchi <simon.marchi@polymtl.ca> [2021-07-20 17:59:45 -0400]:
>
>> On 2021-07-20 5:10 a.m., Andrew Burgess wrote:
>>> Thanks for the feedback on v2.
>>>
>>> In v3 I have:
>>>
>>> - Addressed all of Pedro's feedback on the test in patch #1.
>>>
>>> - Rewritten how the problem in patch #1 is fixed based on Simon's
>>> exception based approach.
>>>
>>> Thanks,
>>> Andrew
>>
>> When speaking to Pedro off-line, it did sound like he had concerns with
>> what I proposed, so let's wait to hear what he has to say.
>
> That's fine. Hopefully Pedro will be able to offer some feedback
> soon.
>
> Before then, I've spun off patch #2 and an additional fix related to
> 'set debug frame on' into a separate patch (below). I think all of
> this code is unrelated to the whole should we use an exception, or
> change the API of get_prev_frame.
>
> What are your thoughts on this patch?
>
> Additionally, this one might be a possible candidate for merging into
> gdb-11-branch. Here's a ChangeLog if we decide that's a good idea.
>
> gdb/ChangeLog:
> * frame.c (get_prev_frame_always_1): Handle case where
> this_frame->prev is nullptr.
> (get_prev_frame_id_by_id): Delete.
> * frame.h (get_prev_frame_id_by_id): Delete declaration.
> * valops.c (value_assign): Remove use of VALUE_FRAME_ID.
> * vaule.c (value_fetch_lazy_register): Likewise.
> * value.h (VALUE_FRAME_ID): Delete.
>
> gdb/testsuite/ChangeLog:
> * gdb.base/premature-dummy-frame-removal.exp: Repeat test with
> 'set debug frame on' in effect.
>
> Thanks,
> Andrew

The patch below does LGTM, thanks.

Simon