gdb: fix frame_unwind_caller_WHAT functions for inline and tail calls

The 3 frame_unwind_caller_WHAT functions:

  + frame_unwind_caller_id
  + frame_unwind_caller_pc
  + frame_unwind_caller_arch

Are, I believe, currently all broken with respect to inline and tail
call functions.

The Python FinishBreakpoint type creates a breakpoint in the caller
function which, when triggered, indicates that the FinishBreakpoint
has gone out of scope.

I was writing a test for the FinishBreakpoint type which included a
tail call function, and the FinishBreakpoint was being created for the
tail call function frame.  What I observed is that the out of scope
breakpoint was never being hit.

The call stack in my new test looked like this:

  main -> tailcall_function -> normal_function

I would stop in normal_function, and then create a FinishBreakpoint
for the parent (tailcall_function) frame.  The FinishBreakpoint's out
of scope breakpoint was being correctly placed in the 'main' function,
but would never trigger.

The problem is that the breakpoint placed in 'main' holds a frame-id.
This frame-id is the frame in which the breakpoint should trigger.
This frame-id exists to prevent premature stops due to recursion.  But
in this case, when the breakpoint in 'main' was hit, despite no
recursion having occurred, the frame-id didn't match, and so the
breakpoint was ignored.

The problem is that in bpfinishpy_init we call frame_unwind_caller_id
to compute the frame-id of the frame in which we should stop, and
frame_unwind_caller_id was returning the wrong frame-id.  As far as I
can tell frame_unwind_caller_id has been broken since it was updated
for inline functions in commit edb3359dff90ef8a.

The frame_unwind_caller_id function, and all the
frame_unwind_caller_WHAT functions, are intended to return the
previous frame, but should skip over any inline, or tail call frames.
Let's look at an example call stack:

  #0 A		// A normal function.
  #1 B		// An inline function.
  #2 C		// An inline function.
  #3 D		// A normal function.
  #4 E		// A normal function.

Starting from #0, a normal function, frame_unwind_caller_id, should
return the frame-id for #3, and this is what happens.

But if we start in #1 and call frame_unwind_caller_id, then we should
still return the frame-id for #3, but this is not what happens.
Instead we return the frame-id for #4, skipping a frame.

The problem is that frame_unwind_caller_id starts by calling
skip_artificial_frames, which calls get_prev_frame_always until we
reach a non-inline (or non-tail call) frame, this moves us from #1 to

Then, back in frame_unwind_caller_id we call get_prev_frame_always,
which moves us to #4.

Then frame_unwind_caller_id finishes with a call to
skip_artificial_frames, this could potentially result in additional
frames being skipped, but in my example above this isn't the case.

The problem here is that if skip_artificial_frames skips anything,
then we have already unwound to the caller frame, and the
get_prev_frame_always call in frame_unwind_caller_id is unnecessary.

I propose to add a new helper function frame_unwind_caller_frame,
which should do the correct thing; it unwinds one frame and then calls
skip_artificial_frames.  This should do exactly what is needed.

Then all the frame_unwind_caller_WHAT functions will be updated to use
this helper function, and just extract the required property from the
resulting frame.

With this fix in place I could then write the FinishBreakpoint test,
which now works.

I took a look for other places where frame_unwind_caller_id is used
and spotted that the 'until' command does much the same thing, placing
a breakpoint in the caller frame.  As predicted, the 'until' command
is also broken when used within a tail call frame.  This patch fixes
that issue too.  There's also a test for the until command.

The bug PR gdb/28683 seems to describe this exact problem with a
specific AArch64 case given.  I haven't actually setup the environment
needed to test this bug, but I'm reasonably sure that this patch will
fix the bug.  Even if it doesn't then it's certainly related and worth
linking into the bug report.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28683

Approved-By: Tom Tromey <tom@tromey.com>

0 files changed, 0 insertions, 0 deletions