llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: jimingham <details> <summary>Changes</summary> That isbetter done by the source line stepping plans. This fixes a test suite failure on the swift fork, but I don't know how to really write a test for it since it depends on getting to a function whose first instruction is attributed to line 0 by an ObjC direct dispatch. Turns out swift does this in its present incarnation. But this division of labor really is the right way to work the machine, since that makes the trampoline plans easier to reason about, and if stepping past line 0 is desired, there's always going to be a source line stepping plan controlling the step that will do the job. --- Full diff: https://github.com/llvm/llvm-project/pull/216770.diff 6 Files Affected: - (modified) lldb/include/lldb/Target/ThreadPlanShouldStopHere.h (+3-1) - (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp (+9) - (modified) lldb/source/Target/ThreadPlanShouldStopHere.cpp (+6-5) - (modified) lldb/source/Target/ThreadPlanStepInRange.cpp (+2-1) - (modified) lldb/source/Target/ThreadPlanStepOut.cpp (+2-1) - (modified) lldb/source/Target/ThreadPlanStepOverRange.cpp (+2-1) ``````````diff diff --git a/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h b/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h index d0094c90b91a5..809c728efdcb1 100644 --- a/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h +++ b/lldb/include/lldb/Target/ThreadPlanShouldStopHere.h @@ -60,7 +60,9 @@ class ThreadPlanShouldStopHere { eAvoidInlines = (1 << 0), eStepInAvoidNoDebug = (1 << 1), eStepOutAvoidNoDebug = (1 << 2), - eStepOutPastThunks = (1 << 3) + eStepOutPastThunks = (1 << 3), + eStepPastLine0 = (1 << 4) // Only source level plans should handle + // step past line 0 - not trampoline handlers. }; // Constructors and Destructors diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp index 5cc99ad12226e..a78a1c860cd7d 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp @@ -270,6 +270,13 @@ AppleThreadPlanStepThroughDirectDispatch :: // We only care about step in. Our parent plan will figure out what to // do when we've stepped out again. GetFlags().Clear(ThreadPlanShouldStopHere::eStepOutAvoidNoDebug); + + // Don't try to do the step past line 0 from here. That gets confused with + // our need to get past any intermediate ObjC message calls which might lie + // in the path of getting to the actual target, and if we need to do this, + // then one of the plans that is using us will make that happen. + GetFlags().Clear(ThreadPlanShouldStopHere::eStepPastLine0); + } AppleThreadPlanStepThroughDirectDispatch:: @@ -374,11 +381,13 @@ bool AppleThreadPlanStepThroughDirectDispatch::ShouldStop(Event *event_ptr) { if (!m_objc_step_through_sp->PlanSucceeded()) { LLDB_LOGF(log, "ObjC Step through plan failed. Stepping out."); } + Status error; if (InvokeShouldStopHereCallback(eFrameCompareYounger, error)) { SetPlanComplete(true); return true; } + // If we didn't want to stop at this msgSend, there might be another so // we should just continue on with the step out and see if our breakpoint // triggers again. diff --git a/lldb/source/Target/ThreadPlanShouldStopHere.cpp b/lldb/source/Target/ThreadPlanShouldStopHere.cpp index e8541cea5911a..270fa43213c02 100644 --- a/lldb/source/Target/ThreadPlanShouldStopHere.cpp +++ b/lldb/source/Target/ThreadPlanShouldStopHere.cpp @@ -97,12 +97,13 @@ bool ThreadPlanShouldStopHere::DefaultShouldStopHereCallback( } } } - // Always avoid code with line number 0. + // Source line stepping plans should always avoid code with line number 0. + // But trampoline plans should not; it's easier to reason about if they + // leave that up to the source line plan that's driving them. // FIXME: At present the ShouldStop and the StepFromHere calculate this - // independently. If this ever - // becomes expensive (this one isn't) we can try to have this set a state - // that the StepFromHere can use. - if (frame) { + // independently. If this ever becomes expensive (this one isn't) we can + // try to have this set a state that the StepFromHere can use. + if (frame && flags.Test(ThreadPlanShouldStopHere::eStepPastLine0)) { SymbolContext sc; sc = frame->GetSymbolContext(eSymbolContextLineEntry); if (sc.line_entry.line == 0) diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp index 0d72ce4f590ef..6ea3bdf40d8db 100644 --- a/lldb/source/Target/ThreadPlanStepInRange.cpp +++ b/lldb/source/Target/ThreadPlanStepInRange.cpp @@ -28,7 +28,8 @@ using namespace lldb_private; uint32_t ThreadPlanStepInRange::s_default_flag_values = ThreadPlanShouldStopHere::eStepInAvoidNoDebug | - ThreadPlanShouldStopHere::eStepOutPastThunks; + ThreadPlanShouldStopHere::eStepOutPastThunks | + ThreadPlanShouldStopHere::eStepPastLine0; // ThreadPlanStepInRange: Step through a stack range, either stepping over or // into based on the value of \a type. diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index 9be4d41ac21d8..0b7090a805fec 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -29,7 +29,8 @@ using namespace lldb; using namespace lldb_private; -uint32_t ThreadPlanStepOut::s_default_flag_values = 0; +uint32_t ThreadPlanStepOut::s_default_flag_values + = ThreadPlanShouldStopHere::eStepPastLine0; /// Computes the target frame this plan should step out to. static StackFrameSP diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index 6e90c6ab78806..56dabf7bd0898 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -26,7 +26,8 @@ using namespace lldb_private; using namespace lldb; -uint32_t ThreadPlanStepOverRange::s_default_flag_values = 0; +uint32_t ThreadPlanStepOverRange::s_default_flag_values + = ThreadPlanShouldStopHere::eStepPastLine0; // ThreadPlanStepOverRange: Step through a stack range, either stepping over or // into based on the value of \a type. `````````` </details> https://github.com/llvm/llvm-project/pull/216770 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
