Fix stepping bug associated with non-contiguous blocks
authorKevin Buettner <kevinb@redhat.com>
Sat, 13 Jul 2019 22:59:44 +0000 (15:59 -0700)
committerKevin Buettner <kevinb@redhat.com>
Sat, 27 Jul 2019 20:43:10 +0000 (13:43 -0700)
commit98a617f8d5cf9bfb53b2e1d5cf3f0e0f188d9e53
treee2d0f50a9016508e1538146404955d99ca95999b
parent5c076da45ce9a24d47046ddb0bac02e158e6bf58
Fix stepping bug associated with non-contiguous blocks

I recently noticed the following behavior while debugging
dw2-ranges-func-low-cold.  This is one of the test programs associated
with the test gdb.dwarf2/dw2-ranges-func.exp.

(gdb) b 70
Breakpoint 1 at 0x401129: file dw2-ranges-func-lo-cold.c, line 70.
(gdb) run
Starting program: dw2-ranges-func-lo-cold

Breakpoint 1, foo ()
    at dw2-ranges-func-lo-cold.c:70
70   if (e) foo_cold (); /* foo foo_cold call */
(gdb) set var e=1
(gdb) step
[Inferior 1 (process 12545) exited normally]

This is incorrect.  When stepping, we expect a step to occur.  We do not
expect the program to exit.  Instead, we should see the following behavior:

...
(gdb) set var e=1
(gdb) step
foo ()
    at dw2-ranges-func-lo-cold.c:54
54   baz (); /* foo_cold baz call */

(Note that I've shortened the paths in the above sessions to improve
readability.)

The bug is in fill_in_stop_func() in infrun.c.  While working on
non-contiguous address range improvements in 2018, I replaced the
call to find_pc_partial_function() with a call to
find_function_entry_range_from_pc().  Although this seemed like the
right thing to do at the time, I now think that calling
find_pc_partial_function (along with some other tweaks) is the right
thing to do.

For blocks with a single contiguous range, these functions do pretty
much the same thing: when the function succeeds, the function name,
start address, and end address are all filled in.  Additionally,
find_pc_partial_function contains an additional output parameter
which is set to the block containing that PC.

For blocks with non-contiguous ranges, find_pc_partial_function
sets the start and end addresses to the start and end addresses
of the range containing the pc.  find_function_entry_range_from_pc
does what it says; it sets the start and end addresses to those
of the range containing the entry pc.

The reason that I had thought that using the entry pc range was
correct is due to the fact that fill_in_stop_func() contains some
code for advancing past the function start and entry point.  To do
this, we'd need the range that contains the entry pc.

However, when stepping, we actually want the range that contains the
stop pc.  If that range also contains the entry pc, we should then
attempt to advance stop_func_start past the start offset and entry
point.  (I haven't thought very hard about the reason for advancing
the stop_func_start in this manner.  Since it's been there for quite
a while, I'm assuming that it's still a good idea.)

Back when I wrote the test case, I had included a test for doing the
step shown in the example above.  I had problems with it, however.  At
the time, I thought it was due to differing compiler versions, so I
disabled that portion of the test.  I have now reenabled those tests,
but have left in place the logic which may be used to disable it.

The changes to dw2-ranges-func.exp depend on my other recent changes
to the file which have not been pushed yet.

Finally, I'll note that the only caller of
find_function_entry_range_from_pc() is/was fill_in_stop_func().  Once
this commit goes in, it'll be dead code.  I considered removing it,
but I think that it ought to be used (instead of
find_pc_partial_function) for determining the correct range to scan
for prologue analysis, so I'm going to leave it in place for now.

gdb/ChangeLog:

* infrun.c (fill_in_stop_func): Use find_pc_partial_function
instead of find_function_entry_range_from_pc.

testsuite/ChangeLog:

* gdb.dwarf2/dw2-ranges-func.exp (enable_foo_cold_stepping):
Enable tests associated with this flag.  Adjust regex
referencing "foo_low" to now refer to "foo_cold" instead.
gdb/ChangeLog
gdb/infrun.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.dwarf2/dw2-ranges-func.exp