[ThreadPlan] fix exec on Linux

Author: walter-erquinigo

lldb/include/lldb/Target/Process.h

@@ -2014,8 +2014,17 @@ class Process : public std::enable_shared_from_this<Process>,
   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 @@ void PruneThreadPlans();
                                 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/include/lldb/Target/ProcessTrace.h

@@ -71,8 +71,8 @@ class ProcessTrace : public PostMortemProcess {
 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,

lldb/include/lldb/Target/ThreadPlan.h

@@ -325,6 +325,12 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
 
   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

lldb/include/lldb/Target/ThreadPlanStack.h

@@ -95,6 +95,12 @@ class ThreadPlanStack {
 
   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 @@ class ThreadPlanStackMap {
       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);

lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp

@@ -166,8 +166,8 @@ Status ProcessFreeBSD::DoResume() {
   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());

lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h

@@ -118,8 +118,8 @@ class ProcessFreeBSD : public lldb_private::Process {
 
   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;
 

lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

@@ -508,8 +508,8 @@ lldb::ThreadSP ProcessKDP::GetKernelThread() {
   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());

lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h

@@ -158,8 +158,8 @@ class ProcessKDP : public lldb_private::Process {
 
   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),

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

@@ -510,8 +510,8 @@ bool ProcessWindows::CanDebug(lldb::TargetSP target_sp,
   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.

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h

@@ -69,8 +69,8 @@ class ProcessWindows : public Process, public ProcessDebugger {
   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,

lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

@@ -261,8 +261,8 @@ lldb_private::DynamicLoader *ProcessElfCore::GetDynamicLoader() {
   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;

lldb/source/Plugins/Process/elf-core/ProcessElfCore.h

@@ -105,8 +105,8 @@ class ProcessElfCore : public lldb_private::PostMortemProcess {
 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 {

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -1602,8 +1602,8 @@ bool ProcessGDBRemote::UpdateThreadIDList() {
   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());

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -312,8 +312,8 @@ class ProcessGDBRemote : public Process,
 
   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();
 

lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

@@ -536,8 +536,8 @@ lldb_private::DynamicLoader *ProcessMachCore::GetDynamicLoader() {
   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.

lldb/source/Plugins/Process/mach-core/ProcessMachCore.h

@@ -81,8 +81,8 @@ class ProcessMachCore : public lldb_private::PostMortemProcess {
 
   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();
 

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

@@ -462,8 +462,8 @@ Status ProcessMinidump::GetMemoryRegions(MemoryRegionInfos &region_list) {
 
 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;
 

lldb/source/Plugins/Process/minidump/ProcessMinidump.h

@@ -98,8 +98,8 @@ class ProcessMinidump : public PostMortemProcess {
 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();
 

lldb/source/Target/Process.cpp

@@ -1079,6 +1079,12 @@ bool Process::SetProcessExitStatus(
   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 ||

lldb/source/Target/ProcessTrace.cpp

@@ -78,8 +78,8 @@ void ProcessTrace::DidAttach(ArchSpec &process_arch) {
   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;
 }
 

lldb/source/Target/ThreadPlan.cpp

@@ -99,6 +99,8 @@ Vote ThreadPlan::ShouldReportRun(Event *event_ptr) {
   return m_run_vote;
 }
 
+void ThreadPlan::ClearThreadCache() { m_thread = nullptr; }
+
 bool ThreadPlan::StopOthers() {
   ThreadPlan *prev_plan;
   prev_plan = GetPreviousPlan();
@@ -134,7 +136,7 @@ bool ThreadPlan::WillResume(StateType resume_state, bool current_plan) {
     }
   }
   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;

lldb/source/Target/ThreadPlanStack.cpp

@@ -369,6 +369,11 @@ ThreadPlan *ThreadPlanStack::GetInnermostExpression() const {
   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();

lldb/test/API/functionalities/exec/TestExec.py

@@ -115,3 +115,68 @@ def cleanup():
                     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.")

lldb/unittests/Process/ProcessEventDataTest.cpp

@@ -53,8 +53,8 @@ class DummyProcess : public Process {
                               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"); }

lldb/unittests/Target/ExecutionContextTest.cpp

@@ -58,8 +58,8 @@ class DummyProcess : public Process {
                               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"); }

lldb/unittests/Thread/ThreadTest.cpp

@@ -51,8 +51,8 @@ class DummyProcess : public Process {
                               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"); }