[PING][PATCH] Fix reverse stepping multiple contiguous PC ranges
Luis Machado
luis.machado@linaro.org
Thu Jun 10 11:49:53 GMT 2021
Ping!
On 4/7/21 12:23 PM, Luis Machado wrote:
> Ping!
>
> On 3/19/21 3:17 PM, Luis Machado wrote:
>> Polite ping.
>>
>> On 3/12/21 12:36 PM, Luis Machado wrote:
>>> Polite ping.
>>>
>>> On 3/4/21 11:30 AM, Luis Machado wrote:
>>>> On 2/26/21 3:58 PM, Luis Machado wrote:
>>>>> On 2/11/21 8:38 AM, Luis Machado wrote:
>>>>>> On 2/1/21 4:19 PM, Luis Machado wrote:
>>>>>>> When running GDB's testsuite on aarch64-linux/Ubuntu 20.04, I
>>>>>>> noticed some
>>>>>>> failures in gdb.reverse/solib-precsave.exp and
>>>>>>> gdb.reverse/solib-reverse.exp.
>>>>>>>
>>>>>>> The failure happens around the following code:
>>>>>>>
>>>>>>> 38 b[1] = shr2(17); /* middle part two */
>>>>>>> 40 b[0] = 6; b[1] = 9; /* generic statement, end part two */
>>>>>>> 42 shr1 ("message 1\n"); /* shr1 one */
>>>>>>>
>>>>>>> Normal execution:
>>>>>>>
>>>>>>> - step from line 1 will land on line 2.
>>>>>>> - step from line 2 will land on line 3.
>>>>>>>
>>>>>>> Reverse execution:
>>>>>>>
>>>>>>> - step from line 3 will land on line 2.
>>>>>>> - step from line 2 will land on line 2.
>>>>>>> - step from line 2 will land on line 1.
>>>>>>>
>>>>>>> The problem here is that line 40 contains two contiguous but
>>>>>>> distinct
>>>>>>> PC ranges, like so:
>>>>>>>
>>>>>>> Line 40 - [0x7ec ~ 0x7f4]
>>>>>>> Line 40 - [0x7f4 ~ 0x7fc]
>>>>>>>
>>>>>>> When stepping forward from line 2, we skip both of these ranges
>>>>>>> and land on
>>>>>>> line 42. When stepping backward from line 3, we stop at the start
>>>>>>> PC of the
>>>>>>> second (or first, going backwards) range of line 40.
>>>>>>>
>>>>>>> This happens because we have this check in
>>>>>>> infrun.c:process_event_stop_test:
>>>>>>>
>>>>>>> /* When stepping backward, stop at beginning of line range
>>>>>>> (unless it's the function entry point, in which case
>>>>>>> keep going back to the call point). */
>>>>>>> CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
>>>>>>> if (stop_pc == ecs->event_thread->control.step_range_start
>>>>>>> && stop_pc != ecs->stop_func_start
>>>>>>> && execution_direction == EXEC_REVERSE)
>>>>>>> end_stepping_range (ecs);
>>>>>>> else
>>>>>>> keep_going (ecs);
>>>>>>>
>>>>>>> Since we've reached ecs->event_thread->control.step_range_start,
>>>>>>> we stop
>>>>>>> stepping backwards.
>>>>>>>
>>>>>>> The right thing to do is to look for adjacent PC ranges for the
>>>>>>> same line,
>>>>>>> until we notice a line change. Then we take that as the start PC
>>>>>>> of the
>>>>>>> range.
>>>>>>>
>>>>>>> Another solution I thought about is to merge the contiguous
>>>>>>> ranges when
>>>>>>> we are reading the line tables. Though I'm not sure if we really
>>>>>>> want to process
>>>>>>> that data as opposed to keeping it as the compiler created, and
>>>>>>> then working
>>>>>>> around that.
>>>>>>>
>>>>>>> In any case, the following patch addresses this problem.
>>>>>>>
>>>>>>> I'm not particularly happy with how we go back in the ranges
>>>>>>> (using "pc - 1").
>>>>>>> Feedback would be welcome.
>>>>>>>
>>>>>>> Validated on aarch64-linux/Ubuntu 20.04/18.04.
>>>>>>>
>>>>>>> Ubuntu 18.04 doesn't actually run into these failures because the
>>>>>>> compiler
>>>>>>> doesn't generate distinct PC ranges for the same line.
>>>>>>>
>>>>>>> gdb/ChangeLog:
>>>>>>>
>>>>>>> YYYY-MM-DD Luis Machado <luis.machado@linaro.org>
>>>>>>>
>>>>>>> * infrun.c (process_event_stop_test): Handle backward stepping
>>>>>>> across multiple ranges for the same line.
>>>>>>> * symtab.c (find_line_range_start): New function.
>>>>>>> * symtab.h (find_line_range_start): New prototype.
>>>>>>> ---
>>>>>>> gdb/infrun.c | 22 +++++++++++++++++++++-
>>>>>>> gdb/symtab.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>>> gdb/symtab.h | 16 ++++++++++++++++
>>>>>>> 3 files changed, 72 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>>>>>>> index e070eff33d..5bb268529d 100644
>>>>>>> --- a/gdb/infrun.c
>>>>>>> +++ b/gdb/infrun.c
>>>>>>> @@ -6534,11 +6534,31 @@ process_event_stop_test (struct
>>>>>>> execution_control_state *ecs)
>>>>>>> have software watchpoints). */
>>>>>>> ecs->event_thread->control.may_range_step = 1;
>>>>>>> + /* When we are stepping inside a particular line range, in
>>>>>>> reverse,
>>>>>>> + and we are sitting at the first address of that range, we
>>>>>>> need to
>>>>>>> + check if this address also shows up in another line range
>>>>>>> as the
>>>>>>> + end address.
>>>>>>> +
>>>>>>> + If so, we need to check what line such a step range points to.
>>>>>>> + If it points to the same line as the current step range, that
>>>>>>> + means we need to keep going in order to reach the first
>>>>>>> address
>>>>>>> + of the line range. We repeat this until we eventually get
>>>>>>> to the
>>>>>>> + first address of a particular line we're stepping through. */
>>>>>>> + CORE_ADDR range_start =
>>>>>>> ecs->event_thread->control.step_range_start;
>>>>>>> + if (execution_direction == EXEC_REVERSE)
>>>>>>> + {
>>>>>>> + gdb::optional<CORE_ADDR> real_range_start
>>>>>>> + = find_line_range_start
>>>>>>> (ecs->event_thread->suspend.stop_pc);
>>>>>>> +
>>>>>>> + if (real_range_start.has_value ())
>>>>>>> + range_start = *real_range_start;
>>>>>>> + }
>>>>>>> +
>>>>>>> /* When stepping backward, stop at beginning of line range
>>>>>>> (unless it's the function entry point, in which case
>>>>>>> keep going back to the call point). */
>>>>>>> CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
>>>>>>> - if (stop_pc == ecs->event_thread->control.step_range_start
>>>>>>> + if (stop_pc == range_start
>>>>>>> && stop_pc != ecs->stop_func_start
>>>>>>> && execution_direction == EXEC_REVERSE)
>>>>>>> end_stepping_range (ecs);
>>>>>>> diff --git a/gdb/symtab.c b/gdb/symtab.c
>>>>>>> index 7ffb52a943..e8f5301951 100644
>>>>>>> --- a/gdb/symtab.c
>>>>>>> +++ b/gdb/symtab.c
>>>>>>> @@ -3382,6 +3382,41 @@ find_pc_line (CORE_ADDR pc, int notcurrent)
>>>>>>> return sal;
>>>>>>> }
>>>>>>> +/* See symtah.h. */
>>>>>>> +
>>>>>>> +gdb::optional<CORE_ADDR>
>>>>>>> +find_line_range_start (CORE_ADDR pc)
>>>>>>> +{
>>>>>>> + struct symtab_and_line current_sal = find_pc_line (pc, 0);
>>>>>>> +
>>>>>>> + if (current_sal.line == 0)
>>>>>>> + return {};
>>>>>>> +
>>>>>>> + struct symtab_and_line prev_sal = find_pc_line (current_sal.pc
>>>>>>> - 1, 0);
>>>>>>> +
>>>>>>> + /* If the previous entry is for a different line, that means
>>>>>>> we are already
>>>>>>> + at the entry with the start PC for this line. */
>>>>>>> + if (prev_sal.line != current_sal.line)
>>>>>>> + return current_sal.pc;
>>>>>>> +
>>>>>>> + /* Otherwise, keep looking for entries for the same line but with
>>>>>>> + smaller PC's. */
>>>>>>> + bool done = false;
>>>>>>> + CORE_ADDR prev_pc;
>>>>>>> + while (!done)
>>>>>>> + {
>>>>>>> + prev_pc = prev_sal.pc;
>>>>>>> +
>>>>>>> + prev_sal = find_pc_line (prev_pc - 1, 0);
>>>>>>> +
>>>>>>> + /* Did we notice a line change? If so, we are done with
>>>>>>> the search. */
>>>>>>> + if (prev_sal.line != current_sal.line)
>>>>>>> + done = true;
>>>>>>> + }
>>>>>>> +
>>>>>>> + return prev_pc;
>>>>>>> +}
>>>>>>> +
>>>>>>> /* See symtab.h. */
>>>>>>> struct symtab *
>>>>>>> diff --git a/gdb/symtab.h b/gdb/symtab.h
>>>>>>> index f060e0ebc1..659cb46292 100644
>>>>>>> --- a/gdb/symtab.h
>>>>>>> +++ b/gdb/symtab.h
>>>>>>> @@ -1916,6 +1916,22 @@ extern struct symtab_and_line find_pc_line
>>>>>>> (CORE_ADDR, int);
>>>>>>> extern struct symtab_and_line find_pc_sect_line (CORE_ADDR,
>>>>>>> struct obj_section *, int);
>>>>>>> +/* Given PC, and assuming it is part of a range of addresses
>>>>>>> that is part of a
>>>>>>> + line, go back through the linetable and find the starting PC
>>>>>>> of that
>>>>>>> + line.
>>>>>>> +
>>>>>>> + For example, suppose we have 3 PC ranges for line X:
>>>>>>> +
>>>>>>> + Line X - [0x0 - 0x8]
>>>>>>> + Line X - [0x8 - 0x10]
>>>>>>> + Line X - [0x10 - 0x18]
>>>>>>> +
>>>>>>> + If we call the function with PC == 0x14, we want to return
>>>>>>> 0x0, as that is
>>>>>>> + the starting PC of line X, and the ranges are contiguous.
>>>>>>> +*/
>>>>>>> +
>>>>>>> +extern gdb::optional<CORE_ADDR> find_line_range_start (CORE_ADDR
>>>>>>> pc);
>>>>>>> +
>>>>>>> /* Wrapper around find_pc_line to just return the symtab. */
>>>>>>> extern struct symtab *find_pc_line_symtab (CORE_ADDR);
>>>>>>>
More information about the Gdb-patches
mailing list