diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -2014,8 +2014,17 @@ virtual Status DisableWatchpoint(Watchpoint *wp, bool notify = true); // Thread Queries - virtual bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) = 0; + + /// Update the thread list. + /// + /// This method performs some general clean up before invoking + /// \a DoUpdateThreadList, which should be implemented by each + /// process plugin. + /// + /// \return + /// \b true if the new thread list could be generated, \b false otherwise. + bool UpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list); void UpdateThreadListIfNeeded(); @@ -2514,6 +2523,15 @@ bool trap_exceptions = false); protected: + /// Update the thread list following process plug-in's specific logic. + /// + /// This method should only be invoked by \a UpdateThreadList. + /// + /// \return + /// \b true if the new thread list could be generated, \b false otherwise. + virtual bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) = 0; + /// Actually do the reading of memory from a process. /// /// Subclasses must override this function and can return fewer bytes than diff --git a/lldb/include/lldb/Target/ProcessTrace.h b/lldb/include/lldb/Target/ProcessTrace.h --- a/lldb/include/lldb/Target/ProcessTrace.h +++ b/lldb/include/lldb/Target/ProcessTrace.h @@ -71,8 +71,8 @@ protected: void Clear(); - bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) override; + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override; private: static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp, diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h --- a/lldb/include/lldb/Target/ThreadPlan.h +++ b/lldb/include/lldb/Target/ThreadPlan.h @@ -325,6 +325,12 @@ const Target &GetTarget() const; + /// Clear the Thread* cache. + /// + /// This is useful in situations like when a new Thread list is being + /// generated. + void ClearThreadCache(); + /// Print a description of this thread to the stream \a s. /// \a thread. Don't expect that the result of GetThread is valid in /// the description method. This might get called when the underlying diff --git a/lldb/include/lldb/Target/ThreadPlanStack.h b/lldb/include/lldb/Target/ThreadPlanStack.h --- a/lldb/include/lldb/Target/ThreadPlanStack.h +++ b/lldb/include/lldb/Target/ThreadPlanStack.h @@ -95,6 +95,12 @@ void WillResume(); + /// Clear the Thread* cache that each ThreadPlan contains. + /// + /// This is useful in situations like when a new Thread list is being + /// generated. + void ClearThreadCache(); + private: const PlanStack &GetStackOfKind(ThreadPlanStack::StackKind kind) const; @@ -145,6 +151,15 @@ return &result->second; } + /// Clear the Thread* cache that each ThreadPlan contains. + /// + /// This is useful in situations like when a new Thread list is being + /// generated. + void ClearThreadCache() { + for (auto &plan_list : m_plans_list) + plan_list.second.ClearThreadCache(); + } + void Clear() { for (auto plan : m_plans_list) plan.second.ThreadDestroyed(nullptr); diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h b/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h --- a/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h +++ b/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h @@ -118,8 +118,8 @@ virtual uint32_t UpdateThreadListIfNeeded(); - bool UpdateThreadList(lldb_private::ThreadList &old_thread_list, - lldb_private::ThreadList &new_thread_list) override; + bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list, + lldb_private::ThreadList &new_thread_list) override; virtual lldb::ByteOrder GetByteOrder() const; diff --git a/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp --- a/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp @@ -166,8 +166,8 @@ return Status(); } -bool ProcessFreeBSD::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessFreeBSD::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS)); LLDB_LOGF(log, "ProcessFreeBSD::%s (pid = %" PRIu64 ")", __FUNCTION__, GetID()); diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h @@ -158,8 +158,8 @@ void Clear(); - bool UpdateThreadList(lldb_private::ThreadList &old_thread_list, - lldb_private::ThreadList &new_thread_list) override; + bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list, + lldb_private::ThreadList &new_thread_list) override; enum { eBroadcastBitAsyncContinue = (1 << 0), diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp @@ -508,8 +508,8 @@ return thread_sp; } -bool ProcessKDP::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessKDP::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { // locker will keep a mutex locked until it goes out of scope Log *log(ProcessKDPLog::GetLogIfAllCategoriesSet(KDP_LOG_THREAD)); LLDB_LOGV(log, "pid = {0}", GetID()); diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h @@ -69,8 +69,8 @@ bool CanDebug(lldb::TargetSP target_sp, bool plugin_specified_by_name) override; bool DestroyRequiresHalt() override { return false; } - bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) override; + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override; bool IsAlive() override; size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -510,8 +510,8 @@ return true; } -bool ProcessWindows::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessWindows::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_THREAD); // Add all the threads that were previously running and for which we did not // detect a thread exited event. diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h @@ -105,8 +105,8 @@ protected: void Clear(); - bool UpdateThreadList(lldb_private::ThreadList &old_thread_list, - lldb_private::ThreadList &new_thread_list) override; + bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list, + lldb_private::ThreadList &new_thread_list) override; private: struct NT_FILE_Entry { diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -261,8 +261,8 @@ return m_dyld_up.get(); } -bool ProcessElfCore::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessElfCore::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { const uint32_t num_threads = GetNumThreadContexts(); if (!m_thread_data_valid) return false; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -312,8 +312,8 @@ void Clear(); - bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) override; + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override; Status ConnectToReplayServer(); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1602,8 +1602,8 @@ return true; } -bool ProcessGDBRemote::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessGDBRemote::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { // locker will keep a mutex locked until it goes out of scope Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_THREAD)); LLDB_LOGV(log, "pid = {0}", GetID()); diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h --- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h +++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h @@ -81,8 +81,8 @@ void Clear(); - bool UpdateThreadList(lldb_private::ThreadList &old_thread_list, - lldb_private::ThreadList &new_thread_list) override; + bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list, + lldb_private::ThreadList &new_thread_list) override; lldb_private::ObjectFile *GetCoreObjectFile(); diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp --- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp +++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp @@ -536,8 +536,8 @@ return m_dyld_up.get(); } -bool ProcessMachCore::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessMachCore::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { if (old_thread_list.GetSize(false) == 0) { // Make up the thread the first time this is called so we can setup our one // and only core thread state. diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -98,8 +98,8 @@ protected: void Clear(); - bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) override; + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override; void ReadModuleList(); diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -462,8 +462,8 @@ void ProcessMinidump::Clear() { Process::m_thread_list.Clear(); } -bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessMinidump::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { for (const minidump::Thread &thread : m_thread_list) { LocationDescriptor context_location = thread.Context; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -1079,6 +1079,12 @@ return false; } +bool Process::UpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { + m_thread_plans.ClearThreadCache(); + return DoUpdateThreadList(old_thread_list, new_thread_list); +} + void Process::UpdateThreadListIfNeeded() { const uint32_t stop_id = GetStopID(); if (m_thread_list.GetSize(false) == 0 || diff --git a/lldb/source/Target/ProcessTrace.cpp b/lldb/source/Target/ProcessTrace.cpp --- a/lldb/source/Target/ProcessTrace.cpp +++ b/lldb/source/Target/ProcessTrace.cpp @@ -78,8 +78,8 @@ Process::DidAttach(process_arch); } -bool ProcessTrace::UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { +bool ProcessTrace::DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { return false; } diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp --- a/lldb/source/Target/ThreadPlan.cpp +++ b/lldb/source/Target/ThreadPlan.cpp @@ -99,6 +99,8 @@ return m_run_vote; } +void ThreadPlan::ClearThreadCache() { m_thread = nullptr; } + bool ThreadPlan::StopOthers() { ThreadPlan *prev_plan; prev_plan = GetPreviousPlan(); @@ -134,7 +136,7 @@ } } bool success = DoWillResume(resume_state, current_plan); - m_thread = nullptr; // We don't cache the thread pointer over resumes. This + ClearThreadCache(); // We don't cache the thread pointer over resumes. This // Thread might go away, and another Thread represent // the same underlying object on a later stop. return success; diff --git a/lldb/source/Target/ThreadPlanStack.cpp b/lldb/source/Target/ThreadPlanStack.cpp --- a/lldb/source/Target/ThreadPlanStack.cpp +++ b/lldb/source/Target/ThreadPlanStack.cpp @@ -369,6 +369,11 @@ return nullptr; } +void ThreadPlanStack::ClearThreadCache() { + for (lldb::ThreadPlanSP thread_plan_sp : m_plans) + thread_plan_sp->ClearThreadCache(); +} + void ThreadPlanStack::WillResume() { m_completed_plans.clear(); m_discarded_plans.clear(); diff --git a/lldb/test/API/functionalities/exec/TestExec.py b/lldb/test/API/functionalities/exec/TestExec.py --- a/lldb/test/API/functionalities/exec/TestExec.py +++ b/lldb/test/API/functionalities/exec/TestExec.py @@ -115,3 +115,68 @@ self.runCmd("bt") self.assertTrue(len(threads) == 1, "Stopped at breakpoint in exec'ed process.") + + @expectedFailureAll(archs=['i386'], + oslist=no_match(["freebsd"]), + bugnumber="rdar://28656532") + @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems + @expectedFailureNetBSD + @skipIfAsan # rdar://problem/43756823 + @skipIfWindows + def test_correct_thread_plan_state_before_exec(self): + ''' + In this test we make sure that the Thread* cache in the ThreadPlans + is cleared correctly when performing exec + ''' + + self.build() + exe = self.getBuildArtifact("a.out") + target = self.dbg.CreateTarget(exe) + + (target, process, thread, breakpoint1) = lldbutil.run_to_source_breakpoint( + self, 'Set breakpoint 1 here', lldb.SBFileSpec('main.cpp', False)) + + # The stop reason of the thread should be breakpoint. + self.assertTrue(process.GetState() == lldb.eStateStopped, + STOPPED_DUE_TO_BREAKPOINT) + + threads = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint1) + self.assertTrue(len(threads) == 1) + + # We perform an instruction step, which effectively sets the cache of the base + # thread plan, which should be cleared when a new thread list appears. + # + # Continuing after this instruction step will trigger a call to + # ThreadPlan::ShouldReportRun, which sets the ThreadPlan's Thread cache to + # the old Thread* value. In Process::UpdateThreadList we are clearing this + # cache in preparation for the new ThreadList. + # + # Not doing this stepping will cause LLDB to first execute a private single step + # past the current breakpoint, which eventually avoids the call to ShouldReportRun, + # thus not setting the cache to its invalid value. + thread.StepInstruction(False) + + # Run and we should stop due to exec + breakpoint2 = target.BreakpointCreateBySourceRegex( + 'Set breakpoint 2 here', lldb.SBFileSpec("secondprog.cpp", False)) + + process.Continue() + + self.assertFalse(process.GetState() == lldb.eStateExited, + "Process should not have exited!") + self.assertTrue(process.GetState() == lldb.eStateStopped, + "Process should be stopped at __dyld_start") + + threads = lldbutil.get_stopped_threads( + process, lldb.eStopReasonExec) + self.assertTrue( + len(threads) == 1, + "We got a thread stopped for exec.") + + # Run and we should stop at breakpoint in main after exec + process.Continue() + + threads = lldbutil.get_threads_stopped_at_breakpoint( + process, breakpoint2) + self.assertTrue(len(threads) == 1, + "Stopped at breakpoint in exec'ed process.") diff --git a/lldb/unittests/Process/ProcessEventDataTest.cpp b/lldb/unittests/Process/ProcessEventDataTest.cpp --- a/lldb/unittests/Process/ProcessEventDataTest.cpp +++ b/lldb/unittests/Process/ProcessEventDataTest.cpp @@ -53,8 +53,8 @@ Status &error) { return 0; } - virtual bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override { return false; } virtual ConstString GetPluginName() { return ConstString("Dummy"); } diff --git a/lldb/unittests/Target/ExecutionContextTest.cpp b/lldb/unittests/Target/ExecutionContextTest.cpp --- a/lldb/unittests/Target/ExecutionContextTest.cpp +++ b/lldb/unittests/Target/ExecutionContextTest.cpp @@ -58,8 +58,8 @@ Status &error) { return 0; } - virtual bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override { return false; } virtual ConstString GetPluginName() { return ConstString("Dummy"); } diff --git a/lldb/unittests/Thread/ThreadTest.cpp b/lldb/unittests/Thread/ThreadTest.cpp --- a/lldb/unittests/Thread/ThreadTest.cpp +++ b/lldb/unittests/Thread/ThreadTest.cpp @@ -51,8 +51,8 @@ Status &error) { return 0; } - virtual bool UpdateThreadList(ThreadList &old_thread_list, - ThreadList &new_thread_list) { + bool DoUpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) override { return false; } virtual ConstString GetPluginName() { return ConstString("Dummy"); }