gdb: improve line number lookup around inline functions

  I wrote this patch while reviewing the following patches:

  https://inbox.sourceware.org/gdb-patches/AS8P193MB1285C58F6F09502252CEC16FE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
  https://inbox.sourceware.org/gdb-patches/AS8P193MB12855708DFF59A5309F5B19EE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM

These two patches fix, as far as I can tell, 3 different issues
related to debugging of optimised code around inline functions.

One of the issues fixed is having GDB display the correct line number
(and corresponding source line) when a frame other than #0 contains an
inline function, in some cases this can go wrong.

I think that there's a simple fix for this issue that doesn't require
all the infrastructure introduced in the two patches linked above, and
this is that fix.

Rather than use the tests from the above patches, which cover all 3 of
the issues that the above patches address, this commit has tests that
focus only on the one issue being fixed here.

All feedback welcome,

Thanks,
Andrew

---

This commit aims to fix an issue where GDB would report the wrong line
for frames other than #0 if a previous frame had just left an inline
function.

Consider this example which is compiled at -Og:

  volatile int global = 0;

  static inline int bar (void) { return 1; }

  static void foo (int count)
  { global += count; }

  int main (void)
  {
    foo (bar ());
    return 0;
  }

Used in this GDB session:

  (gdb) break foo
  Breakpoint 1 at 0x401106: file test.c, line 6.
  (gdb) run
  Starting program: /tmp/inline-bt/test.x

  Breakpoint 1, foo (count=count@entry=1) at test.c:6
  6	{ global += count; }
  (gdb) frame 1
  #1  0x0000000000401121 in main () at test.c:3
  3	static inline int bar (void) { return 1; }

Notice that GDB incorrectly reports frame #1 as being at line 3 when
it should really be reporting this line:

  foo (bar ());

The cause of this problem is in find_pc_sect_line (symtab.c).  This
function is passed a PC for which GDB must find the symtab_and_line
information.  The function can be called in two modes based on the
NOTCURRENT argument.

When NOTCURRENT is false then we are looking for information about the
current PC, i.e. the PC at which the inferior is currently stopped
at.

When NOTCURRENT is true we are looking for information about a PC that
it not the current PC, but is instead the PC for a previous frame.
The interesting thing in this case is that the PC passed in will be
the address after the address we actually want to lookup information
for, this is because as we unwind the program counter from frame #0
what we get is the return address in frame #1.  The return address is
often (or sometimes) on the line after the calling line, and so in
find_pc_sect_line, when NOTCURRENT is true, we subtract 1 from PC and
then proceed as normal looking for information about this new PC
value.

Now lets look at the x86-64 disassembly for 'main' from the above
example.  The location marker (=>) represents the return address in
'main' after calling 'foo':

  (gdb) run
  Starting program: /tmp/inline-bt/test.x

  Breakpoint 1, foo (count=count@entry=1) at test.c:6
  6	{ global += count; }
  (gdb) up
  #1  0x0000000000401121 in main () at test.c:3
  3	static inline int bar (void) { return 1; }
  (gdb) disassemble
  Dump of assembler code for function main:
     0x0000000000401117 <+0>:	mov    $0x1,%edi
     0x000000000040111c <+5>:	call   0x401106 <foo>
  => 0x0000000000401121 <+10>:	mov    $0x0,%eax
     0x0000000000401126 <+15>:	ret
  End of assembler dump.

And the corresponding line table:

  (gdb) maintenance info line-table
  objfile: /tmp/inline-bt/test.x ((struct objfile *) 0x49ff190)
  compunit_symtab: test.c ((struct compunit_symtab *) 0x49f63f0)
  symtab: /tmp/inline-bt/test.c ((struct symtab *) 0x49f6470)
  linetable: ((struct linetable *) 0x49f6d40):
  INDEX  LINE   REL-ADDRESS        UNREL-ADDRESS      IS-STMT PROLOGUE-END
  0      6      0x0000000000401106 0x0000000000401106 Y
  1      6      0x0000000000401106 0x0000000000401106
  2      6      0x0000000000401108 0x0000000000401108 Y
  3      6      0x0000000000401108 0x0000000000401108
  4      6      0x000000000040110e 0x000000000040110e
  5      6      0x0000000000401116 0x0000000000401116
  6      9      0x0000000000401117 0x0000000000401117 Y
  7      10     0x0000000000401117 0x0000000000401117 Y
  8      3      0x0000000000401117 0x0000000000401117 Y
  9      10     0x0000000000401117 0x0000000000401117
  10     11     0x0000000000401121 0x0000000000401121 Y
  11     12     0x0000000000401121 0x0000000000401121
  12     END    0x0000000000401127 0x0000000000401127 Y

When looking for the line information of frame #1 we start with the
return address 0x401121, however, as this is not the current program
counter value we subtract one and look for line information for
0x401120.

We will find entry at index 9, this is the last entry with an address
less than the address we're looking for where the next entry has an
address greater than the one we're looking for.  The entry at index 9
is for line 10 which is the correct line, but GDB reports line 3, so
what's going on?

Having found a matching entry GDB checks to see if the entry is marked
as is-stmt (is statement).  In our case index 9 (line 10) is not a
statement, and so GDB looks backwards for entries at the same address,
if any of these are marked is-stmt then GDB will use the last of these
instead.  In our case the previous entry at index 8 is marked is-stmt,
and so GDB uses that.  The entry at index 8 is for line 3, and that is
why GDB reports the wrong line.  So why perform the backward is-stmt
check?

When NOTCURRENT is false (not our case) the backward scan makes
sense.  If the inferior has just stopped at some new location, and we
want to report that location to the user, then it is better (I think)
to select an is-stmt entry.  In this way we will report a line number
for a line which the inferior is just about to start executing, and
non of the side effects of that line have yet taken place.  The line
GDB prints will correspond with the reported line, and if the user
queries the inferior state, the inferior should (assuming the compiler
emitted correct is-stmt markers) correspond to the line in question
having not yet been started.

However, in our case NOTCURRENT is true.  We're looking back to
previous frames that are currently in-progress.  If upon return to the
previous frame we are about to execute the next line then (is seems to
me) that this indicates we must be performing the very last action
from the previous line.  As such, looking back through the line table
in order to report a line that has not yet started is the wrong thing
to do.  We really want to report the very last line table entry for
the previous address as this is (I think) most likely to represent the
previous line that is just about to complete.

Further, in the NOTCURRENT case, we should care less about reporting
an is-stmt line.  When a user looks back to a previous frame I don't
think they expect the line being reported to have not yet started.  In
fact I think the expectation is the reverse ... after all, the
previous line must have executed enough to call the current frame.

