wallace created this revision.
wallace added reviewers: clayborg, jingham.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

On Linux, unlike Darwin, after a process performs exec, the thread id doesn't 
change. This causes any living thread plans to fail because the original thread 
is now invalid. This crash only happens if thread plans are still executing 
when the original thread dies.

I've been testing this on my Linux machine and it happens mostly when I attach 
to a process that then execs. If LLDB launches that process, this problem 
almost never happens. As with most race conditions, it's hard to predict and 
reproduce. In any case, I added a test with an attach scenario that works 
correctly on my machine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93874

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/exec/TestExec.py
  lldb/test/API/functionalities/exec/main.cpp

Index: lldb/test/API/functionalities/exec/main.cpp
===================================================================
--- lldb/test/API/functionalities/exec/main.cpp
+++ lldb/test/API/functionalities/exec/main.cpp
@@ -1,5 +1,6 @@
 #define _POSIX_C_SOURCE 200809L
 
+#include <csignal>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
@@ -8,6 +9,9 @@
 #include <unistd.h>
 
 int main(int argc, char const **argv) {
+  if (getenv("LLDB_TEST_SHOULD_ATTACH") != nullptr)
+    raise(SIGSTOP);
+
   char *buf = strdup(argv[0]); // Set breakpoint 1 here
   std::string directory_name(::dirname(buf));
 
Index: lldb/test/API/functionalities/exec/TestExec.py
===================================================================
--- lldb/test/API/functionalities/exec/TestExec.py
+++ lldb/test/API/functionalities/exec/TestExec.py
@@ -24,7 +24,8 @@
     @skipIfAsan # rdar://problem/43756823
     @skipIfWindows
     def test_hitting_exec (self):
-        self.do_test(False)
+        self.do_test(False, False)
+        self.do_test(False, True)
 
     @expectedFailureAll(archs=['i386'],
                         oslist=no_match(["freebsd"]),
@@ -34,9 +35,10 @@
     @skipIfAsan # rdar://problem/43756823
     @skipIfWindows
     def test_skipping_exec (self):
-        self.do_test(True)
+        self.do_test(True, False)
+        self.do_test(True, True)
 
-    def do_test(self, skip_exec):
+    def do_test(self, skip_exec, should_attach):
         self.build()
         exe = self.getBuildArtifact("a.out")
         secondprog = self.getBuildArtifact("secondprog")
@@ -52,10 +54,31 @@
             '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)
+        if not should_attach:
+            # Launch the process
+            process = target.LaunchSimple(
+                None, None, self.get_process_working_directory())
+            self.assertTrue(process, PROCESS_IS_VALID)
+        else:
+            # Spawn the process and attach to it
+            popen = subprocess.Popen(exe, env={"LLDB_TEST_SHOULD_ATTACH": "1"})
+            if self.TraceOn():
+                print("Will attach to PID" + str(popen.pid))
+
+            error = lldb.SBError()
+            attach_info = lldb.SBAttachInfo(popen.pid)
+            process = target.Attach(attach_info, error)
+            self.assertTrue(error.Success() and process)
+
+            # In some systems lldb will stop for a second time at the SIGSTOP attach
+            # point. That might be need to be fixed in a different patch.
+            # To avoid making guesses, we continue until we hit breakpoint1.
+            while True:
+                error = process.Continue()
+                self.assertTrue(error.Success())
+                threads = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint1)
+                if len(threads) > 0:
+                    break
 
         if self.TraceOn():
             self.runCmd("settings show target.process.stop-on-exec", check=False)
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1248,6 +1248,12 @@
   return false;
 }
 
+void Process::ProcessDidExec() {
+  m_thread_list_real.Clear();
+  m_thread_list.Clear();
+  m_thread_plans.Clear();
+}
+
 void Process::UpdateThreadListIfNeeded() {
   const uint32_t stop_id = GetStopID();
   if (m_thread_list.GetSize(false) == 0 ||
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
@@ -2611,8 +2611,7 @@
     Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
     LLDB_LOGF(log, "ProcessGDBRemote::SetLastStopPacket () - detected exec");
 
-    m_thread_list_real.Clear();
-    m_thread_list.Clear();
+    ProcessDidExec();
     BuildDynamicRegisterInfo(true);
     m_gdb_comm.ResetDiscoverableSettings(did_exec);
   }
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2075,6 +2075,10 @@
 
   void UpdateThreadListIfNeeded();
 
+  /// Method to be invoked to signal that the underlying process has exec'ed
+  /// and some clean up should be done to prepare for the new process state.
+  void ProcessDidExec();
+
   ThreadList &GetThreadList() { return m_thread_list; }
 
   // When ExtendedBacktraces are requested, the HistoryThreads that are created
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to