[PATCH 2/2] gdb: Add support for tracking the DWARF line table is-stmt field

[Bernd Edlinger]

On 2/14/20 9:05 PM, Bernd Edlinger wrote:
> On 2/11/20 2:57 PM, Andrew Burgess wrote:
>> * Bernd Edlinger <bernd.edlinger@hotmail.de> [2020-02-05 17:55:37 +0000]:
>>
>>> On 2/5/20 12:37 PM, Andrew Burgess wrote:
>>>
>>> did you mean, when we don't land in the middle of a line ?
>>
>> No, in this case I did write the correct thing.
>>
>> So GDB had/has some code designed to "improve" the handling of looping
>> constructs.  I think the idea is to handle cases like this:
>>
>>   0x100      LN=5
>>   0x104 <-\
>>   0x108   |
>>   0x10c   |  LN=6
>>   0x110   |
>>   0x114 --/
>>
>> So when line 6 branches back to the middle of line 5, we don't stop
>> (because we are in the middle of line 5), but we do want to stop at
>> the start of line 6, so we "reset" the starting point back to line 5.
>>
>> This is done by calling set_step_info in process_event_stop_test, in
>> infrun.c.
>>
>> What we actually did though was whenever we were at an address that
>> didn't exactly match a SAL (in the example above, marked LN=5, LN=6)
>> we called set_step_info.  This worked fine, at address 0x104 we reset
>> back to LN=5, same at address 0x108.
>>
>> However, in the new world things can look different, for example, like
>> this:
>>
>>   0x100      LN=5,STMT=1
>>   0x104
>>   0x108      LN=6,STMT=0
>>   0x10c      LN=6,STMT=1
>>   0x110
>>   0x114
>>
>> In this world, when we move forward to 0x100 we don't have a matching
>> SAL, so we get the SAL for address 0x100.  We can then safely call
>> set_step_info like we did before, that's fine.  But when we step to
>> 0x108 we do now have a matching SAL, but we don't want to stop yet as
>> this address is 'STMT=0'.  However, if we call set_step_info then GDB
>> is going to think we are stepping away from LN=6, and so will refuse
>> to stop at address 0x10c.
>>
>> The proposal in this commit is that if we land at an address which
>> doesn't specifically match a SAL, 0x104, 0x110, 0x114 in the above
>> example, then we do call set_step_info, but otherwise we don't.
>>
>> There are going to be situations where the behaviour of GDB changes
>> after this patch, but I don't think we can avoid that.  What we had
>> previously was just a heuristic.  I think enabling this new
>> information in GDB will allow us to do better overall, so I think we
>> should make this change and fix any issues as they show up.
>>
> 
> I agree with that, thanks for this explanation.
> 
> However, I think I found a small problem in this patch.
> You can see it with the next-inline test case.
> When you use just step the execution does not stop
> between the inline functions, because not calling 
> 
> current behaviour is this:
> 
> Breakpoint 1, main () at next-inline.cc:63
> 63	  get_alias_set (&xx);
> (gdb) s
> get_alias_set (t=0x404040 <xx>) at next-inline.cc:50
> 50	  if (t != NULL
> (gdb) s
> 51	      && TREE_TYPE (t).z != 1
> (gdb) s
> 0x0000000000401169 in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> 34	  if (t->x != i)
> (gdb) s
> 0x000000000040117d in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> 34	  if (t->x != i)
> (gdb) s
> 0x000000000040118f in tree_check (i=0, t=0x404040 <xx>) at next-inline.h:34
> 34	  if (t->x != i)
> (gdb) s
> main () at next-inline.cc:64
> 64	  return 0;
> (gdb) 
> 
> I debugged a bit because I was not sure why that happens, and it looks
> like set_step_info does also set the current inline frame, but you
> only want to suppress the line number not the currently executing
> inline function.
> With this small patch the step works as expected.
> 

Hmm, I think this got stuck, so I just worked a follow-up patch out for you.

I also noticed that the test case cannot work with gcc before gcc-8,
since the is_stmt feature is not implemented earlier, at least with
gcc-4.8 and gcc-5.4 the test case fails.

I thought it would be good to detect that by adding the -gstatement-frontiers
option, so that the gcc driver complains when it is not able to generate
sufficient debug info for that test.

Thoughts?


Bernd.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-next-inline-test-case-with-step.patch
Type: text/x-patch
Size: 4079 bytes
Desc: 0001-Fix-next-inline-test-case-with-step.patch
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20200305/6022da5a/attachment.bin>