So my proposal is that the backward scan of the line table looking for
an is-stmt entry should not be performed when NOTCURRENT is true.  In
the case above this means we will report the entry at index 9, which
is for line 10, which is correct.

For testing this commit I have:

 1. Extended the existing gdb.opt/inline-bt.exp test.  I've extended
 the source code to include a test similar to the example above.  I
 have also extended the script so that the test is compiled at a
 variety of optimisation levels (O0, Og, O1, O2).

 2. Added a new DWARF assembler test which hard codes a line table
 similar to the example given above.  My hope is that even if test
 case (1) changes (due to compiler changes) this test will continue to
 test the specific case I'm interested in.

I was inspired to write this patch while reviewing these patches:

  https://inbox.sourceware.org/gdb-patches/AS8P193MB1285C58F6F09502252CEC16FE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
  https://inbox.sourceware.org/gdb-patches/AS8P193MB12855708DFF59A5309F5B19EE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM

though this patch only covers one of the issues addressed by these
patches, and the approach taken is quite different.  Still, those
patches are worth reading for the history of this fix.
---
 gdb/symtab.c                                  |  25 ++-
 gdb/testsuite/gdb.dwarf2/dw2-inline-bt-lbls.c |  50 +++++
 gdb/testsuite/gdb.dwarf2/dw2-inline-bt-src.c  |  35 +++
 gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp    | 212 ++++++++++++++++++
 gdb/testsuite/gdb.opt/inline-bt.c             |  20 ++
 gdb/testsuite/gdb.opt/inline-bt.exp           | 127 +++++++----
 6 files changed, 422 insertions(+), 47 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-bt-lbls.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-bt-src.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp


base-commit: b0056f89497ce28e88b275250099d4fab0999c0a
  

Hi Andrew,

thanks for working on this.

On 7/26/24 17:32, Andrew Burgess wrote:
> I wrote this patch while reviewing the following patches:
> 
>   https://inbox.sourceware.org/gdb-patches/AS8P193MB1285C58F6F09502252CEC16FE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>   https://inbox.sourceware.org/gdb-patches/AS8P193MB12855708DFF59A5309F5B19EE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
> 
> These two patches fix, as far as I can tell, 3 different issues
> related to debugging of optimised code around inline functions.
> 
> One of the issues fixed is having GDB display the correct line number
> (and corresponding source line) when a frame other than #0 contains an
> inline function, in some cases this can go wrong.
> 
> I think that there's a simple fix for this issue that doesn't require
> all the infrastructure introduced in the two patches linked above, and
> this is that fix.
> 
> Rather than use the tests from the above patches, which cover all 3 of
> the issues that the above patches address, this commit has tests that
> focus only on the one issue being fixed here.
> 
> All feedback welcome,
> 
> Thanks,
> Andrew
> 
> ---
> 
> This commit aims to fix an issue where GDB would report the wrong line
> for frames other than #0 if a previous frame had just left an inline
> function.
> 
> Consider this example which is compiled at -Og:
> 
>   volatile int global = 0;
> 
>   static inline int bar (void) { return 1; }
> 
>   static void foo (int count)
>   { global += count; }
> 
>   int main (void)
>   {
>     foo (bar ());
>     return 0;
>   }
> 

Which gcc version were you using?

Initially I had not been able to reproduce the reported issue
with this test case.  Since I used gcc-12 + gcc-14.

Then I realized that there was something wrong with the debug
info in gcc-9 + gcc-10 but that was fixed in gcc-11 and following.

The problem is that the inline function is completely optimized
away, so there is no DW_TAG_inlined_subroutine in the debug info
but the line numbers from the subroutine are still there, and make
the problems, however the problem is not solved by your patch.
Consider this command:

$ gcc -g -Og -fcf-protection=none test.c
$ gdb a.out
(gdb) b main
Breakpoint 1 at 0x113a: file test.c, line 3.
(gdb) r
Breakpoint 1, main () at test.c:3
3	static inline int bar (void) { return 1; }
(gdb) s
foo (count=count@entry=1) at test.c:6
6	{ global += count; }
(gdb) bt
#0  foo (count=count@entry=1) at test.c:6
#1  0x0000555555555144 in main () at test.c:10


So, I have built gdb with your patch, and as you can see, the line table
is completely bogus, and can not work properly.  But why does this
no longer happen with gcc-11 and following?

Well, the reason is that I fixed this bug already some years ago:

commit 2e6562043c48c0ae6bc9823d438685269eb11aab (HEAD, refs/bisect/new)
Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date:   Mon Dec 7 12:00:00 2020 +0100

    Remove misleading debug line entries
    
    This removes gimple_debug_begin_stmts without block info which remain
    after a gimple block originating from an inline function is unused.
    
    The line numbers from these stmts are from the inline function,
    but since the inline function is completely optimized away,
    there will be no DW_TAG_inlined_subroutine so the debugger has
    no callstack available at this point, and therefore those
    line table entries are not helpful to the user.
    
    2020-12-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>
    
            * cfgexpand.c (expand_gimple_basic_block): Remove special handling
            of debug_inline_entries without block info.
            * tree-inline.c (remap_gimple_stmt): Drop debug_nonbind_markers when
            the call statement has no block info.
            (copy_debug_stmt): Remove debug_nonbind_markers when inlining
            and the block info is mapped to NULL.
            * tree-ssa-live.c (clear_unused_block_pointer): Remove
            debug_nonbind_markers originating from removed inline functions.

This is from the gcc git repository, this commit landed in gcc-11 but I
never considered it appropriate for a back-port to earlier stable branches.
So with gcc built from this revision onwards, the line table does simply
no longer contain the line 3 in above test example.

So in comparison to my patch I found this attempt to be more radical,
and still incomplete as this little experiment shows.  Admittedly my
patch focuses only on inline-functions that do have block-ranges, and therefore
would not change the result with a debug-info that is simply inconsistent
as in this example.


Regarding the test case there I noticed that it does something that gcc never
does, namely to not emit a weak line table entry at the end of the inline
function.

>   7      10     0x0000000000401117 0x0000000000401117 Y
>   8      3      0x0000000000401117 0x0000000000401117 Y
>   9      10     0x0000000000401117 0x0000000000401117
>   10     11     0x0000000000401121 0x0000000000401121 Y

so at the end of the inline function's block-range we always have the
weak line number from the calling function, that is what find_pc_sect_line
returns with my patch, but the test case does not emit any line table entries
at the end of the inline range, therefore the test case is failed with
my patch, which I think is a bug in the test itself, I can make it
passed if apply this change:

