From efd04a6c755af24e0909f42f60ea1cc671840310 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 2 Feb 2016 10:40:56 +0000 Subject: [PATCH] Fix single-stepping onto a breakpoint Summary: r259344 introduced a bug, where we fail to perform a single step, when the instruction we are stepping onto contains a breakpoint which is not valid for this thread. This fixes the problem and add a test case. Reviewers: tberghammer, emaste Subscribers: abhishek.aggarwal, lldb-commits, emaste Differential Revision: http://reviews.llvm.org/D16767 llvm-svn: 259488 --- .../TestConsecutiveBreakpoints.py | 92 ++++++++++++++++------ .../Plugins/Process/FreeBSD/FreeBSDThread.cpp | 19 ++--- .../Process/gdb-remote/ProcessGDBRemote.cpp | 15 +--- 3 files changed, 75 insertions(+), 51 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py index 7f5e5ad..d2bd9f2 100644 --- a/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py +++ b/lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py @@ -1,5 +1,5 @@ """ -Test continue from a breakpoint when there is a breakpoint on the next instruction also. +Test that we handle breakpoints on consecutive instructions correctly. """ from __future__ import print_function @@ -15,44 +15,86 @@ class ConsecutiveBreakpointsTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) - def test (self): - self.build () - self.consecutive_breakpoints_tests() - - def consecutive_breakpoints_tests(self): + def prepare_test(self): + self.build() exe = os.path.join (os.getcwd(), "a.out") # Create a target by the debugger. - target = self.dbg.CreateTarget(exe) - self.assertTrue(target, VALID_TARGET) + self.target = self.dbg.CreateTarget(exe) + self.assertTrue(self.target, VALID_TARGET) - breakpoint1 = target.BreakpointCreateBySourceRegex("Set breakpoint here", lldb.SBFileSpec("main.cpp")) - self.assertTrue(breakpoint1 and - breakpoint1.GetNumLocations() == 1, - VALID_BREAKPOINT) + breakpoint1 = self.target.BreakpointCreateBySourceRegex("Set breakpoint here", lldb.SBFileSpec("main.cpp")) + self.assertTrue(breakpoint1 and breakpoint1.GetNumLocations() == 1, VALID_BREAKPOINT) # Now launch the process, and do not stop at entry point. - process = target.LaunchSimple (None, None, self.get_process_working_directory()) - self.assertTrue(process, PROCESS_IS_VALID) + self.process = self.target.LaunchSimple (None, None, self.get_process_working_directory()) + self.assertIsNotNone(self.process, PROCESS_IS_VALID) # We should be stopped at the first breakpoint - thread = lldbutil.get_one_thread_stopped_at_breakpoint(process, breakpoint1) - self.assertIsNotNone(thread, "Expected one thread to be stopped at breakpoint 1") + self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, breakpoint1) + self.assertIsNotNone(self.thread, "Expected one thread to be stopped at breakpoint 1") # Set breakpoint to the next instruction - frame = thread.GetFrameAtIndex(0) + frame = self.thread.GetFrameAtIndex(0) address = frame.GetPCAddress() - instructions = target.ReadInstructions(address, 2) + instructions = self.target.ReadInstructions(address, 2) self.assertTrue(len(instructions) == 2) - address = instructions[1].GetAddress() + self.bkpt_address = instructions[1].GetAddress() + self.breakpoint2 = self.target.BreakpointCreateByAddress(self.bkpt_address.GetLoadAddress(self.target)) + self.assertTrue(self.breakpoint2 and self.breakpoint2.GetNumLocations() == 1, VALID_BREAKPOINT) + + def finish_test(self): + # Run the process until termination + self.process.Continue() + self.assertEquals(self.process.GetState(), lldb.eStateExited) - breakpoint2 = target.BreakpointCreateByAddress(address.GetLoadAddress(target)) - process.Continue() + @no_debug_info_test + def test_continue(self): + """Test that continue stops at the second breakpoint.""" + self.prepare_test() + self.process.Continue() + self.assertEquals(self.process.GetState(), lldb.eStateStopped) # We should be stopped at the second breakpoint - thread = lldbutil.get_one_thread_stopped_at_breakpoint(process, breakpoint2) - self.assertIsNotNone(thread, "Expected one thread to be stopped at breakpoint 2") + self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoint2) + self.assertIsNotNone(self.thread, "Expected one thread to be stopped at breakpoint 2") - # Run the process until termination - process.Continue() + self.finish_test() + + @no_debug_info_test + def test_single_step(self): + """Test that single step stops at the second breakpoint.""" + self.prepare_test() + + step_over = False + self.thread.StepInstruction(step_over) + + self.assertEquals(self.process.GetState(), lldb.eStateStopped) + self.assertEquals(self.thread.GetFrameAtIndex(0).GetPCAddress().GetLoadAddress(self.target), + self.bkpt_address.GetLoadAddress(self.target)) + self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoint2) + self.assertIsNotNone(self.thread, "Expected one thread to be stopped at breakpoint 2") + + self.finish_test() + + @no_debug_info_test + def test_single_step_thread_specific(self): + """Test that single step stops, even though the second breakpoint is not valid.""" + self.prepare_test() + + # Choose a thread other than the current one. A non-existing thread is fine. + thread_index = self.process.GetNumThreads()+1 + self.assertFalse(self.process.GetThreadAtIndex(thread_index).IsValid()) + self.breakpoint2.SetThreadIndex(thread_index) + + step_over = False + self.thread.StepInstruction(step_over) + + self.assertEquals(self.process.GetState(), lldb.eStateStopped) + self.assertEquals(self.thread.GetFrameAtIndex(0).GetPCAddress().GetLoadAddress(self.target), + self.bkpt_address.GetLoadAddress(self.target)) + self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete, + "Stop reason should be 'plan complete'") + + self.finish_test() diff --git a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp index 46cf2bd..3cb1cec 100644 --- a/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/FreeBSDThread.cpp @@ -569,20 +569,11 @@ FreeBSDThread::TraceNotify(const ProcessMessage &message) // If the current pc is a breakpoint site then set the StopInfo to Breakpoint. // Otherwise, set the StopInfo to Watchpoint or Trace. - if (bp_site) - { - lldb::break_id_t bp_id = bp_site->GetID(); - // If we have an operating system plug-in, we might have set a thread specific breakpoint using the - // operating system thread ID, so we can't make any assumptions about the thread ID so we must always - // report the breakpoint regardless of the thread. - if (bp_site->ValidForThisThread(this) || GetProcess()->GetOperatingSystem () != NULL) - SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_id)); - else - { - const bool should_stop = false; - SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_id, should_stop)); - } - } + // If we have an operating system plug-in, we might have set a thread specific breakpoint using the + // operating system thread ID, so we can't make any assumptions about the thread ID so we must always + // report the breakpoint regardless of the thread. + if (bp_site && (bp_site->ValidForThisThread(this) || GetProcess()->GetOperatingSystem () != NULL)) + SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_site->GetID())); else { POSIXBreakpointProtocol* reg_ctx = GetPOSIXBreakpointProtocol(); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 21dad4e..2636399 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2006,19 +2006,10 @@ ProcessGDBRemote::SetThreadStopInfo (lldb::tid_t tid, // If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint // Otherwise, it will be set to Trace. - if (bp_site_sp) + if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get())) { - // If the breakpoint is for this thread, then we'll report the hit, but if it is for another thread, - // we can just report no reason. - if (bp_site_sp->ValidForThisThread (thread_sp.get())) - { - thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID (*thread_sp, bp_site_sp->GetID())); - } - else - { - StopInfoSP invalid_stop_info_sp; - thread_sp->SetStopInfo (invalid_stop_info_sp); - } + thread_sp->SetStopInfo( + StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID())); } else thread_sp->SetStopInfo (StopInfo::CreateStopReasonToTrace (*thread_sp)); -- 2.7.4