Re: [RFC][PATCH 0/6] Step program considering the source column information

[Tom de Vries <tdevries@suse.de>]

On 27-05-2020 18:04, Hannes Domani via Gdb-patches wrote:
>  Am Mittwoch, 27. Mai 2020, 17:33:37 MESZ hat Tom de Vries <tdevries@suse.de> Folgendes geschrieben:
> 
>> On 16-05-2020 19:26, Hannes Domani via Gdb-patches wrote:
>>> Basically, this implements the new commands nextc (nc) and stepc (sc),
>>> which allow to step through the program until an instruction is reached,
>>> that belongs to another source column (according to the debug information).
>>>
>>> The current column location is visualized in the frame info, and in the TUI.
>>>
>>> Also, the column information is added to 'maint info line-table' and the
>>> python line table interface.
>>>
>>> Since the frame info output is different, this certainly breaks the
>>> testsuite, so this column indicator needs a parameter to disable, but I
>>> wasn't concerned about this yet.
>>>
>>> With the example code from PR25913, it looks like this:
>>>
>>> (gdb) start
>>> Temporary breakpoint 2 at 0x40162d: file gdb-25911.c, line 4.
>>> Starting program: C:\src\tests\gdb-25911.exe
>>>
>>> Temporary breakpoint 2, main () at gdb-25911.c:4
>>> 4        int a = 4;
>>>               ^
>>> (gdb) nc
>>> 6        a = 5; a = 6; a = 7;
>>>             ^
>>> (gdb) nc
>>> 6        a = 5; a = 6; a = 7;
>>>                     ^
>>> (gdb) nc
>>> 6        a = 5; a = 6; a = 7;
>>>                           ^
>>> (gdb) nc
>>> 8        return 0;
>>>
>>>
>>> What do you think of this so far?
>>>
>>
>> Very nice :)
>>
>> As I've just learned by looking at this (
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95360 ) PR, interesting
>> code in relation to this patch series is dwarf_record_line_p.
>>
>> The function implements filtering of line-table elements, and has as
>> related test-case gdb.dwarf2/dw2-single-line-discriminators.exp.
>>
>> If we disable the filtering (by returning 1 at the end instead of 0), we
>> get for test-case gdb.dwarf2/dw2-single-line-discriminators.exp:
>> ...
>> Temporary breakpoint 2, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>> ...
>>
>> That is: no progress is visible after the stepi.  If we re-enable the
>> filtering, we have:
>> ...
>> Temporary breakpoint 1, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>> (gdb) si
>> 0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>> ...
>>
>> So, progress is visible.
>>
>> If we now use your patch series, and with the filtering enabled, we have:
>> ...
>> Temporary breakpoint 1, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>>            ^
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                ^
>> (gdb) si
>> 0x00000000004004cd      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb)
>> 0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb)
>> 0x00000000004004d3      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb)
>> 0x00000000004004d5      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> ...
>>
>> So, we see progress in the insn addresses, but the column does not progress.
>>
>> OTOH, if we disable filtering:
>> ...
>> Temporary breakpoint 1, main () at
>> gdb.dwarf2/dw2-single-line-discriminators.c:26
>> 26        x = 0;
>>            ^
>> (gdb) n
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                ^
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                ^
>> (gdb) si
>> 0x00000000004004d1      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                        ^
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                                    ^
>> (gdb) si
>> 28        for (i = 0; i < 10; ++i) continue; /* stepi line */
>>                                ^
>> (gdb) si
>> 0x00000000004004d8      28        for (i = 0; i < 10; ++i) continue; /*
>> stepi line */
>>                                                        ^
>> ...
>> we don't see progress after the first stepi, but eventually we do see
>> progress in the column.
>>
>> Looking at the line-info:
>> ...
>>    [0x00000147]  Set column to 8
>>    [0x00000149]  Special opcode 161: advance Address by 11 to 0x4004c6
>> and Line by 2 to 28
>>    [0x0000014a]  Extended opcode 4: set Discriminator to 4
>>    [0x0000014e]  Special opcode 103: advance Address by 7 to 0x4004cd and
>> Line by 0 to 28
>> ...
>> perhaps we we should collapse entries that have the same column, so
>> something like this in dwarf_record_line_p:
>> ...
>>    if (line != last_line)
>>      return 1;
>>    if (colum != last_column)
>>      return 1;
>>
>>    return 0;
>>
>> ...
> 

Sorry for the late reply, I didn't notice this earlier (not on To/CC
list for some reason).

> Interesting.
> I'm just wondering, if dwarf_record_line_p merges all entries of the same line,
> why does my column-based stepping even work at all?
> 
> Is it because of line_has_non_zero_discriminator?

Yes. ( See also https://sourceware.org/bugzilla/show_bug.cgi?id=17276 ).

> If yes, what is the discriminator?

DWARF 4 standard, 6.2.2 State Machine Registers :
...
An unsigned integer identifying the block to which the current
instruction belongs. Discriminator values are assigned arbitrarily by
the DWARF producer and serve to distinguish among multiple blocks that
may all be associated with the same source file, line, and column. Where
only one lock exists for a given source position, the discriminator
value should be zero.
...

Thanks,
- Tom