Skip to content

Commit 7049b0a

Browse files
committed
Stop-hooks weren't getting called on step-out. Fix that.
There was a little bit of logic in the StopInfoBreakpoint::PerformAction that would null out the StopInfo once we had a completed plan so that the next call to GetStopInfo would replace it with the StopInfoThreadPlan. But the stop-hooks check for whether a thread stopped for a reason didn't trigger this conversion. So I added an API to do that directly, and then called it where before we just reset the StopInfo. <rdar://problem/54270767> Differential Revision: https://reviews.llvm.org/D66241 llvm-svn: 369052
1 parent fdee340 commit 7049b0a

File tree

6 files changed

+85
-4
lines changed

6 files changed

+85
-4
lines changed

lldb/include/lldb/Target/Thread.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,17 @@ class Thread : public std::enable_shared_from_this<Thread>,
11011101
// right even if you have not calculated this yourself, or if it disagrees
11021102
// with what you might have calculated.
11031103
virtual lldb::StopInfoSP GetPrivateStopInfo();
1104+
1105+
// Calculate the stop info that will be shown to lldb clients. For instance,
1106+
// a "step out" is implemented by running to a breakpoint on the function
1107+
// return PC, so the process plugin initially sets the stop info to a
1108+
// StopInfoBreakpoint. But once we've run the ShouldStop machinery, we
1109+
// discover that there's a completed ThreadPlanStepOut, and that's really
1110+
// the StopInfo we want to show. That will happen naturally the next
1111+
// time GetStopInfo is called, but if you want to force the replacement,
1112+
// you can call this.
1113+
1114+
void CalculatePublicStopInfo();
11041115

11051116
// Ask the thread subclass to set its stop info.
11061117
//
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
LEVEL = ../../make
2+
3+
C_SOURCES := main.c
4+
CFLAGS_EXTRAS += -std=c99
5+
6+
include $(LEVEL)/Makefile.rules
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
"""
2+
Test that stop hooks trigger on "step-out"
3+
"""
4+
5+
from __future__ import print_function
6+
7+
8+
import lldb
9+
import lldbsuite.test.lldbutil as lldbutil
10+
from lldbsuite.test.lldbtest import *
11+
12+
13+
class TestStopHooks(TestBase):
14+
15+
mydir = TestBase.compute_mydir(__file__)
16+
17+
# If your test case doesn't stress debug info, the
18+
# set this to true. That way it won't be run once for
19+
# each debug info format.
20+
NO_DEBUG_INFO_TESTCASE = True
21+
22+
def test_stop_hooks_step_out(self):
23+
"""Test that stop hooks fire on step-out."""
24+
self.build()
25+
self.main_source_file = lldb.SBFileSpec("main.c")
26+
self.step_out_test()
27+
28+
def setUp(self):
29+
# Call super's setUp().
30+
TestBase.setUp(self)
31+
32+
def step_out_test(self):
33+
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
34+
"Set a breakpoint here", self.main_source_file)
35+
36+
interp = self.dbg.GetCommandInterpreter()
37+
result = lldb.SBCommandReturnObject()
38+
interp.HandleCommand("target stop-hook add -o 'expr g_var++'", result)
39+
self.assertTrue(result.Succeeded, "Set the target stop hook")
40+
thread.StepOut()
41+
var = target.FindFirstGlobalVariable("g_var")
42+
self.assertTrue(var.IsValid())
43+
self.assertEqual(var.GetValueAsUnsigned(), 1, "Updated g_var")
44+
45+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#include <stdio.h>
2+
3+
static int g_var = 0;
4+
5+
int step_out_of_me()
6+
{
7+
return g_var; // Set a breakpoint here and step out.
8+
}
9+
10+
int
11+
main()
12+
{
13+
return step_out_of_me();
14+
}

lldb/source/Target/StopInfo.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -543,10 +543,10 @@ class StopInfoBreakpoint : public StopInfo {
543543
// additionally to the breakpoint
544544
m_should_stop = true;
545545

546-
// Here we clean the preset stop info so the next GetStopInfo call will
547-
// find the appropriate stop info, which should be the stop info
548-
// related to the completed plan
549-
thread_sp->ResetStopInfo();
546+
// We know we're stopping for a completed plan and we don't want to
547+
// show the breakpoint stop, so compute the public stop info immediately
548+
// here.
549+
thread_sp->CalculatePublicStopInfo();
550550
}
551551

552552
LLDB_LOGF(log,

lldb/source/Target/Thread.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,11 @@ lldb::StopInfoSP Thread::GetStopInfo() {
390390
}
391391
}
392392

393+
void Thread::CalculatePublicStopInfo() {
394+
ResetStopInfo();
395+
SetStopInfo(GetStopInfo());
396+
}
397+
393398
lldb::StopInfoSP Thread::GetPrivateStopInfo() {
394399
if (m_destroy_called)
395400
return m_stop_info_sp;

0 commit comments

Comments
 (0)