--- a/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
@@ -168,13 +168,15 @@ proc do_test { } {
                line $bar_body
                DW_LNS_copy
 
-               DW_LNE_set_address line_label_4
+               DW_LNE_set_address line_label_5
+               DW_LNS_advance_line 1
+               DW_LNS_copy
+
+               DW_LNE_set_address line_label_5
                line $call_line
                DW_LNS_negate_stmt
                DW_LNS_copy
 
-               # Skip line_label_5.
-
                DW_LNE_set_address line_label_6
                DW_LNS_advance_line 1
                DW_LNS_copy


The inline function range is from line_label_4 to line_label_5,
and at the end of the inline range, we expect to see an is-stmt
line at the end of the inline function followed by a non-is-stmt
line at the calling function.

So the test case would be okay with this change applied, but please
do not fix the find_pc_sect_line in this way, as it would have an effect
not only on inline functions, and it would of course conflict with my
patch, while the issue was resolved by my earlier gcc-patch already 4
years ago.  


Thanks
Bernd.
  

Hi all,

Sorry for taking so long to get to this.
On 7/29/24 2:01 AM, Bernd Edlinger wrote:
> Hi Andrew,
>
> thanks for working on this.
>
> On 7/26/24 17:32, Andrew Burgess wrote:
>> I wrote this patch while reviewing the following patches:
>>
>>    https://inbox.sourceware.org/gdb-patches/AS8P193MB1285C58F6F09502252CEC16FE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>>    https://inbox.sourceware.org/gdb-patches/AS8P193MB12855708DFF59A5309F5B19EE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>>
>> These two patches fix, as far as I can tell, 3 different issues
>> related to debugging of optimised code around inline functions.
>>
>> One of the issues fixed is having GDB display the correct line number
>> (and corresponding source line) when a frame other than #0 contains an
>> inline function, in some cases this can go wrong.
>>
>> I think that there's a simple fix for this issue that doesn't require
>> all the infrastructure introduced in the two patches linked above, and
>> this is that fix.
>>
>> Rather than use the tests from the above patches, which cover all 3 of
>> the issues that the above patches address, this commit has tests that
>> focus only on the one issue being fixed here.
>>
>> All feedback welcome,
>>
>> Thanks,
>> Andrew
>>
>> ---
>>
>> This commit aims to fix an issue where GDB would report the wrong line
>> for frames other than #0 if a previous frame had just left an inline
>> function.
>>
>> Consider this example which is compiled at -Og:
>>
>>    volatile int global = 0;
>>
>>    static inline int bar (void) { return 1; }
>>
>>    static void foo (int count)
>>    { global += count; }
>>
>>    int main (void)
>>    {
>>      foo (bar ());
>>      return 0;
>>    }
>>
> Which gcc version were you using?

I also can't reproduce the issue because of different debug info (using 
gcc 14.2 from fedora 40).

