wallace updated this revision to Diff 318640.
wallace added a comment.
Herald added a subscriber: emaste.
Updated based on @jingham's idea, and added an independent test for this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93874/new/
https://reviews.llvm.org/D93874
Files:
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/ProcessTrace.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanStack.h
lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/source/Target/Process.cpp
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanStack.cpp
lldb/test/API/functionalities/exec/TestExec.py
lldb/unittests/Process/ProcessEventDataTest.cpp
lldb/unittests/Target/ExecutionContextTest.cpp
lldb/unittests/Thread/ThreadTest.cpp
Index: lldb/unittests/Thread/ThreadTest.cpp
===================================================================
--- lldb/unittests/Thread/ThreadTest.cpp
+++ 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"); }
Index: lldb/unittests/Target/ExecutionContextTest.cpp
===================================================================
--- lldb/unittests/Target/ExecutionContextTest.cpp
+++ 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"); }
Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===================================================================
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ 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"); }
Index: lldb/test/API/functionalities/exec/TestExec.py
===================================================================
--- lldb/test/API/functionalities/exec/TestExec.py
+++ lldb/test/API/functionalities/exec/TestExec.py
@@ -115,3 +115,78 @@
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")
+ secondprog = self.getBuildArtifact("secondprog")
+
+ # Create the target
+ target = self.dbg.CreateTarget(exe)
+
+ # Create any breakpoints we need
+ breakpoint1 = target.BreakpointCreateBySourceRegex(
+ 'Set breakpoint 1 here', lldb.SBFileSpec("main.cpp", False))
+ self.assertTrue(breakpoint1, VALID_BREAKPOINT)
+ breakpoint2 = target.BreakpointCreateBySourceRegex(
+ 'Set breakpoint 2 here', lldb.SBFileSpec("secondprog.cpp", False))
+ self.assertTrue(breakpoint2, VALID_BREAKPOINT)
+
+ # Launch the process
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory())
+ self.assertTrue(process, PROCESS_IS_VALID)
+
+ # 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.
+ threads[0].StepInstruction(False)
+
+ # Run and we should stop due to exec
+ 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.")
Index: lldb/source/Target/ThreadPlanStack.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStack.cpp
+++ 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();
Index: lldb/source/Target/ThreadPlan.cpp
===================================================================
--- lldb/source/Target/ThreadPlan.cpp
+++ 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;
Index: lldb/source/Target/ProcessTrace.cpp
===================================================================
--- lldb/source/Target/ProcessTrace.cpp
+++ 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;
}
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ 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 ||
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.h
===================================================================
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.h
+++ 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();
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===================================================================
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ 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;
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
+++ 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();
Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ 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.
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ 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();
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ 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());
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
===================================================================
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
+++ 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 {
Index: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
===================================================================
--- lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ 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;
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ 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,
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ 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.
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
===================================================================
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
+++ 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),
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===================================================================
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ 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());
Index: lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
===================================================================
--- lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
+++ 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;
Index: lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
===================================================================
--- lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
+++ 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());
Index: lldb/include/lldb/Target/ThreadPlanStack.h
===================================================================
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ 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);
Index: lldb/include/lldb/Target/ThreadPlan.h
===================================================================
--- lldb/include/lldb/Target/ThreadPlan.h
+++ 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
Index: lldb/include/lldb/Target/ProcessTrace.h
===================================================================
--- lldb/include/lldb/Target/ProcessTrace.h
+++ 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,
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ 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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
