Re: [PATCH v3 2/2] Fix an undefined behavior in record_line

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 4/4/20 6:21 AM, Bernd Edlinger wrote:
> 
> 
> On 4/4/20 12:53 AM, Luis Machado wrote:
>> Hi,
>>
>> This seems to have caused a few regressions for aarch64-linux. I'm seeing the following:
>>
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step into foo from main
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step into bar from foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step out of bar to foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step into foo_cold from foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step into baz from foo_cold
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step out of baz to foo_cold
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step out of foo_cold to foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: lo-cold: step-test-3: step out of foo to main
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step into foo from main
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step into bar from foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step out of bar to foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step into foo_cold from foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step into baz from foo_cold
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step out of baz to foo_cold
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step out of foo_cold to foo
>> FAIL: gdb.dwarf2/dw2-ranges-func.exp: hi-cold: step-test-3: step out of foo to main
>>
>> git bisect pointed at this commit:
>>

Louis,

So, I cannot do much, to debug aarch64 here.
I would however like to know what the output of the test result is,
when it fails, that is where the step does stop when it is not where it should.

And how the line table looks in the test case when it is compiled on aarch64,
btw. which gcc version do you use?

Thanks
Bernd.

> 
> Oh, dear.
> 
> Andrew, please watch out,
> 
> your other patch is also about to
> change something in this area.
> 
> I tested on x86_64 where everything looked good,
> (at least for me, but sime test cases are always faling
> or are unstable ...)
> 
> It could be that your patch
> 
> PATCH 2/2] gdb: Preserve is-stmt lines when switch between files
> 
> I just saw in my inbox is also trying to address the same issue.
> 
> I was not aware that you were working on the same issue.
> 
> 
> Thanks
> Bernd.
> 
>> ---
>>
>> commit 64dc2d4bd24ff7119c913fff91184414f09b8042
>> Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
>> Date:   Thu Mar 12 11:52:34 2020 +0100
>>
>>     Fix an undefined behavior in record_line
>>
>>     Additionally do not completely remove symbols
>>     at the same PC than the end marker, instead
>>     make them non-is-stmt breakpoints.
>>
>>     2020-04-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>>             * buildsym.c (record_line): Fix undefined behavior and preserve lines at eof.
>>
>> ---
>>
>> What i see in the log is stepping through lines not working as expected.
>>
>>
>> On 3/27/20 12:50 AM, Bernd Edlinger wrote:
>>> Additionally do not completely remove symbols
>>> at the same PC than the end marker, instead
>>> make them non-is-stmt breakpoints.
>>>
>>> 2020-03-27  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>     * buildsym.c (record_line): Fix undefined behavior and preserve
>>>     lines at eof.
>>> ---
>>>   gdb/buildsym.c | 34 ++++++++++++++++++----------------
>>>   1 file changed, 18 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/gdb/buildsym.c b/gdb/buildsym.c
>>> index 2d1e441..46c5bb1 100644
>>> --- a/gdb/buildsym.c
>>> +++ b/gdb/buildsym.c
>>> @@ -705,27 +705,29 @@ struct blockvector *
>>>                 * sizeof (struct linetable_entry))));
>>>       }
>>>   -  /* Normally, we treat lines as unsorted.  But the end of sequence
>>> -     marker is special.  We sort line markers at the same PC by line
>>> -     number, so end of sequence markers (which have line == 0) appear
>>> -     first.  This is right if the marker ends the previous function,
>>> -     and there is no padding before the next function.  But it is
>>> -     wrong if the previous line was empty and we are now marking a
>>> -     switch to a different subfile.  We must leave the end of sequence
>>> -     marker at the end of this group of lines, not sort the empty line
>>> -     to after the marker.  The easiest way to accomplish this is to
>>> -     delete any empty lines from our table, if they are followed by
>>> -     end of sequence markers.  All we lose is the ability to set
>>> -     breakpoints at some lines which contain no instructions
>>> -     anyway.  */
>>> +  /* The end of sequence marker is special.  We need to reset the
>>> +     is_stmt flag on previous lines at the same PC, otherwise these
>>> +     lines may cause problems since they might be at the same address
>>> +     as the following function.  For instance suppose a function calls
>>> +     abort there is no reason to emit a ret after that point (no joke).
>>> +     So the label may be at the same address where the following
>>> +     function begins.  A similar problem appears if a label is at the
>>> +     same address where an inline function ends we cannot reliably tell
>>> +     if this is considered part of the inline function or the calling
>>> +     program or even the next inline function, so stack traces may
>>> +     give surprising results.  Expect gdb.cp/step-and-next-inline.exp
>>> +     to fail if these lines are not modified here.  */
>>>     if (line == 0 && subfile->line_vector->nitems > 0)
>>>       {
>>> -      e = subfile->line_vector->item + subfile->line_vector->nitems - 1;
>>> -      while (subfile->line_vector->nitems > 0 && e->pc == pc)
>>> +      e = subfile->line_vector->item + subfile->line_vector->nitems;
>>> +      do
>>>       {
>>>         e--;
>>> -      subfile->line_vector->nitems--;
>>> +      if (e->pc != pc || e->line == 0)
>>> +        break;
>>> +      e->is_stmt = 0;
>>>       }
>>> +      while (e > subfile->line_vector->item);
>>>       }
>>>       e = subfile->line_vector->item + subfile->line_vector->nitems++;
>>>