gdb: fix-up truncated inline function block ranges

This commit aims to improve GDB's handling of inline functions.  There
are two mechanisms which can tell GDB, or the user of GDB, that the
inferior is within an inline function, these are the block range
associated with an inline instance of a function, and also the line
table, which associates addresses with source lines in the program.

Currently, gcc truncates the address range for, at least some, inline
function blocks, such that a given address is considered outside the
inline function.  However, the line table maps that same address to a
line within the inline function.

A consequence of this, is that, when using 'next' to move the inferior
forward, GDB will often stop the inferior believing that the inferior
has left an inline function, and indeed, GDB will claim that the
inferior is in the outer, non-inline function, but GDB will then
display a source line from the inline function as the current location.

An example of this problem can be seen with the test
gdb.cp/step-and-next-inline.exp.  Using the
step-and-next-inline-no-header binary that is built as part of the
test:

  (gdb) file ./gdb/testsuite/outputs/gdb.cp/step-and-next-inline/step-and-next-inline-no-header
  Reading symbols from ./gdb/testsuite/outputs/gdb.cp/step-and-next-inline/step-and-next-inline-no-header...
  (gdb) break get_alias_set
  Breakpoint 1 at 0x401160: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/step-and-next-inline.cc, line 51.
  (gdb) run
  Starting program: /tmp/build/gdb/testsuite/outputs/gdb.cp/step-and-next-inline/step-and-next-inline-no-header

  Breakpoint 1, get_alias_set (t=t@entry=0x404038 <xx>)
      at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/step-and-next-inline.cc:51
  51	  if (t != NULL
  (gdb) next
  52	      && TREE_TYPE (t).z != 1
  (gdb) next
  43	  return x;	<------------- Problem line.
  (gdb) bt
  #0  get_alias_set (t=t@entry=0x404038 <xx>) at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/step-and-next-inline.cc:43
  #1  0x000000000040105e in main () at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/step-and-next-inline.cc:64
  (gdb)

I've labelled the issue as 'Problem line'.  After the second 'next'
GDB stopped thinking it was in get_alias_set, but printed a line from
the inline function behind the TREE_TYPE macro.  The 'Problem line'
should have been line 53, not line 43.

The $pc at which GDB stopped is 0x40116f.  If we then use 'objdump
--dwarf=decodedline' to view the line table for the executable, this
is what we see:

  File name                        Line number    Starting address    View    Stmt
  ...
  step-and-next-inline.cc                   38            0x401165               x
  step-and-next-inline.cc                   40            0x401165       1       x
  step-and-next-inline.cc                   40            0x401165       2
  step-and-next-inline.cc                   40            0x401167
  step-and-next-inline.cc                   42            0x40116f               x
  step-and-next-inline.cc                   43            0x40116f       1       x
  step-and-next-inline.cc                   43            0x40116f       2
  step-and-next-inline.cc                   52            0x40116f       3
  step-and-next-inline.cc                   52            0x401172
  step-and-next-inline.cc                   38            0x401177               x
  step-and-next-inline.cc                   40            0x401177       1       x
  step-and-next-inline.cc                   40            0x401177       2
  ...

NOTE: I use objdump to view the line table, not 'maintenance info
line-table' as GDB drops some line table entries that it sees as
irrelevant.  Using objdump give a complete view of the line table.

We can see that address 0x40116f is associated with three line
numbers, 42, and 43 are both within the inline function, and 52 is the
line from which the inline function was called.  Notice too that 52 is
a non-statement line.

If we now look at the block structure for the previous $pc
value 0x40116e (i.e. $pc - 1), then we see this:

  (gdb) maintenance info blocks 0x40116e
  ...
  [(block *) 0x3c62900] 0x401040..0x4011a6
    entry pc: 0x401160
    function: get_alias_set(tree*)
    symbol count: 1
    address ranges:
      0x401160..0x4011a6
      0x401040..0x401046
  [(block *) 0x3c61260] 0x401041..0x40116f
    entry pc: 0x401165
    inline function: tree_check(tree*, int)
    symbol count: 4
    address ranges:
      0x401165..0x40116f
      0x401041..0x401046
  (gdb)

Here we see 'tree_check', the inline function that backs the TREE_TYPE
macro, this is the inline function we have just stepped out of.  This
makes sense as the end-address for the tree_check block is 0x40116f,
and as the block's end address is not inclusive, that means that
0x40116f is the first address outside the block, which, is the current
$pc value.

And so, we can see what's going on.  When the 'next' starts GDB is in
get_alias_set, GDB steps forward, entering tree_check.  GDB then uses
the extent of the block to figure out where the inline function ends,
and steps forward to that address (0x40116f).  At this point, GDB
looks up the current line in the line table (43), and reports a stop
at this line.

In this commit, the fix I propose is to look for the line table
pattern seen above, a sequence of line table entries, that end with a
non-statement entry for the calling line of an inline function,
located at the exact end address of an inline function block.

When such a pattern is found, then we can extend the inline function's
address range to the next line table address, so long as doing so does
not extend the inline function beyond the extent of the containing,
non-inline, function.

In the above example, the block for the tree_check function would be
extended to end at 0x401172.  With this fix in place, and with the
same test binary, GDB now behaves like this:

  (gdb) break get_alias_set
  Breakpoint 1 at 0x401160: file /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/step-and-next-inline.cc, line 51.
  (gdb) run
  Starting program: /tmp/build/gdb/testsuite/outputs/gdb.cp/step-and-next-inline/step-and-next-inline-no-header

  Breakpoint 1, get_alias_set (t=t@entry=0x404038 <xx>)
      at /tmp/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.cp/step-and-next-inline.cc:51
  51	  if (t != NULL
  (gdb) next
  52	      && TREE_TYPE (t).z != 1
  (gdb) next
  53	      && TREE_TYPE (t).z != 2
  (gdb)

The block for the inline function has been updated, like this:

(gdb) maintenance info blocks 0x40116e
...
[(block *) 0x4965530] 0x401040..0x4011a6
  entry pc: 0x401160
  function: get_alias_set(tree*)
  symbol count: 1
  address ranges:
    0x401160..0x4011a6
    0x401040..0x401046
[(block *) 0x49640b0] 0x401040..0x401172
  entry pc: 0x401165
  inline function: tree_check(tree*, int)
  symbol count: 4
  address ranges:
    0x401165..0x401172
    0x401040..0x401041
    0x401041..0x401046

This is my alternative to the patch presented here:

  https://inbox.sourceware.org/gdb-patches/AS1PR01MB946510286FBF2497A6F03E83E4922@AS1PR01MB9465.eurprd01.prod.exchangelabs.com/

This original patch series from Bernd Edlinger contains a number of
different fixes, some have already been split out and merged into GDB,
but the core idea for how to improve inline function handling by
extending the inline block range is the same, however, the mechanism
Bernd uses is significantly different.

In the above series, the approach taken is to mark the line table at
the end address of an inline function, and a few addresses beyond
that (see the is-weak flag in the above series).  Then, when looking
up the block for a given address, if the address is within this marked
region, then we actually return the previous (inline function) block.

I believe that the above is a fair high-level summary of how the above
patch solves the inline function range problem.  Any differences are
down to my misunderstanding the above patch, for which I apologise.

My problem with the above patch is that it breaks what I think should
be an invariant of GDB, that when looking up a block for a given
address, the block returned must contain the address within its
ranges.  I feel that, if we start to break this invariant, then we
risk introducing bugs within, e.g. the stepping control code.

In contrast, my approach solves this problem during the DWARF parsing,
where problem cases are identified, and the DWARF "fixed" by extending
the block ranges.  After this, no additional changes are needed in
GDB, the address to block mapping can work as normal, and the stepping
logic can continue to work just as it always has.

The test changes for gdb.cp/step-and-next-inline.exp have been taken
from Bernd's original patch series, and I've added a Co-Author tag for
Bernd to reflect this, as well as for the inspiration that I took from
his original series when creating this alternative proposal.

If/when this patch is merged, I plan to follow up with some cleanup to
the test case gdb.cp/step-and-next-inline.exp.  I think this test
should really be moved to gdb.opt/, it's really testing optimisation
debug, not C++ features, but also the structure of the test file is a
bit of a mess.  I think with some restructuring we could make the test
more readable, and also, maybe, test some additional compiler
flags (e.g. more optimisation levels).  I've not done the refactoring
in this patch in order to make it clearer what new tests I've added,
and also, I want to leave the test similar to what's in Bernd's
original series, to make comparison easier.

The gdb.cp/step-and-next-inline.exp test was originally added by me
back in 2019, so the problems with it are of my own making.

For testing I've obviously run the entire test suite, but of
particular interest are these tests:

  gdb.cp/step-and-next-inline.exp
  gdb.dwarf2/dw2-inline-bt.exp
  gdb.opt/empty-inline-cxx.exp
  gdb.opt/empty-inline.exp
  gdb.opt/inline-bt.exp

I've run these tests with a range of different gcc versions:  9.5.0,
10.5.0, 11.5.0, 12.2.0, 13.3.0, 14.2.0, 15.1.0.  These tests all
relate to optimised debug of inline functions, and all passed with all
compiler versions listed here.

Co-Authored-By: Bernd Edlinger <bernd.edlinger@hotmail.de>
4 files changed