[Lldb-commits] [lldb] r285698 - Minidump plugin: Fix flaky test

Dimitar Vlahovski via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 1 08:48:25 PDT 2016


Author: dvlahovski
Date: Tue Nov  1 10:48:24 2016
New Revision: 285698

URL: http://llvm.org/viewvc/llvm-project?rev=285698&view=rev
Log:
Minidump plugin: Fix flaky test

Summary:
One of the tests was flaky, because similarly to
https://reviews.llvm.org/D18697 (rL265391) - if there is a process running
which is with the same PID as in the core file, the minidump
core file debugging will fail, because we get some information from the
running process.
The fix is routing the ProcessInfo requests through the Process class
and overriding it in ProcessMinidump to return correct data.

Reviewers: labath

Subscribers: lldb-commits, beanz

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

Modified:
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
    lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
    lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h

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=285698&r1=285697&r2=285698&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 Nov  1 10:48:24 2016
@@ -5,6 +5,7 @@ Test basics of Minidump debugging.
 from __future__ import print_function
 from six import iteritems
 
+import shutil
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -18,6 +19,10 @@ class MiniDumpNewTestCase(TestBase):
 
     NO_DEBUG_INFO_TESTCASE = True
 
+    _linux_x86_64_pid = 29917
+    _linux_x86_64_not_crashed_pid = 29939
+    _linux_x86_64_not_crashed_pid_offset = 0xD967
+
     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
@@ -26,7 +31,7 @@ class MiniDumpNewTestCase(TestBase):
         self.process = self.target.LoadCore("linux-x86_64.dmp")
         self.assertTrue(self.process, PROCESS_IS_VALID)
         self.assertEqual(self.process.GetNumThreads(), 1)
-        self.assertEqual(self.process.GetProcessID(), 29917)
+        self.assertEqual(self.process.GetProcessID(), self._linux_x86_64_pid)
 
     def test_thread_info_in_minidump(self):
         """Test that lldb can read the thread information from the Minidump."""
@@ -49,6 +54,7 @@ class MiniDumpNewTestCase(TestBase):
         self.target = self.dbg.GetSelectedTarget()
         self.process = self.target.LoadCore("linux-x86_64.dmp")
         self.assertEqual(self.process.GetNumThreads(), 1)
+        self.assertEqual(self.process.GetProcessID(), self._linux_x86_64_pid)
         thread = self.process.GetThreadAtIndex(0)
         # frame #0: linux-x86_64`crash()
         # frame #1: linux-x86_64`_start
@@ -61,7 +67,8 @@ class MiniDumpNewTestCase(TestBase):
         self.assertEqual(pc, eip.GetValueAsUnsigned())
 
     def test_snapshot_minidump(self):
-        """Test that if we load a snapshot minidump file (meaning the process did not crash) there is no stop reason."""
+        """Test that if we load a snapshot minidump file (meaning the process
+        did not crash) there is no stop reason."""
         # target create -c linux-x86_64_not_crashed.dmp
         self.dbg.CreateTarget(None)
         self.target = self.dbg.GetSelectedTarget()
@@ -72,14 +79,13 @@ class MiniDumpNewTestCase(TestBase):
         stop_description = thread.GetStopDescription(256)
         self.assertEqual(stop_description, None)
 
-    def test_deeper_stack_in_minidump(self):
-        """Test that we can examine a more interesting stack in a Minidump."""
-        # Launch with the Minidump, and inspect the stack.
-        # 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")
+    def do_test_deeper_stack(self, binary, core, pid):
+        target = self.dbg.CreateTarget(binary)
+        process = target.LoadCore(core)
         thread = process.GetThreadAtIndex(0)
 
+        self.assertEqual(process.GetProcessID(), pid)
+
         expected_stack = {1: 'bar', 2: 'foo', 3: '_start'}
         self.assertGreaterEqual(thread.GetNumFrames(), len(expected_stack))
         for index, name in iteritems(expected_stack):
@@ -88,6 +94,60 @@ class MiniDumpNewTestCase(TestBase):
             function_name = frame.GetFunctionName()
             self.assertTrue(name in function_name)
 
