Author: bulasevich Date: Wed Feb 15 05:42:47 2017 New Revision: 295168 URL: http://llvm.org/viewvc/llvm-project?rev=295168&view=rev Log: Bug 30863 - Step doesn't stop with conditional breakpoint on the next line Differential Revisions: https://reviews.llvm.org/D26497 (committed r290168, temporary reverted r290197) https://reviews.llvm.org/D28945 (fix for Ubuntu tests fail) https://reviews.llvm.org/D29909 (fix for TestCallThatThrows test fail)
Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/Makefile lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/main.cpp Modified: lldb/trunk/include/lldb/Target/Thread.h lldb/trunk/include/lldb/Target/ThreadPlan.h lldb/trunk/source/Target/Process.cpp lldb/trunk/source/Target/StopInfo.cpp lldb/trunk/source/Target/Thread.cpp lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp lldb/trunk/source/Target/ThreadPlanStepRange.cpp Modified: lldb/trunk/include/lldb/Target/Thread.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Thread.h?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/include/lldb/Target/Thread.h (original) +++ lldb/trunk/include/lldb/Target/Thread.h Wed Feb 15 05:42:47 2017 @@ -126,6 +126,7 @@ public: // bit of data. lldb::StopInfoSP stop_info_sp; // You have to restore the stop info or you // might continue with the wrong signals. + std::vector<lldb::ThreadPlanSP> m_completed_plan_stack; lldb::RegisterCheckpointSP register_backup_sp; // You need to restore the registers, of course... uint32_t current_inlined_depth; @@ -1029,6 +1030,15 @@ public: bool WasThreadPlanDiscarded(ThreadPlan *plan); //------------------------------------------------------------------ + /// Check if we have completed plan to override breakpoint stop reason + /// + /// @return + /// Returns true if completed plan stack is not empty + /// false otherwise. + //------------------------------------------------------------------ + bool CompletedPlanOverridesBreakpoint(); + + //------------------------------------------------------------------ /// Queues a generic thread plan. /// /// @param[in] plan_sp @@ -1213,6 +1223,8 @@ public: void SetStopInfo(const lldb::StopInfoSP &stop_info_sp); + void ResetStopInfo(); + void SetShouldReportStop(Vote vote); //---------------------------------------------------------------------- Modified: lldb/trunk/include/lldb/Target/ThreadPlan.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ThreadPlan.h?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/include/lldb/Target/ThreadPlan.h (original) +++ lldb/trunk/include/lldb/Target/ThreadPlan.h Wed Feb 15 05:42:47 2017 @@ -40,9 +40,10 @@ namespace lldb_private { // The thread maintaining a thread plan stack, and you program the actions of a // particular thread // by pushing plans onto the plan stack. -// There is always a "Current" plan, which is the head of the plan stack, +// There is always a "Current" plan, which is the top of the plan stack, // though in some cases -// a plan may defer to plans higher in the stack for some piece of information. +// a plan may defer to plans higher in the stack for some piece of information +// (let us define that the plan stack grows downwards). // // The plan stack is never empty, there is always a Base Plan which persists // through the life @@ -109,6 +110,15 @@ namespace lldb_private { // plans in the time between when // your plan gets unshipped and the next resume. // +// Thread State Checkpoint: +// +// Note that calling functions on target process (ThreadPlanCallFunction) changes +// current thread state. The function can be called either by direct user demand or +// internally, for example lldb allocates memory on device to calculate breakpoint +// condition expression - on Linux it is performed by calling mmap on device. +// ThreadStateCheckpoint saves Thread state (stop info and completed +// plan stack) to restore it after completing function call. +// // Over the lifetime of the plan, various methods of the ThreadPlan are then // called in response to changes of state in // the process we are debugging as follows: @@ -149,7 +159,7 @@ namespace lldb_private { // If the Current plan answers "true" then it is asked if the stop should // percolate all the way to the // user by calling the ShouldStop method. If the current plan doesn't explain -// the stop, then we query down +// the stop, then we query up // the plan stack for a plan that does explain the stop. The plan that does // explain the stop then needs to // figure out what to do about the plans below it in the stack. If the stop is @@ -170,7 +180,7 @@ namespace lldb_private { // event it didn't directly handle // it can designate itself a "Master" plan by responding true to IsMasterPlan, // and then if it wants not to be -// discarded, it can return true to OkayToDiscard, and it and all its dependent +// discarded, it can return false to OkayToDiscard, and it and all its dependent // plans will be preserved when // we resume execution. // @@ -207,7 +217,7 @@ namespace lldb_private { // // If a plan says responds "true" to ShouldStop, then it is asked if it's job // is complete by calling -// MischiefManaged. If that returns true, the thread is popped from the plan +// MischiefManaged. If that returns true, the plan is popped from the plan // stack and added to the // Completed Plan Stack. Then the next plan in the stack is asked if it // ShouldStop, and it returns "true", @@ -241,9 +251,9 @@ namespace lldb_private { // // When the process stops, the thread is given a StopReason, in the form of a // StopInfo object. If there is a completed -// plan corresponding to the stop, then the "actual" stop reason will be +// plan corresponding to the stop, then the "actual" stop reason can be // suppressed, and instead a StopInfoThreadPlan -// object will be cons'ed up from the highest completed plan in the stack. +// object will be cons'ed up from the top completed plan in the stack. // However, if the plan doesn't want to be // the stop reason, then it can call SetPlanComplete and pass in "false" for // the "success" parameter. In that case, Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/Makefile?rev=295168&view=auto ============================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/Makefile (added) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/Makefile Wed Feb 15 05:42:47 2017 @@ -0,0 +1,9 @@ +LEVEL = ../../../make + +CXX_SOURCES := main.cpp + +ifneq (,$(findstring icc,$(CC))) + CXXFLAGS += -debug inline-debug-info +endif + +include $(LEVEL)/Makefile.rules Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py?rev=295168&view=auto ============================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py (added) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py Wed Feb 15 05:42:47 2017 @@ -0,0 +1,119 @@ +""" +Test that breakpoints do not affect stepping. +Check for correct StopReason when stepping to the line with breakpoint +which chould be eStopReasonBreakpoint in general, +and eStopReasonPlanComplete when breakpoint's condition fails. +""" + +from __future__ import print_function + +import unittest2 +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class StepOverBreakpointsTestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def setUp(self): + TestBase.setUp(self) + + self.build() + exe = os.path.join(os.getcwd(), "a.out") + src = lldb.SBFileSpec("main.cpp") + + # Create a target by the debugger. + self.target = self.dbg.CreateTarget(exe) + self.assertTrue(self.target, VALID_TARGET) + + # Setup four breakpoints, two of them with false condition + self.line1 = line_number('main.cpp', "breakpoint_1") + self.line4 = line_number('main.cpp', "breakpoint_4") + + self.breakpoint1 = self.target.BreakpointCreateByLocation(src, self.line1) + self.assertTrue( + self.breakpoint1 and self.breakpoint1.GetNumLocations() == 1, + VALID_BREAKPOINT) + + self.breakpoint2 = self.target.BreakpointCreateBySourceRegex("breakpoint_2", src) + self.breakpoint2.GetLocationAtIndex(0).SetCondition('false') + + self.breakpoint3 = self.target.BreakpointCreateBySourceRegex("breakpoint_3", src) + self.breakpoint3.GetLocationAtIndex(0).SetCondition('false') + + self.breakpoint4 = self.target.BreakpointCreateByLocation(src, self.line4) + + # Start debugging + self.process = self.target.LaunchSimple( + None, None, self.get_process_working_directory()) + self.assertIsNotNone(self.process, PROCESS_IS_VALID) + self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoint1) + self.assertIsNotNone(self.thread, "Didn't stop at breakpoint 1.") + + def test_step_instruction(self): + # Count instructions between breakpoint_1 and breakpoint_4 + contextList = self.target.FindFunctions('main', lldb.eFunctionNameTypeAuto) + self.assertEquals(contextList.GetSize(), 1) + symbolContext = contextList.GetContextAtIndex(0) + function = symbolContext.GetFunction() + self.assertTrue(function) + instructions = function.GetInstructions(self.target) + addr_1 = self.breakpoint1.GetLocationAtIndex(0).GetAddress() + addr_4 = self.breakpoint4.GetLocationAtIndex(0).GetAddress() + for i in range(instructions.GetSize()) : + addr = instructions.GetInstructionAtIndex(i).GetAddress() + if (addr == addr_1) : index_1 = i + if (addr == addr_4) : index_4 = i + + steps_expected = index_4 - index_1 + step_count = 0 + # Step from breakpoint_1 to breakpoint_4 + while True: + self.thread.StepInstruction(True) + step_count = step_count + 1 + self.assertEquals(self.process.GetState(), lldb.eStateStopped) + self.assertTrue(self.thread.GetStopReason() == lldb.eStopReasonPlanComplete or + self.thread.GetStopReason() == lldb.eStopReasonBreakpoint) + if (self.thread.GetStopReason() == lldb.eStopReasonBreakpoint) : + # we should not stop on breakpoint_2 and _3 because they have false condition + self.assertEquals(self.thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), self.line4) + # breakpoint_2 and _3 should not affect step count + self.assertTrue(step_count >= steps_expected) + break + + # Run the process until termination + self.process.Continue() + self.assertEquals(self.process.GetState(), lldb.eStateExited) + + def test_step_over(self): + #lldb.DBG.EnableLog("lldb", ["step","breakpoint"]) + + self.thread.StepOver() + # We should be stopped at the breakpoint_2 line with stop plan complete reason + self.assertEquals(self.process.GetState(), lldb.eStateStopped) + self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete) + + self.thread.StepOver() + # We should be stopped at the breakpoint_3 line with stop plan complete reason + self.assertEquals(self.process.GetState(), lldb.eStateStopped) + self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonPlanComplete) + + self.thread.StepOver() + # We should be stopped at the breakpoint_4 + self.assertEquals(self.process.GetState(), lldb.eStateStopped) + self.assertEquals(self.thread.GetStopReason(), lldb.eStopReasonBreakpoint) + thread1 = lldbutil.get_one_thread_stopped_at_breakpoint(self.process, self.breakpoint4) + self.assertEquals(self.thread, thread1, "Didn't stop at breakpoint 4.") + + # Check that stepping does not affect breakpoint's hit count + self.assertEquals(self.breakpoint1.GetHitCount(), 1) + self.assertEquals(self.breakpoint2.GetHitCount(), 0) + self.assertEquals(self.breakpoint3.GetHitCount(), 0) + self.assertEquals(self.breakpoint4.GetHitCount(), 1) + + # Run the process until termination + self.process.Continue() + self.assertEquals(self.process.GetState(), lldb.eStateExited) + Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/main.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/main.cpp?rev=295168&view=auto ============================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/main.cpp (added) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/step_over_breakpoint/main.cpp Wed Feb 15 05:42:47 2017 @@ -0,0 +1,12 @@ + +int func() { return 1; } + +int +main(int argc, char const *argv[]) +{ + int a = 0; // breakpoint_1 + int b = func(); // breakpoint_2 + a = b + func(); // breakpoint_3 + return 0; // breakpoint_4 +} + Modified: lldb/trunk/source/Target/Process.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/source/Target/Process.cpp (original) +++ lldb/trunk/source/Target/Process.cpp Wed Feb 15 05:42:47 2017 @@ -5227,15 +5227,9 @@ Process::RunThreadPlan(ExecutionContext do_resume = false; handle_running_event = true; } else { - StopInfoSP stop_info_sp(thread_sp->GetStopInfo()); - StopReason stop_reason = eStopReasonInvalid; - if (stop_info_sp) - stop_reason = stop_info_sp->GetStopReason(); + ThreadPlanSP plan = thread->GetCompletedPlan(); + if (plan == thread_plan_sp && plan->PlanSucceeded()) { - // FIXME: We only check if the stop reason is plan complete, - // should we make sure that - // it is OUR plan that is complete? - if (stop_reason == eStopReasonPlanComplete) { if (log) log->PutCString("Process::RunThreadPlan(): execution " "completed successfully."); @@ -5246,9 +5240,11 @@ Process::RunThreadPlan(ExecutionContext return_value = eExpressionCompleted; } else { + StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); // Something restarted the target, so just wait for it to // stop for real. - if (stop_reason == eStopReasonBreakpoint) { + if (stop_info_sp && + stop_info_sp->GetStopReason() == eStopReasonBreakpoint) { if (log) log->Printf("Process::RunThreadPlan() stopped for " "breakpoint: %s.", Modified: lldb/trunk/source/Target/StopInfo.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StopInfo.cpp?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/source/Target/StopInfo.cpp (original) +++ lldb/trunk/source/Target/StopInfo.cpp Wed Feb 15 05:42:47 2017 @@ -269,6 +269,7 @@ protected: if (!m_should_perform_action) return; m_should_perform_action = false; + bool internal_breakpoint = true; ThreadSP thread_sp(m_thread_wp.lock()); @@ -495,6 +496,9 @@ protected: if (callback_says_stop) m_should_stop = true; + if (m_should_stop && !bp_loc_sp->GetBreakpoint().IsInternal()) + internal_breakpoint = false; + // If we are going to stop for this breakpoint, then remove the // breakpoint. if (callback_says_stop && bp_loc_sp && @@ -526,6 +530,20 @@ protected: "Process::%s could not find breakpoint site id: %" PRId64 "...", __FUNCTION__, m_value); } + + if ((m_should_stop == false || internal_breakpoint) + && thread_sp->CompletedPlanOverridesBreakpoint()) { + + // Override should_stop decision when we have + // completed step plan additionally to the breakpoint + m_should_stop = true; + + // Here we clean the preset stop info so the next + // GetStopInfo call will find the appropriate stop info, + // which should be the stop info related to the completed plan + thread_sp->ResetStopInfo(); + } + if (log) log->Printf("Process::%s returning from action with m_should_stop: %d.", __FUNCTION__, m_should_stop); Modified: lldb/trunk/source/Target/Thread.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cpp?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/source/Target/Thread.cpp (original) +++ lldb/trunk/source/Target/Thread.cpp Wed Feb 15 05:42:47 2017 @@ -380,24 +380,32 @@ lldb::StopInfoSP Thread::GetStopInfo() { if (m_destroy_called) return m_stop_info_sp; - ThreadPlanSP plan_sp(GetCompletedPlan()); + ThreadPlanSP completed_plan_sp(GetCompletedPlan()); ProcessSP process_sp(GetProcess()); const uint32_t stop_id = process_sp ? process_sp->GetStopID() : UINT32_MAX; - if (plan_sp && plan_sp->PlanSucceeded()) { - return StopInfo::CreateStopReasonWithPlan(plan_sp, GetReturnValueObject(), - GetExpressionVariable()); + + // Here we select the stop info according to priorirty: + // - m_stop_info_sp (if not trace) - preset value + // - completed plan stop info - new value with plan from completed plan stack + // - m_stop_info_sp (trace stop reason is OK now) + // - ask GetPrivateStopInfo to set stop info + + bool have_valid_stop_info = m_stop_info_sp && + m_stop_info_sp ->IsValid() && + m_stop_info_stop_id == stop_id; + bool have_valid_completed_plan = completed_plan_sp && completed_plan_sp->PlanSucceeded(); + bool plan_overrides_trace = + have_valid_stop_info && have_valid_completed_plan + && (m_stop_info_sp->GetStopReason() == eStopReasonTrace); + + if (have_valid_stop_info && !plan_overrides_trace) { + return m_stop_info_sp; + } else if (have_valid_completed_plan) { + return StopInfo::CreateStopReasonWithPlan( + completed_plan_sp, GetReturnValueObject(), GetExpressionVariable()); } else { - if ((m_stop_info_stop_id == stop_id) || // Stop info is valid, just return - // what we have (even if empty) - (m_stop_info_sp && - m_stop_info_sp - ->IsValid())) // Stop info is valid, just return what we have - { - return m_stop_info_sp; - } else { - GetPrivateStopInfo(); - return m_stop_info_sp; - } + GetPrivateStopInfo(); + return m_stop_info_sp; } } @@ -459,6 +467,12 @@ bool Thread::StopInfoIsUpToDate() const // date... } +void Thread::ResetStopInfo() { + if (m_stop_info_sp) { + m_stop_info_sp.reset(); + } +} + void Thread::SetStopInfo(const lldb::StopInfoSP &stop_info_sp) { m_stop_info_sp = stop_info_sp; if (m_stop_info_sp) { @@ -526,7 +540,8 @@ bool Thread::CheckpointThreadState(Threa if (process_sp) saved_state.orig_stop_id = process_sp->GetStopID(); saved_state.current_inlined_depth = GetCurrentInlinedDepth(); - + saved_state.m_completed_plan_stack = m_completed_plan_stack; + return true; } @@ -559,6 +574,7 @@ bool Thread::RestoreThreadStateFromCheck SetStopInfo(saved_state.stop_info_sp); GetStackFrameList()->SetCurrentInlinedDepth( saved_state.current_inlined_depth); + m_completed_plan_stack = saved_state.m_completed_plan_stack; return true; } @@ -895,6 +911,9 @@ bool Thread::ShouldStop(Event *event_ptr if (should_stop) { ThreadPlan *plan_ptr = GetCurrentPlan(); + + // Discard the stale plans and all plans below them in the stack, + // plus move the completed plans to the completed plan stack while (!PlanIsBasePlan(plan_ptr)) { bool stale = plan_ptr->IsPlanStale(); ThreadPlan *examined_plan = plan_ptr; @@ -905,7 +924,15 @@ bool Thread::ShouldStop(Event *event_ptr log->Printf( "Plan %s being discarded in cleanup, it says it is already done.", examined_plan->GetName()); - DiscardThreadPlansUpToPlan(examined_plan); + while (GetCurrentPlan() != examined_plan) { + DiscardPlan(); + } + if (examined_plan->IsPlanComplete()) { + // plan is complete but does not explain the stop (example: step to a line + // with breakpoint), let us move the plan to completed_plan_stack anyway + PopPlan(); + } else + DiscardPlan(); } } } @@ -1133,6 +1160,10 @@ bool Thread::WasThreadPlanDiscarded(Thre return false; } +bool Thread::CompletedPlanOverridesBreakpoint() { + return (!m_completed_plan_stack.empty()) ; +} + ThreadPlan *Thread::GetPreviousPlan(ThreadPlan *current_plan) { if (current_plan == nullptr) return nullptr; Modified: lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp (original) +++ lldb/trunk/source/Target/ThreadPlanStepInstruction.cpp Wed Feb 15 05:42:47 2017 @@ -94,6 +94,15 @@ bool ThreadPlanStepInstruction::IsPlanSt Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP)); StackID cur_frame_id = m_thread.GetStackFrameAtIndex(0)->GetStackID(); if (cur_frame_id == m_stack_id) { + // Set plan Complete when we reach next instruction + uint64_t pc = m_thread.GetRegisterContext()->GetPC(0); + uint32_t max_opcode_size = m_thread.CalculateTarget() + ->GetArchitecture().GetMaximumOpcodeByteSize(); + bool next_instruction_reached = (pc > m_instruction_addr) && + (pc <= m_instruction_addr + max_opcode_size); + if (next_instruction_reached) { + SetPlanComplete(); + } return (m_thread.GetRegisterContext()->GetPC(0) != m_instruction_addr); } else if (cur_frame_id < m_stack_id) { // If the current frame is younger than the start frame and we are stepping Modified: lldb/trunk/source/Target/ThreadPlanStepRange.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlanStepRange.cpp?rev=295168&r1=295167&r2=295168&view=diff ============================================================================== --- lldb/trunk/source/Target/ThreadPlanStepRange.cpp (original) +++ lldb/trunk/source/Target/ThreadPlanStepRange.cpp Wed Feb 15 05:42:47 2017 @@ -461,6 +461,16 @@ bool ThreadPlanStepRange::IsPlanStale() // One tricky bit here is that some stubs don't push a frame, so we should. // check that we are in the same symbol. if (!InRange()) { + // Set plan Complete when we reach next instruction just after the range + lldb::addr_t addr = m_thread.GetRegisterContext()->GetPC() - 1; + size_t num_ranges = m_address_ranges.size(); + for (size_t i = 0; i < num_ranges; i++) { + bool in_range = m_address_ranges[i].ContainsLoadAddress( + addr, m_thread.CalculateTarget().get()); + if (in_range) { + SetPlanComplete(); + } + } return true; } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits