[Lldb-commits] [lldb] r313655 - Re-land r313210 - Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

Adrian McCarthy via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 19 11:07:33 PDT 2017


Author: amccarth
Date: Tue Sep 19 11:07:33 2017
New Revision: 313655

URL: http://llvm.org/viewvc/llvm-project?rev=313655&view=rev
Log:
Re-land r313210 - Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

The main change is to avoid setting the process state as running when
debugging core/minidumps (details in the bug).  Also included a few small,
related fixes around how the errors propagate in this case.

Fixed the FreeBSD/Windows break: the intention was to keep
Process::WillResume() and Process::DoResume() "in-sync", but this had the
unfortunate consequence of breaking Process sub-classes which don't override
WillResume().

The safer approach is to keep Process::WillResume() untouched and only
override it in the minidump and core implementations.

patch by lemo

Bug: https://bugs.llvm.org/show_bug.cgi?id=34532

Differential Revision: https://reviews.llvm.org/D37651

Modified:
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
    lldb/trunk/source/Commands/CommandObjectThread.cpp
    lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
    lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py?rev=313655&r1=313654&r2=313655&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/TestLinuxCore.py Tue Sep 19 11:07:33 2017
@@ -244,6 +244,34 @@ class LinuxCoreTestCase(TestBase):
             end_region.GetRegionBase())
         self.assertEqual(end_region.GetRegionEnd(), lldb.LLDB_INVALID_ADDRESS)
 
+    def check_state(self, process):
+        with open(os.devnull) as devnul:
+            # sanitize test output
+            self.dbg.SetOutputFileHandle(devnul, False)
+            self.dbg.SetErrorFileHandle(devnul, False)
+
+            self.assertTrue(process.is_stopped)
+
+            # Process.Continue
+            error = process.Continue()
+            self.assertFalse(error.Success())
+            self.assertTrue(process.is_stopped)
+
+            # Thread.StepOut
+            thread = process.GetSelectedThread()
+            thread.StepOut()
+            self.assertTrue(process.is_stopped)
+
+            # command line
+            self.dbg.HandleCommand('s')
+            self.assertTrue(process.is_stopped)
+            self.dbg.HandleCommand('c')
+            self.assertTrue(process.is_stopped)
+
+            # restore file handles
+            self.dbg.SetOutputFileHandle(None, False)
+            self.dbg.SetErrorFileHandle(None, False)
+
     def do_test(self, filename, pid, region_count):
         target = self.dbg.CreateTarget(filename + ".out")
         process = target.LoadCore(filename + ".core")
@@ -251,6 +279,8 @@ class LinuxCoreTestCase(TestBase):
         self.assertEqual(process.GetNumThreads(), 1)
         self.assertEqual(process.GetProcessID(), pid)
 
+        self.check_state(process)
+
         thread = process.GetSelectedThread()
         self.assertTrue(thread)
         self.assertEqual(thread.GetThreadID(), pid)

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py?rev=313655&r1=313654&r2=313655&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py Tue Sep 19 11:07:33 2017
@@ -31,6 +31,34 @@ class MiniDumpNewTestCase(TestBase):
         lldb.DBG.SetSelectedPlatform(self._initial_platform)
         super(MiniDumpNewTestCase, self).tearDown()
 
+    def check_state(self):
+        with open(os.devnull) as devnul:
+            # sanitize test output
+            self.dbg.SetOutputFileHandle(devnul, False)
+            self.dbg.SetErrorFileHandle(devnul, False)
+
+            self.assertTrue(self.process.is_stopped)
+
+            # Process.Continue
+            error = self.process.Continue()
+            self.assertFalse(error.Success())
+            self.assertTrue(self.process.is_stopped)
+
+            # Thread.StepOut
+            thread = self.process.GetSelectedThread()
+            thread.StepOut()
+            self.assertTrue(self.process.is_stopped)
+
+            # command line
+            self.dbg.HandleCommand('s')
+            self.assertTrue(self.process.is_stopped)
+            self.dbg.HandleCommand('c')
+            self.assertTrue(self.process.is_stopped)
+
+            # restore file handles
+            self.dbg.SetOutputFileHandle(None, False)
+            self.dbg.SetErrorFileHandle(None, False)
+
     def test_process_info_in_minidump(self):
         """Test that lldb can read the process information from the Minidump."""
         # target create -c linux-x86_64.dmp
@@ -40,6 +68,7 @@ class MiniDumpNewTestCase(TestBase):
         self.assertTrue(self.process, PROCESS_IS_VALID)
         self.assertEqual(self.process.GetNumThreads(), 1)
         self.assertEqual(self.process.GetProcessID(), self._linux_x86_64_pid)
+        self.check_state()
 
     def test_thread_info_in_minidump(self):
         """Test that lldb can read the thread information from the Minidump."""
@@ -47,6 +76,7 @@ class MiniDumpNewTestCase(TestBase):
         self.dbg.CreateTarget(None)
         self.target = self.dbg.GetSelectedTarget()
         self.process = self.target.LoadCore("linux-x86_64.dmp")
+        self.check_state()
         # This process crashed due to a segmentation fault in its
         # one and only thread.
         self.assertEqual(self.process.GetNumThreads(), 1)