That said, I tried this with the original reproducer in the original bug 
(https://sourceware.org/bugzilla/show_bug.cgi?id=25987) and this patch 
fixes the backtrace issue in that problem, and the logic behind why we 
shouldn't search for an is-stmt line makes sense to me.

>
> Initially I had not been able to reproduce the reported issue
> with this test case.  Since I used gcc-12 + gcc-14.
>
> Then I realized that there was something wrong with the debug
> info in gcc-9 + gcc-10 but that was fixed in gcc-11 and following.
>
> The problem is that the inline function is completely optimized
> away, so there is no DW_TAG_inlined_subroutine in the debug info
> but the line numbers from the subroutine are still there, and make
> the problems, however the problem is not solved by your patch.
> Consider this command:
>
> $ gcc -g -Og -fcf-protection=none test.c
> $ gdb a.out
> (gdb) b main
> Breakpoint 1 at 0x113a: file test.c, line 3.
> (gdb) r
> Breakpoint 1, main () at test.c:3
> 3	static inline int bar (void) { return 1; }
> (gdb) s
> foo (count=count@entry=1) at test.c:6
> 6	{ global += count; }
> (gdb) bt
> #0  foo (count=count@entry=1) at test.c:6
> #1  0x0000555555555144 in main () at test.c:10
>
>
> So, I have built gdb with your patch, and as you can see, the line table
> is completely bogus, and can not work properly.  But why does this
> no longer happen with gcc-11 and following?

I disagree though. The line table isn't completely bogus, GDB is just 
having trouble parsing it when searching for the current location. The 
backtrace - which is the only thing that Andrew's patches change - is 
working correctly. GDB can't step into bar (i think) because of the 0 
size subprogram, which is unrelated to printing the correct line in 
either case.

>
> Well, the reason is that I fixed this bug already some years ago:
>
> commit 2e6562043c48c0ae6bc9823d438685269eb11aab (HEAD, refs/bisect/new)
> Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date:   Mon Dec 7 12:00:00 2020 +0100
>
>      Remove misleading debug line entries
>      
>      This removes gimple_debug_begin_stmts without block info which remain
>      after a gimple block originating from an inline function is unused.
>      
>      The line numbers from these stmts are from the inline function,
>      but since the inline function is completely optimized away,
>      there will be no DW_TAG_inlined_subroutine so the debugger has
>      no callstack available at this point, and therefore those
>      line table entries are not helpful to the user.
>      
>      2020-12-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>      
>              * cfgexpand.c (expand_gimple_basic_block): Remove special handling
>              of debug_inline_entries without block info.
>              * tree-inline.c (remap_gimple_stmt): Drop debug_nonbind_markers when
>              the call statement has no block info.
>              (copy_debug_stmt): Remove debug_nonbind_markers when inlining
>              and the block info is mapped to NULL.
>              * tree-ssa-live.c (clear_unused_block_pointer): Remove
>              debug_nonbind_markers originating from removed inline functions.
>
> This is from the gcc git repository, this commit landed in gcc-11 but I
> never considered it appropriate for a back-port to earlier stable branches.
> So with gcc built from this revision onwards, the line table does simply
> no longer contain the line 3 in above test example.

I guess this commit explains why I cant reproduce it anymore. However, 
since you should be able to use gcc 9 to build gdb (first to have stable 
C++17 support), I think its reasonable to expect GDB to support DWARF 
info emitted by gcc9 when possible, so we shouldn't discount the line 
table that is emitted previous to this change.

Also, with the updated gcc, there is no way that GDB will ever enter (or 
at least report to the user that the inferior entered) the bar function, 
because the line table doesn't ever mention it. If GDB correctly handles 
the fact that bar exists and is entered in the right PC, we won't have 
any line table to use to print the right line, so debugging this case 
will always be confusing based on this (gcc) patch.

>
> So in comparison to my patch I found this attempt to be more radical,
> and still incomplete as this little experiment shows.  Admittedly my
> patch focuses only on inline-functions that do have block-ranges, and therefore
> would not change the result with a debug-info that is simply inconsistent
> as in this example.

I disagree that this approach is more radical. Andrew is just changing a 
small if condition based on whether you're searching for the current PC 
or not, so that GDB doesn't report to the user that "nothing in this 
line has happened yet" even though it clearly has because the function 
was called already.

Your solution is adding a whole new heuristic that changes all line 
table handling by GDB, for multiple different commands, and that will 
likely clash with the Location View Numbering that looks like it will be 
added in DWARF6 (https://dwarfstd.org/issues/170427.1.html).

>
>
> Regarding the test case there I noticed that it does something that gcc never
> does, namely to not emit a weak line table entry at the end of the inline
> function.
>
>>    7      10     0x0000000000401117 0x0000000000401117 Y
>>    8      3      0x0000000000401117 0x0000000000401117 Y
>>    9      10     0x0000000000401117 0x0000000000401117
>>    10     11     0x0000000000401121 0x0000000000401121 Y
> so at the end of the inline function's block-range we always have the
> weak line number from the calling function, that is what find_pc_sect_line
> returns with my patch, but the test case does not emit any line table entries
> at the end of the inline range, therefore the test case is failed with
> my patch, which I think is a bug in the test itself, I can make it
> passed if apply this change:
As I said, if gcc 9 or 10 emits a line table like this, we should try to 
support it, so I think your patch should also work with this test.
>
> --- a/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
> @@ -168,13 +168,15 @@ proc do_test { } {
>                  line $bar_body
>                  DW_LNS_copy
>   
> -               DW_LNE_set_address line_label_4
> +               DW_LNE_set_address line_label_5
> +               DW_LNS_advance_line 1
> +               DW_LNS_copy
> +
> +               DW_LNE_set_address line_label_5
>                  line $call_line
>                  DW_LNS_negate_stmt
>                  DW_LNS_copy
>   
> -               # Skip line_label_5.
> -
>                  DW_LNE_set_address line_label_6
>                  DW_LNS_advance_line 1
>                  DW_LNS_copy
>
>
> The inline function range is from line_label_4 to line_label_5,
> and at the end of the inline range, we expect to see an is-stmt
> line at the end of the inline function followed by a non-is-stmt
> line at the calling function.
>
> So the test case would be okay with this change applied, but please
> do not fix the find_pc_sect_line in this way, as it would have an effect
> not only on inline functions, and it would of course conflict with my
> patch, while the issue was resolved by my earlier gcc-patch already 4
> years ago.
I hope I made a reasonable case for why changing more than inline 
functions is reasonable.
  

Hi,

thanks for your reply.

On 8/20/24 21:13, Guinevere Larsen wrote:
> Hi all,
> 
> Sorry for taking so long to get to this.
> On 7/29/24 2:01 AM, Bernd Edlinger wrote:
>> Hi Andrew,
>>
>> thanks for working on this.
>>
>> On 7/26/24 17:32, Andrew Burgess wrote:
>>> I wrote this patch while reviewing the following patches:
>>>
>>>    https://inbox.sourceware.org/gdb-patches/AS8P193MB1285C58F6F09502252CEC16FE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>>>    https://inbox.sourceware.org/gdb-patches/AS8P193MB12855708DFF59A5309F5B19EE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>>>
>>> These two patches fix, as far as I can tell, 3 different issues
>>> related to debugging of optimised code around inline functions.
>>>
>>> One of the issues fixed is having GDB display the correct line number
>>> (and corresponding source line) when a frame other than #0 contains an
>>> inline function, in some cases this can go wrong.
>>>
>>> I think that there's a simple fix for this issue that doesn't require
>>> all the infrastructure introduced in the two patches linked above, and
>>> this is that fix.
>>>
>>> Rather than use the tests from the above patches, which cover all 3 of
>>> the issues that the above patches address, this commit has tests that
>>> focus only on the one issue being fixed here.
>>>
>>> All feedback welcome,
>>>
>>> Thanks,
>>> Andrew
>>>
>>> ---
>>>
>>> This commit aims to fix an issue where GDB would report the wrong line
>>> for frames other than #0 if a previous frame had just left an inline
>>> function.
>>>
>>> Consider this example which is compiled at -Og:
>>>
>>>    volatile int global = 0;
>>>
>>>    static inline int bar (void) { return 1; }
>>>
>>>    static void foo (int count)
>>>    { global += count; }
>>>
>>>    int main (void)
>>>    {
>>>      foo (bar ());
>>>      return 0;
>>>    }
>>>
>> Which gcc version were you using?
> 
> I also can't reproduce the issue because of different debug info (using gcc 14.2 from fedora 40).
> 
> That said, I tried this with the original reproducer in the original bug (https://sourceware.org/bugzilla/show_bug.cgi?id=25987) and this patch fixes the backtrace issue in that problem, and the logic behind why we shouldn't search for an is-stmt line makes sense to me.
> 

Well, maybe, but currently we known only about issues with inline functions,
especially at the end of the inline range, where searching backwards for
is-stmt lines yields completely wrong results.

It is wrong in frame 1, but also in frame 0.

But I am anxious that if we change the paradigma completely, then other
examples will come up, where an is-stmt is followed by out-of-place non-is-stmt
lines, e.g. stuff that was moved by code-motion optimization e.g. from the function
header, to some other line, which will be a non-is-stmt line of course, i.e. one that
will probably look more confusing than the is-stmt line before.

Furthermore I think the main reason why is-stmt lines are preferred over non-is-stmt lines
is, that it shall be possible to set a break-point on that line, which is of course
not possible when we hand out non-is-stmt lines here, that means, unless absolutely necessary.

>>
>> Initially I had not been able to reproduce the reported issue
>> with this test case.  Since I used gcc-12 + gcc-14.
>>
>> Then I realized that there was something wrong with the debug
>> info in gcc-9 + gcc-10 but that was fixed in gcc-11 and following.
>>
>> The problem is that the inline function is completely optimized
>> away, so there is no DW_TAG_inlined_subroutine in the debug info
>> but the line numbers from the subroutine are still there, and make
>> the problems, however the problem is not solved by your patch.
>> Consider this command:
>>
>> $ gcc -g -Og -fcf-protection=none test.c
>> $ gdb a.out
>> (gdb) b main
>> Breakpoint 1 at 0x113a: file test.c, line 3.
>> (gdb) r
>> Breakpoint 1, main () at test.c:3
>> 3    static inline int bar (void) { return 1; }
>> (gdb) s
>> foo (count=count@entry=1) at test.c:6
>> 6    { global += count; }
>> (gdb) bt
>> #0  foo (count=count@entry=1) at test.c:6
>> #1  0x0000555555555144 in main () at test.c:10
>>
>>
>> So, I have built gdb with your patch, and as you can see, the line table
>> is completely bogus, and can not work properly.  But why does this
>> no longer happen with gcc-11 and following?
> 
> I disagree though. The line table isn't completely bogus, GDB is just having trouble parsing it when searching for the current location. The backtrace - which is the only thing that Andrew's patches change - is working correctly. GDB can't step into bar (i think) because of the 0 size subprogram, which is unrelated to printing the correct line in either case.
> 

This example is more or less an exceptional one, not the normal case
that happens most of the time.

That means in general these old gcc versions do generate correct debug info.

What happens here, is that the function bar is completely optimized
away, as if it were a macro.  That means there is no
DW_TAG_inlined_subroutine debug info for bar at all, just one is-stmt line
entry, nothing else.  My gcc patch did simply remove that line table entry,
because this function does no longer exist after optimization.
It is a ghost, simply de-materialized.

It is something slightly different to an empty inline, as in the PR 25987
that you mentioned, here the debug info generated with gcc-8, 9 and 10
looks very different.  BTW: this is the empty-inline.cc test case
in my path, I've taken it from there.

The inline function has an DW_TAG_inlined_subroutine
 <2><11c>: Abbrev Number: 17 (DW_TAG_inlined_subroutine)
    <11d>   DW_AT_abstract_origin: <0x18a>
    <121>   DW_AT_entry_pc    : 0x40112b
    <129>   DW_AT_GNU_entry_view: 2
    <12a>   DW_AT_low_pc      : 0x40112b
    <132>   DW_AT_high_pc     : 0
    <13a>   DW_AT_call_file   : 1
    <13b>   DW_AT_call_line   : 11
    <13c>   DW_AT_call_column : 18
    <13d>   DW_AT_sibling     : <0x14f>

And the line-table around that PC looks like this:

main.cpp                                   9            0x40112a        
main.cpp                                  10            0x40112b               x
main.cpp                                  11            0x40112b       1       x
main.cpp                                   3            0x40112b       2       x
main.cpp                                   3            0x40112b       3       x
main.cpp                                   3            0x40112b       4
main.cpp                                  11            0x40112b       5
main.cpp                                  12            0x401130        


but when I compare that to the test case, then I see something completely different:

 <2><71>: Abbrev Number: 5 (DW_TAG_inlined_subroutine)
    <72>   DW_AT_abstract_origin: <0x53>
    <76>   DW_AT_low_pc      : 0x1152
    <7e>   DW_AT_high_pc     : 0x115c
    <86>   DW_AT_call_file   : 1
    <87>   DW_AT_call_line   : 33

This is no empty subroutine as in the example above, and the line table has
no entries at the high_pc address 0x115c at all:

dw2-inline-bt-src.c                       32              0x1144               x
dw2-inline-bt-src.c                       33              0x1152               x
dw2-inline-bt-src.c                       27              0x1152               x
dw2-inline-bt-src.c                       33              0x1152        
dw2-inline-bt-src.c                       34              0x116b        


This is not like anything that any gcc version ever did, thus it is unrealistic.
the line table at the inline entry does always look like this

dw2-inline-bt-src.c                       33              0x1152               x
dw2-inline-bt-src.c                       27              0x1152               x

and the line-table at the exit point does always look like this:

dw2-inline-bt-src.c                       28              0x115c               x
dw2-inline-bt-src.c                       33              0x115c        

that means a correct line table that matches the inlined_subroutine should look
like this:

dw2-inline-bt-src.c                       32              0x1144               x
dw2-inline-bt-src.c                       33              0x1152               x
dw2-inline-bt-src.c                       27              0x1152               x
dw2-inline-bt-src.c                       28              0x115c               x
dw2-inline-bt-src.c                       33              0x115c        
dw2-inline-bt-src.c                       34              0x116b        

So I would be okay when the test case had this kind of line table,
then of course my patch would also not have a glitch when this test case
is executed.

>>
>> Well, the reason is that I fixed this bug already some years ago:
>>
>> commit 2e6562043c48c0ae6bc9823d438685269eb11aab (HEAD, refs/bisect/new)
>> Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
>> Date:   Mon Dec 7 12:00:00 2020 +0100
>>
>>      Remove misleading debug line entries
>>           This removes gimple_debug_begin_stmts without block info which remain
>>      after a gimple block originating from an inline function is unused.
>>           The line numbers from these stmts are from the inline function,
>>      but since the inline function is completely optimized away,
>>      there will be no DW_TAG_inlined_subroutine so the debugger has
>>      no callstack available at this point, and therefore those
>>      line table entries are not helpful to the user.
>>           2020-12-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>                   * cfgexpand.c (expand_gimple_basic_block): Remove special handling
>>              of debug_inline_entries without block info.
>>              * tree-inline.c (remap_gimple_stmt): Drop debug_nonbind_markers when
>>              the call statement has no block info.
>>              (copy_debug_stmt): Remove debug_nonbind_markers when inlining
>>              and the block info is mapped to NULL.
>>              * tree-ssa-live.c (clear_unused_block_pointer): Remove
>>              debug_nonbind_markers originating from removed inline functions.
>>
>> This is from the gcc git repository, this commit landed in gcc-11 but I
>> never considered it appropriate for a back-port to earlier stable branches.
>> So with gcc built from this revision onwards, the line table does simply
>> no longer contain the line 3 in above test example.
> 
> I guess this commit explains why I cant reproduce it anymore. However, since you should be able to use gcc 9 to build gdb (first to have stable C++17 support), I think its reasonable to expect GDB to support DWARF info emitted by gcc9 when possible, so we shouldn't discount the line table that is emitted previous to this change.
> 
> Also, with the updated gcc, there is no way that GDB will ever enter (or at least report to the user that the inferior entered) the bar function, because the line table doesn't ever mention it. If GDB correctly handles the fact that bar exists and is entered in the right PC, we won't have any line table to use to print the right line, so debugging this case will always be confusing based on this (gcc) patch.
> 

As I said this function does no longer exist, it is converted to a macro.
The optimizer simply knows that bar() is a synonym for 1.
So my patch is not to blame here.
The line-entry in the debug-info was just an artefact without any context.

>>
>> So in comparison to my patch I found this attempt to be more radical,
>> and still incomplete as this little experiment shows.  Admittedly my
>> patch focuses only on inline-functions that do have block-ranges, and therefore
>> would not change the result with a debug-info that is simply inconsistent
>> as in this example.
> 
> I disagree that this approach is more radical. Andrew is just changing a small if condition based on whether you're searching for the current PC or not, so that GDB doesn't report to the user that "nothing in this line has happened yet" even though it clearly has because the function was called already.
> 
> Your solution is adding a whole new heuristic that changes all line table handling by GDB, for multiple different commands, and that will likely clash with the Location View Numbering that looks like it will be added in DWARF6 (https://dwarfstd.org/issues/170427.1.html).
> 

I've done some research about this, it seems to be fully implemented by gcc,
but I must say that this will not solve the problem with the inline function
range in the line table.  It focuses only on the visibility of local variables,
and since e.g. my test case empty-inline.c has no local variables at all, there
are no W_LLE_view_pair at all in the debug info here.

There is however a DW_AT_GNU_entry_view, that is not mentioned in that amendment,
but we have no issues with the line table at the entry pc.

So my impression is, that if the location view for the DW_AT_low_pc and DW_AT_high_pc
and for range begin/end will be specified in the future, then that will not
clash with the weak bit of the line table, but instead make it more precise, which
line table at the end of an inline are weak, this means they belong to the inline,
and not to the outer function, also possibly also that the entry of an inline can have
weak table entries, which would be then the ones that do not belong to the inline
block, but are from the calling function.

>>
>>
>> Regarding the test case there I noticed that it does something that gcc never
>> does, namely to not emit a weak line table entry at the end of the inline
>> function.
>>
>>>    7      10     0x0000000000401117 0x0000000000401117 Y
>>>    8      3      0x0000000000401117 0x0000000000401117 Y
>>>    9      10     0x0000000000401117 0x0000000000401117
>>>    10     11     0x0000000000401121 0x0000000000401121 Y
>> so at the end of the inline function's block-range we always have the
>> weak line number from the calling function, that is what find_pc_sect_line
>> returns with my patch, but the test case does not emit any line table entries
>> at the end of the inline range, therefore the test case is failed with
>> my patch, which I think is a bug in the test itself, I can make it
>> passed if apply this change:
> As I said, if gcc 9 or 10 emits a line table like this, we should try to support it, so I think your patch should also work with this test.

Be careful what you wish, this is just a bug in the compiler, and of
course not one that happens very often.

If we do something about it, then maybe after checking the gcc version,
that produced the debug info, but to me it looks like overkill.

As I said, the patch would work, if the test would not produce
broken line tables that do not match to the rest of the debug info.
And really no gcc version would have produced a debug info like that.

>>
>> --- a/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
>> @@ -168,13 +168,15 @@ proc do_test { } {
>>                  line $bar_body
>>                  DW_LNS_copy
>>   -               DW_LNE_set_address line_label_4
>> +               DW_LNE_set_address line_label_5
>> +               DW_LNS_advance_line 1
>> +               DW_LNS_copy
>> +
>> +               DW_LNE_set_address line_label_5
>>                  line $call_line
>>                  DW_LNS_negate_stmt
>>                  DW_LNS_copy
>>   -               # Skip line_label_5.
>> -
>>                  DW_LNE_set_address line_label_6
>>                  DW_LNS_advance_line 1
>>                  DW_LNS_copy
>>
>>
>> The inline function range is from line_label_4 to line_label_5,
>> and at the end of the inline range, we expect to see an is-stmt
>> line at the end of the inline function followed by a non-is-stmt
>> line at the calling function.
>>
>> So the test case would be okay with this change applied, but please
>> do not fix the find_pc_sect_line in this way, as it would have an effect
>> not only on inline functions, and it would of course conflict with my
>> patch, while the issue was resolved by my earlier gcc-patch already 4
>> years ago.
> I hope I made a reasonable case for why changing more than inline functions is reasonable.
> 

Sorry, when I do not agree here.
As I said, I think that showing is-stmt lines in the backtrace has the
advantage, that it should be possible to set break-points there.
And who knows what other non-is-stmt lines can exist without any inline
function, that can easily be very confusing.


Thanks,
Bernd.
  

Bernd,

I just wanted to let you know that reviewing your inline function
patches is high on my todo list.  I'm certainly hoping that I can get
back around to them in the next month.

When I do review them I'll take on board what you've written in this
email too.

Thanks for your patience, and sorry I've not been able to get to them
sooner.

Thanks,
Andrew





Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> Hi,
>
> thanks for your reply.
>
> On 8/20/24 21:13, Guinevere Larsen wrote:
>> Hi all,
>> 
>> Sorry for taking so long to get to this.
>> On 7/29/24 2:01 AM, Bernd Edlinger wrote:
>>> Hi Andrew,
>>>
>>> thanks for working on this.
>>>
>>> On 7/26/24 17:32, Andrew Burgess wrote:
>>>> I wrote this patch while reviewing the following patches:
>>>>
>>>>    https://inbox.sourceware.org/gdb-patches/AS8P193MB1285C58F6F09502252CEC16FE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>>>>    https://inbox.sourceware.org/gdb-patches/AS8P193MB12855708DFF59A5309F5B19EE4DF2@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
>>>>
>>>> These two patches fix, as far as I can tell, 3 different issues
>>>> related to debugging of optimised code around inline functions.
>>>>
>>>> One of the issues fixed is having GDB display the correct line number
>>>> (and corresponding source line) when a frame other than #0 contains an
>>>> inline function, in some cases this can go wrong.
>>>>
>>>> I think that there's a simple fix for this issue that doesn't require
>>>> all the infrastructure introduced in the two patches linked above, and
>>>> this is that fix.
>>>>
>>>> Rather than use the tests from the above patches, which cover all 3 of
>>>> the issues that the above patches address, this commit has tests that
>>>> focus only on the one issue being fixed here.
>>>>
>>>> All feedback welcome,
>>>>
>>>> Thanks,
>>>> Andrew
>>>>
>>>> ---
>>>>
>>>> This commit aims to fix an issue where GDB would report the wrong line
>>>> for frames other than #0 if a previous frame had just left an inline
>>>> function.
>>>>
>>>> Consider this example which is compiled at -Og:
>>>>
>>>>    volatile int global = 0;
>>>>
>>>>    static inline int bar (void) { return 1; }
>>>>
>>>>    static void foo (int count)
>>>>    { global += count; }
>>>>
>>>>    int main (void)
>>>>    {
>>>>      foo (bar ());
>>>>      return 0;
>>>>    }
>>>>
>>> Which gcc version were you using?
>> 
>> I also can't reproduce the issue because of different debug info (using gcc 14.2 from fedora 40).
>> 
>> That said, I tried this with the original reproducer in the original bug (https://sourceware.org/bugzilla/show_bug.cgi?id=25987) and this patch fixes the backtrace issue in that problem, and the logic behind why we shouldn't search for an is-stmt line makes sense to me.
>> 
>
> Well, maybe, but currently we known only about issues with inline functions,
> especially at the end of the inline range, where searching backwards for
> is-stmt lines yields completely wrong results.
>
> It is wrong in frame 1, but also in frame 0.
>
> But I am anxious that if we change the paradigma completely, then other
> examples will come up, where an is-stmt is followed by out-of-place non-is-stmt
> lines, e.g. stuff that was moved by code-motion optimization e.g. from the function
> header, to some other line, which will be a non-is-stmt line of course, i.e. one that
> will probably look more confusing than the is-stmt line before.
>
> Furthermore I think the main reason why is-stmt lines are preferred over non-is-stmt lines
> is, that it shall be possible to set a break-point on that line, which is of course
> not possible when we hand out non-is-stmt lines here, that means, unless absolutely necessary.
>
>>>
>>> Initially I had not been able to reproduce the reported issue
>>> with this test case.  Since I used gcc-12 + gcc-14.
>>>
>>> Then I realized that there was something wrong with the debug
>>> info in gcc-9 + gcc-10 but that was fixed in gcc-11 and following.
>>>
>>> The problem is that the inline function is completely optimized
>>> away, so there is no DW_TAG_inlined_subroutine in the debug info
>>> but the line numbers from the subroutine are still there, and make
>>> the problems, however the problem is not solved by your patch.
>>> Consider this command:
>>>
>>> $ gcc -g -Og -fcf-protection=none test.c
>>> $ gdb a.out
>>> (gdb) b main
>>> Breakpoint 1 at 0x113a: file test.c, line 3.
>>> (gdb) r
>>> Breakpoint 1, main () at test.c:3
>>> 3    static inline int bar (void) { return 1; }
>>> (gdb) s
>>> foo (count=count@entry=1) at test.c:6
>>> 6    { global += count; }
>>> (gdb) bt
>>> #0  foo (count=count@entry=1) at test.c:6
>>> #1  0x0000555555555144 in main () at test.c:10
>>>
>>>
>>> So, I have built gdb with your patch, and as you can see, the line table
>>> is completely bogus, and can not work properly.  But why does this
>>> no longer happen with gcc-11 and following?
>> 
>> I disagree though. The line table isn't completely bogus, GDB is just having trouble parsing it when searching for the current location. The backtrace - which is the only thing that Andrew's patches change - is working correctly. GDB can't step into bar (i think) because of the 0 size subprogram, which is unrelated to printing the correct line in either case.
>> 
>
> This example is more or less an exceptional one, not the normal case
> that happens most of the time.
>
> That means in general these old gcc versions do generate correct debug info.
>
> What happens here, is that the function bar is completely optimized
> away, as if it were a macro.  That means there is no
> DW_TAG_inlined_subroutine debug info for bar at all, just one is-stmt line
> entry, nothing else.  My gcc patch did simply remove that line table entry,
> because this function does no longer exist after optimization.
> It is a ghost, simply de-materialized.
>
> It is something slightly different to an empty inline, as in the PR 25987
> that you mentioned, here the debug info generated with gcc-8, 9 and 10
> looks very different.  BTW: this is the empty-inline.cc test case
> in my path, I've taken it from there.
>
> The inline function has an DW_TAG_inlined_subroutine
>  <2><11c>: Abbrev Number: 17 (DW_TAG_inlined_subroutine)
>     <11d>   DW_AT_abstract_origin: <0x18a>
>     <121>   DW_AT_entry_pc    : 0x40112b
>     <129>   DW_AT_GNU_entry_view: 2
>     <12a>   DW_AT_low_pc      : 0x40112b
>     <132>   DW_AT_high_pc     : 0
>     <13a>   DW_AT_call_file   : 1
>     <13b>   DW_AT_call_line   : 11
>     <13c>   DW_AT_call_column : 18
>     <13d>   DW_AT_sibling     : <0x14f>
>
> And the line-table around that PC looks like this:
>
> main.cpp                                   9            0x40112a        
> main.cpp                                  10            0x40112b               x
> main.cpp                                  11            0x40112b       1       x
> main.cpp                                   3            0x40112b       2       x
> main.cpp                                   3            0x40112b       3       x
> main.cpp                                   3            0x40112b       4
> main.cpp                                  11            0x40112b       5
> main.cpp                                  12            0x401130        
>
>
> but when I compare that to the test case, then I see something completely different:
>
>  <2><71>: Abbrev Number: 5 (DW_TAG_inlined_subroutine)
>     <72>   DW_AT_abstract_origin: <0x53>
>     <76>   DW_AT_low_pc      : 0x1152
>     <7e>   DW_AT_high_pc     : 0x115c
>     <86>   DW_AT_call_file   : 1
>     <87>   DW_AT_call_line   : 33
>
> This is no empty subroutine as in the example above, and the line table has
> no entries at the high_pc address 0x115c at all:
>
> dw2-inline-bt-src.c                       32              0x1144               x
> dw2-inline-bt-src.c                       33              0x1152               x
> dw2-inline-bt-src.c                       27              0x1152               x
> dw2-inline-bt-src.c                       33              0x1152        
> dw2-inline-bt-src.c                       34              0x116b        
>
>
> This is not like anything that any gcc version ever did, thus it is unrealistic.
> the line table at the inline entry does always look like this
>
> dw2-inline-bt-src.c                       33              0x1152               x
> dw2-inline-bt-src.c                       27              0x1152               x
>
> and the line-table at the exit point does always look like this:
>
> dw2-inline-bt-src.c                       28              0x115c               x
> dw2-inline-bt-src.c                       33              0x115c        
>
> that means a correct line table that matches the inlined_subroutine should look
> like this:
>
> dw2-inline-bt-src.c                       32              0x1144               x
> dw2-inline-bt-src.c                       33              0x1152               x
> dw2-inline-bt-src.c                       27              0x1152               x
> dw2-inline-bt-src.c                       28              0x115c               x
> dw2-inline-bt-src.c                       33              0x115c        
> dw2-inline-bt-src.c                       34              0x116b        
>
> So I would be okay when the test case had this kind of line table,
> then of course my patch would also not have a glitch when this test case
> is executed.
>
>>>
>>> Well, the reason is that I fixed this bug already some years ago:
>>>
>>> commit 2e6562043c48c0ae6bc9823d438685269eb11aab (HEAD, refs/bisect/new)
>>> Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
>>> Date:   Mon Dec 7 12:00:00 2020 +0100
>>>
>>>      Remove misleading debug line entries
>>>           This removes gimple_debug_begin_stmts without block info which remain
>>>      after a gimple block originating from an inline function is unused.
>>>           The line numbers from these stmts are from the inline function,
>>>      but since the inline function is completely optimized away,
>>>      there will be no DW_TAG_inlined_subroutine so the debugger has
>>>      no callstack available at this point, and therefore those
>>>      line table entries are not helpful to the user.
>>>           2020-12-10  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>>                   * cfgexpand.c (expand_gimple_basic_block): Remove special handling
>>>              of debug_inline_entries without block info.
>>>              * tree-inline.c (remap_gimple_stmt): Drop debug_nonbind_markers when
>>>              the call statement has no block info.
>>>              (copy_debug_stmt): Remove debug_nonbind_markers when inlining
>>>              and the block info is mapped to NULL.
>>>              * tree-ssa-live.c (clear_unused_block_pointer): Remove
>>>              debug_nonbind_markers originating from removed inline functions.
>>>
>>> This is from the gcc git repository, this commit landed in gcc-11 but I
>>> never considered it appropriate for a back-port to earlier stable branches.
>>> So with gcc built from this revision onwards, the line table does simply
>>> no longer contain the line 3 in above test example.
>> 
>> I guess this commit explains why I cant reproduce it anymore. However, since you should be able to use gcc 9 to build gdb (first to have stable C++17 support), I think its reasonable to expect GDB to support DWARF info emitted by gcc9 when possible, so we shouldn't discount the line table that is emitted previous to this change.
>> 
>> Also, with the updated gcc, there is no way that GDB will ever enter (or at least report to the user that the inferior entered) the bar function, because the line table doesn't ever mention it. If GDB correctly handles the fact that bar exists and is entered in the right PC, we won't have any line table to use to print the right line, so debugging this case will always be confusing based on this (gcc) patch.
>> 
>
> As I said this function does no longer exist, it is converted to a macro.
> The optimizer simply knows that bar() is a synonym for 1.
> So my patch is not to blame here.
> The line-entry in the debug-info was just an artefact without any context.
>
>>>
>>> So in comparison to my patch I found this attempt to be more radical,
>>> and still incomplete as this little experiment shows.  Admittedly my
>>> patch focuses only on inline-functions that do have block-ranges, and therefore
>>> would not change the result with a debug-info that is simply inconsistent
>>> as in this example.
>> 
>> I disagree that this approach is more radical. Andrew is just changing a small if condition based on whether you're searching for the current PC or not, so that GDB doesn't report to the user that "nothing in this line has happened yet" even though it clearly has because the function was called already.
>> 
>> Your solution is adding a whole new heuristic that changes all line table handling by GDB, for multiple different commands, and that will likely clash with the Location View Numbering that looks like it will be added in DWARF6 (https://dwarfstd.org/issues/170427.1.html).
>> 
>
> I've done some research about this, it seems to be fully implemented by gcc,
> but I must say that this will not solve the problem with the inline function
> range in the line table.  It focuses only on the visibility of local variables,
> and since e.g. my test case empty-inline.c has no local variables at all, there
> are no W_LLE_view_pair at all in the debug info here.
>
> There is however a DW_AT_GNU_entry_view, that is not mentioned in that amendment,
> but we have no issues with the line table at the entry pc.
>
> So my impression is, that if the location view for the DW_AT_low_pc and DW_AT_high_pc
> and for range begin/end will be specified in the future, then that will not
> clash with the weak bit of the line table, but instead make it more precise, which
> line table at the end of an inline are weak, this means they belong to the inline,
> and not to the outer function, also possibly also that the entry of an inline can have
> weak table entries, which would be then the ones that do not belong to the inline
> block, but are from the calling function.
>
>>>
>>>
>>> Regarding the test case there I noticed that it does something that gcc never
>>> does, namely to not emit a weak line table entry at the end of the inline
>>> function.
>>>
>>>>    7      10     0x0000000000401117 0x0000000000401117 Y
>>>>    8      3      0x0000000000401117 0x0000000000401117 Y
>>>>    9      10     0x0000000000401117 0x0000000000401117
>>>>    10     11     0x0000000000401121 0x0000000000401121 Y
>>> so at the end of the inline function's block-range we always have the
>>> weak line number from the calling function, that is what find_pc_sect_line
>>> returns with my patch, but the test case does not emit any line table entries
>>> at the end of the inline range, therefore the test case is failed with
>>> my patch, which I think is a bug in the test itself, I can make it
>>> passed if apply this change:
>> As I said, if gcc 9 or 10 emits a line table like this, we should try to support it, so I think your patch should also work with this test.
>
> Be careful what you wish, this is just a bug in the compiler, and of
> course not one that happens very often.
>
> If we do something about it, then maybe after checking the gcc version,
> that produced the debug info, but to me it looks like overkill.
>
> As I said, the patch would work, if the test would not produce
> broken line tables that do not match to the rest of the debug info.
> And really no gcc version would have produced a debug info like that.
>
>>>
>>> --- a/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
>>> +++ b/gdb/testsuite/gdb.dwarf2/dw2-inline-bt.exp
>>> @@ -168,13 +168,15 @@ proc do_test { } {
>>>                  line $bar_body
>>>                  DW_LNS_copy
>>>   -               DW_LNE_set_address line_label_4
>>> +               DW_LNE_set_address line_label_5
>>> +               DW_LNS_advance_line 1
>>> +               DW_LNS_copy
>>> +
>>> +               DW_LNE_set_address line_label_5
>>>                  line $call_line
>>>                  DW_LNS_negate_stmt
>>>                  DW_LNS_copy
>>>   -               # Skip line_label_5.
>>> -
>>>                  DW_LNE_set_address line_label_6
>>>                  DW_LNS_advance_line 1
>>>                  DW_LNS_copy
>>>
>>>
>>> The inline function range is from line_label_4 to line_label_5,
>>> and at the end of the inline range, we expect to see an is-stmt
>>> line at the end of the inline function followed by a non-is-stmt
>>> line at the calling function.
>>>
>>> So the test case would be okay with this change applied, but please
>>> do not fix the find_pc_sect_line in this way, as it would have an effect
>>> not only on inline functions, and it would of course conflict with my
>>> patch, while the issue was resolved by my earlier gcc-patch already 4
>>> years ago.
>> I hope I made a reasonable case for why changing more than inline functions is reasonable.
>> 
>
> Sorry, when I do not agree here.
> As I said, I think that showing is-stmt lines in the backtrace has the
> advantage, that it should be possible to set break-points there.
> And who knows what other non-is-stmt lines can exist without any inline
> function, that can easily be very confusing.
>
>
> Thanks,
> Bernd.