[lldb] Update ThreadPlanStepOut to handle new breakpoint behavior (llvm#126838)

I will be changing breakpoint hitting behavior soon, where currently
lldb reports a breakpoint as being hit when a thread is *at* a
BreakpointSite, but possibly has not executed the breakpoint instruction
and trapped yet, to having lldb only report a breakpoint hit when the
breakpoint instruction has actually been executed.

One corner case bug with this change is that when you are stopped at a
breakpoint (that has been hit) on the last instruction of a function,
and you do `finish`, a ThreadPlanStepOut is pushed to the thread's plan
stack to put a breakpoint on the return address and resume execution.
And when the thread is asked to resume, it sees that it is at a
BreakpointSite that has been hit, and pushes a
ThreadPlanStepOverBreakpoint on the thread.   The StepOverBreakpoint
plan sees that the thread's state is eStateRunning (not eStateStepping),
so it marks itself as "auto continue" -- so once the breakpoint has
been stepped over, we will execution on the thread.

With current lldb stepping behavior ("a thread *at* a BreakpointSite is
said to have stopped with a breakpoint-hit stop reason, even if the
breakpoint hasn't been executed yet"),
`ThreadPlanStepOverBreakpoint::DoPlanExplainsStop` has a special bit of
code which detects when the thread stops with a eStopReasonBreakpoint.
It first checks if the pc is the same as when we started -- did our
"step instruction" not actually step? -- says the stop reason is
explained. Otherwise it sets auto-continue to false (because we've hit
an *unexpected* breakpoint, and we have advanced past our original pc,
and returns false - the stop reason is not explained.

So we do the "finish", lldb instruction steps, we stop *at* the
return-address breakpoint and lldb sets the thread's stop reason to
breakpoint-hit. ThreadPlanStepOverBreakpoint sees an
eStopReasonBreakpoint, sets its auto-continue to false, and says we
stopped for osme reason other than this plan. (and it will also report
`IsPlanStale()==true` so it will remove itself) Meanwhile the
ThreadPlanStepOut sees that it has stopped in the StackID it wanted to
run to, and return success.

This all changes when stopping at a breakpoint site doesn't report
breakpoint-hit until we actually execute the instruction. Now the
ThraedPlanStepOverBreakpoint looks at the thread's stop reason, it's
eStopReasonTrace (we've instruction stepped), and so it leaves its
auto-continue to `true`. ThreadPlanStepOut sees that it has reached its
goal StackID, removes its breakpoint, and says it is done.
Thread::ShouldStop thinks the auto-continue == yes vote from
ThreadPlanStepOverBreakpoint wins, and we lose control of the process.

This patch changes ThreadPlanStepOut to require that *both* (1) we are
at the StackID of the caller function, where we wanted to end up, and
(2) we have actually hit the breakpoint that we inserted.

This in effect means that now lldb instruction-steps over the breakpoint
in the callee function, stops at the return address of the caller
function. StepOverBreakpoint has completed. StepOut is still running,
and we continue the thread again. We immediatley hit the breakpoint
(that we're sitting at), and now ThreadPlanStepOut marks itself as
completed, and we return control to the user.

Jim suggests that ThreadPlanStepOverBreakpoint is a bit unusual because
it's not something pushed on the stack by a higher-order thread plan
that "owns" it, it is inserted by the Thread as it is about to resume,
if we're at a BreakpointSite. It has no connection to the thread plans
above it, but tries to set the auto-continue mode based on the state of
the thread when it is inserted (and tries to detect an unexpected
breakpoint and unset that auto-continue it previously decided on,
because it now realizes it should not influence execution control any
more). Instead maybe the
ThreadPlanStepOverBreakpoint should be inserted as a child plan of
whatever the lowest plan is on the stack at the point it is added.

I added an API test that will catch this bug in the new thread
breakpoint algorithm.

Author: jasonmolenda

lldb/source/Target/ThreadPlanStepOut.cpp

@@ -364,8 +364,11 @@ bool ThreadPlanStepOut::ShouldStop(Event *event_ptr) {
   }
 
   if (!done) {
-    StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID();
-    done = !(frame_zero_id < m_step_out_to_id);
+    StopInfoSP stop_info_sp = GetPrivateStopInfo();
+    if (stop_info && stop_info_sp->GetStopReason() == eStopReasonBreakpoint) {
+      StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID();
+      done = !(frame_zero_id < m_step_out_to_id);
+    }
   }
 
   // The normal step out computations think we are done, so all we need to do

lldb/test/API/functionalities/thread/finish-from-empty-func/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/functionalities/thread/finish-from-empty-func/TestEmptyFuncThreadStepOut.py

@@ -0,0 +1,43 @@
+"""
+Test finish out of an empty function (may be one-instruction long)
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class FinishFromEmptyFunctionTestCase(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_finish_from_empty_function(self):
+        """Test that when stopped at a breakpoint in an empty function, finish leaves it correctly."""
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+        target, process, thread, _ = lldbutil.run_to_name_breakpoint(
+            self, "done", exe_name=exe
+        )
+        if self.TraceOn():
+            self.runCmd("bt")
+
+        correct_stepped_out_line = line_number("main.c", "leaving main")
+        return_statement_line = line_number("main.c", "return 0")
+        safety_bp = target.BreakpointCreateByLocation(
+            lldb.SBFileSpec("main.c"), return_statement_line
+        )
+        self.assertTrue(safety_bp.IsValid())
+
+        error = lldb.SBError()
+        thread.StepOut(error)
+        self.assertTrue(error.Success())
+
+        if self.TraceOn():
+            self.runCmd("bt")
+
+        frame = thread.GetSelectedFrame()
+        self.assertEqual(
+            frame.line_entry.GetLine(),
+            correct_stepped_out_line,
+            "Step-out lost control of execution, ran too far",
+        )

lldb/test/API/functionalities/thread/finish-from-empty-func/main.c

@@ -0,0 +1,8 @@
+#include <stdio.h>
+void done() {}
+int main() {
+  puts("in main");
+  done();
+  puts("leaving main");
+  return 0;
+}