@@ -61,6 +91,7 @@ class MiniDumpNewTestCase(TestBase):
         self.dbg.CreateTarget("linux-x86_64")
         self.target = self.dbg.GetSelectedTarget()
         self.process = self.target.LoadCore("linux-x86_64.dmp")
+        self.check_state()
         self.assertEqual(self.process.GetNumThreads(), 1)
         self.assertEqual(self.process.GetProcessID(), self._linux_x86_64_pid)
         thread = self.process.GetThreadAtIndex(0)
@@ -81,6 +112,7 @@ class MiniDumpNewTestCase(TestBase):
         self.dbg.CreateTarget(None)
         self.target = self.dbg.GetSelectedTarget()
         self.process = self.target.LoadCore("linux-x86_64_not_crashed.dmp")
+        self.check_state()
         self.assertEqual(self.process.GetNumThreads(), 1)
         thread = self.process.GetThreadAtIndex(0)
         self.assertEqual(thread.GetStopReason(), lldb.eStopReasonNone)
@@ -161,9 +193,10 @@ class MiniDumpNewTestCase(TestBase):
         """Test that we can examine local variables in a Minidump."""
         # Launch with the Minidump, and inspect a local variable.
         # target create linux-x86_64_not_crashed -c linux-x86_64_not_crashed.dmp
-        target = self.dbg.CreateTarget("linux-x86_64_not_crashed")
-        process = target.LoadCore("linux-x86_64_not_crashed.dmp")
-        thread = process.GetThreadAtIndex(0)
+        self.target = self.dbg.CreateTarget("linux-x86_64_not_crashed")
+        self.process = self.target.LoadCore("linux-x86_64_not_crashed.dmp")
+        self.check_state()
+        thread = self.process.GetThreadAtIndex(0)
         frame = thread.GetFrameAtIndex(1)
         value = frame.EvaluateExpression('x')
         self.assertEqual(value.GetValueAsSigned(), 3)

Modified: lldb/trunk/source/Commands/CommandObjectThread.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectThread.cpp?rev=313655&r1=313654&r2=313655&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandObjectThread.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectThread.cpp Tue Sep 19 11:07:33 2017
@@ -94,7 +94,7 @@ public:
     bool all_threads = false;
     if (command.GetArgumentCount() == 0) {
       Thread *thread = m_exe_ctx.GetThreadPtr();
-      if (!HandleOneThread(thread->GetID(), result))
+      if (!thread || !HandleOneThread(thread->GetID(), result))
         return false;
       return result.Succeeded();
     } else if (command.GetArgumentCount() == 1) {
@@ -775,6 +775,12 @@ protected:
       else
         error = process->Resume();
 
+      if (!error.Success()) {
+        result.AppendMessage(error.AsCString());
+        result.SetStatus(eReturnStatusFailed);
+        return false;
+      }
+
       // There is a race condition where this thread will return up the call
       // stack to the main command handler
       // and show an (lldb) prompt before HandlePrivateEvent (from

Modified: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h?rev=313655&r1=313654&r2=313655&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h (original)
+++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h Tue Sep 19 11:07:33 2017
@@ -84,11 +84,21 @@ public:
 
   void RefreshStateAfterStop() override;
 
+  lldb_private::Status WillResume() override {
+    lldb_private::Status error;
+    error.SetErrorStringWithFormat(
+        "error: %s does not support resuming processes",
+        GetPluginName().GetCString());
+    return error;
+  }
+
   //------------------------------------------------------------------
   // Process Queries
   //------------------------------------------------------------------
   bool IsAlive() override;
 
+  bool WarnBeforeDetach() const override { return false; }
+
   //------------------------------------------------------------------
   // Process Memory
   //------------------------------------------------------------------

Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h?rev=313655&r1=313654&r2=313655&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h Tue Sep 19 11:07:33 2017
@@ -82,6 +82,14 @@ public:
 
   bool GetProcessInfo(ProcessInstanceInfo &info) override;
 
+  Status WillResume() override {
+    Status error;
+    error.SetErrorStringWithFormat(
+        "error: %s does not support resuming processes",
+        GetPluginName().GetCString());
+    return error;
+  }
+
   MinidumpParser m_minidump_parser;
 
 protected:

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=313655&r1=313654&r2=313655&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Tue Sep 19 11:07:33 2017
@@ -1621,7 +1621,12 @@ Status Process::Resume() {
       log->Printf("Process::Resume: -- TrySetRunning failed, not resuming.");
     return error;
   }
-  return PrivateResume();
+  Status error = PrivateResume();
+  if (!error.Success()) {
+    // Undo running state change
+    m_public_run_lock.SetStopped();
+  }
+  return error;
 }
 
 Status Process::ResumeSynchronous(Stream *stream) {
@@ -1650,6 +1655,9 @@ Status Process::ResumeSynchronous(Stream
       error.SetErrorStringWithFormat(
           "process not in stopped state after synchronous resume: %s",
           StateAsCString(state));
+  } else {
+    // Undo running state change
+    m_public_run_lock.SetStopped();
   }
 
   // Undo the hijacking of process events...




More information about the lldb-commits mailing list