+    def test_deeper_stack_in_minidump(self):
+        """Test that we can examine a more interesting stack in a Minidump."""
+        # Launch with the Minidump, and inspect the stack.
+        # target create linux-x86_64_not_crashed -c linux-x86_64_not_crashed.dmp
+        self.do_test_deeper_stack("linux-x86_64_not_crashed",
+                                  "linux-x86_64_not_crashed.dmp",
+                                  self._linux_x86_64_not_crashed_pid)
+
+    def do_change_pid_in_minidump(self, core, newcore, offset, oldpid, newpid):
+        """ This assumes that the minidump is breakpad generated on Linux -
+        meaning that the PID in the file will be an ascii string part of
+        /proc/PID/status which is written in the file
+        """
+        shutil.copyfile(core, newcore)
+        with open(newcore, "r+") as f:
+            f.seek(offset)
+            self.assertEqual(f.read(5), oldpid)
+
+            f.seek(offset)
+            if len(newpid) < len(oldpid):
+                newpid += " " * (len(oldpid) - len(newpid))
+            newpid += "\n"
+            f.write(newpid)
+
+    def test_deeper_stack_in_minidump_with_same_pid_running(self):
+        """Test that we read the information from the core correctly even if we
+        have a running process with the same PID"""
+        try:
+            self.do_change_pid_in_minidump("linux-x86_64_not_crashed.dmp",
+                                           "linux-x86_64_not_crashed-pid.dmp",
+                                           self._linux_x86_64_not_crashed_pid_offset,
+                                           str(self._linux_x86_64_not_crashed_pid),
+                                           str(os.getpid()))
+            self.do_test_deeper_stack("linux-x86_64_not_crashed",
+                                      "linux-x86_64_not_crashed-pid.dmp",
+                                      os.getpid())
+        finally:
+            self.RemoveTempFile("linux-x86_64_not_crashed-pid.dmp")
+
+    def test_two_cores_same_pid(self):
+        """Test that we handle the situation if we have two core files with the same PID """
+        try:
+            self.do_change_pid_in_minidump("linux-x86_64_not_crashed.dmp",
+                                           "linux-x86_64_not_crashed-pid.dmp",
+                                           self._linux_x86_64_not_crashed_pid_offset,
+                                           str(self._linux_x86_64_not_crashed_pid),
+                                           str(self._linux_x86_64_pid))
+            self.do_test_deeper_stack("linux-x86_64_not_crashed",
+                                      "linux-x86_64_not_crashed-pid.dmp",
+                                      self._linux_x86_64_pid)
+            self.test_stack_info_in_minidump()
+        finally:
+            self.RemoveTempFile("linux-x86_64_not_crashed-pid.dmp")
+
     def test_local_variables_in_minidump(self):
         """Test that we can examine local variables in a Minidump."""
         # Launch with the Minidump, and inspect a local variable.

Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=285698&r1=285697&r2=285698&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Tue Nov  1 10:48:24 2016
@@ -180,14 +180,14 @@ bool ProcessMinidump::IsAlive() { return
 bool ProcessMinidump::WarnBeforeDetach() const { return false; }
 
 size_t ProcessMinidump::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
-                                   lldb_private::Error &error) {
+                                   Error &error) {
   // Don't allow the caching that lldb_private::Process::ReadMemory does
   // since we have it all cached in our dump file anyway.
   return DoReadMemory(addr, buf, size, error);
 }
 
 size_t ProcessMinidump::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
-                                     lldb_private::Error &error) {
+                                     Error &error) {
 
   llvm::ArrayRef<uint8_t> mem = m_minidump_parser.GetMemory(addr, size);
   if (mem.empty()) {
@@ -211,8 +211,8 @@ ArchSpec ProcessMinidump::GetArchitectur
   return ArchSpec(triple);
 }
 
-Error ProcessMinidump::GetMemoryRegionInfo(
-    lldb::addr_t load_addr, lldb_private::MemoryRegionInfo &range_info) {
+Error ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
+                                           MemoryRegionInfo &range_info) {
   Error error;
   auto info = m_minidump_parser.GetMemoryRegionInfo(load_addr);
   if (!info) {
@@ -225,9 +225,8 @@ Error ProcessMinidump::GetMemoryRegionIn
 
 void ProcessMinidump::Clear() { Process::m_thread_list.Clear(); }
 
-bool ProcessMinidump::UpdateThreadList(
-    lldb_private::ThreadList &old_thread_list,
-    lldb_private::ThreadList &new_thread_list) {
+bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list,
+                                       ThreadList &new_thread_list) {
   uint32_t num_threads = 0;
   if (m_thread_list.size() > 0)
     num_threads = m_thread_list.size();
@@ -291,3 +290,16 @@ void ProcessMinidump::ReadModuleList() {
                               load_addr_changed);
   }
 }
+
+bool ProcessMinidump::GetProcessInfo(ProcessInstanceInfo &info) {
+  info.Clear();
+  info.SetProcessID(GetID());
+  info.SetArchitecture(GetArchitecture());
+  lldb::ModuleSP module_sp = GetTarget().GetExecutableModule();
+  if (module_sp) {
+    const bool add_exe_file_as_first_arg = false;
+    info.SetExecutableFile(GetTarget().GetExecutableModule()->GetFileSpec(),
+                           add_exe_file_as_first_arg);
+  }
+  return true;
+}

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=285698&r1=285697&r2=285698&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h Tue Nov  1 10:48:24 2016
@@ -46,8 +46,7 @@ public:
   static const char *GetPluginDescriptionStatic();
 
   ProcessMinidump(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-                  const lldb_private::FileSpec &core_file,
-                  MinidumpParser minidump_parser);
+                  const FileSpec &core_file, MinidumpParser minidump_parser);
 
   ~ProcessMinidump() override;
 
@@ -81,6 +80,8 @@ public:
   Error GetMemoryRegionInfo(lldb::addr_t load_addr,
                             MemoryRegionInfo &range_info) override;
 
+  bool GetProcessInfo(ProcessInstanceInfo &info) override;
+
   MinidumpParser m_minidump_parser;
 
 protected:




More information about the lldb-commits mailing list