Re: [PATCHv2] Make "skip" work on inline frames

Simon Marchi
2019-10-27T02:18:03Z

On 2019-10-26 9:52 p.m., Simon Marchi wrote:
> On 2019-10-20 2:48 a.m., Bernd Edlinger wrote:
>> On 10/19/19 6:38 AM, Bernd Edlinger wrote:
>>> Hmm,
>>>
>>> I noticed that the patch does not yet handle
>>> the step <count> correctly, the count is decremented
>>> although the inline frame is skipped and should not be
>>> counted...
>>>
>>> Thus I will need to change at least this:
>>>
>>> --- a/gdb/infcmd.c
>>> +++ b/gdb/infcmd.c
>>> @@ -1122,7 +1122,6 @@ prepare_one_step (struct step_command_fsm *sm)
>>> set_running (resume_ptid, 1);
>>>
>>> step_into_inline_frame (tp);
>>> - sm->count--;
>>>
>>> sal = find_frame_sal (frame);
>>> sym = get_frame_function (frame);
>>> @@ -1132,13 +1131,17 @@ prepare_one_step (struct step_command_fsm *sm)
>>>
>>> if (sal.line == 0
>>> || !function_name_is_marked_for_skip (fn, sal))
>>> - return prepare_one_step (sm);
>>> + {
>>> + sm->count--;
>>> + return prepare_one_step (sm);
>>> + }
>>> }
>>>
>>>
>>
>> Attached is an updated patch that fixes this issue,
>> and also adds the following after step_into_inline_frame ():
>>
>> frame = get_current_frame ();
>>
>> That I consider safer, since this function calls reinit_frame_cache ().
>> It was probably just by chance that this did not seem to cause any
>> problems for me.
>>
>>
>> Thanks
>> Bernd.
>
> Hi Bernd,
>
> Sorry for the delay. I'll start looking at this patch, but I first need to play with
> it a bit first and get more familiar with that area of the code.
>
> In the mean time, I looked for your name in the copyright assignment list, and don't find
> it. I think this patch is large enough to warrant one Do you already have one in place?
> If not, please follow instructions here:
>
> https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future
>
> Simon

Oh, and I noticed that the patch doesn't come with a test, we'll need one before getting
the patch in. There are already some skip tests at testsuite/gdb.base/skip*.exp, so I
could very well imagine a new test named gdb.base/skip-inline.exp.

See these pages for details on how to write and run tests:

- https://sourceware.org/gdb/wiki/GDBTestcaseCookbook
- https://sourceware.org/gdb/wiki/TestingGDB

If you can't manage to make a test, at the very least please provide a minimal reproducer
so somebody else will be able to translate that into a test.

Thanks,

Simon