dvlahovski created this revision. dvlahovski added a reviewer: labath. dvlahovski added subscribers: beanz, lldb-commits.
One of the tests was flaky, because similarly to https://reviews.llvm.org/D18697 (https://reviews.llvm.org/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. https://reviews.llvm.org/D26193 Files: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py source/Plugins/Process/minidump/ProcessMinidump.cpp source/Plugins/Process/minidump/ProcessMinidump.h
Index: source/Plugins/Process/minidump/ProcessMinidump.h =================================================================== --- source/Plugins/Process/minidump/ProcessMinidump.h +++ source/Plugins/Process/minidump/ProcessMinidump.h @@ -46,8 +46,7 @@ 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 @@ Error GetMemoryRegionInfo(lldb::addr_t load_addr, MemoryRegionInfo &range_info) override; + bool GetProcessInfo(ProcessInstanceInfo &info) override; + MinidumpParser m_minidump_parser; protected: Index: source/Plugins/Process/minidump/ProcessMinidump.cpp =================================================================== --- source/Plugins/Process/minidump/ProcessMinidump.cpp +++ source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -180,14 +180,14 @@ 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 @@ 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 @@ 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 @@ 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; +} Index: packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py =================================================================== --- packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py +++ packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py @@ -5,6 +5,7 @@ from __future__ import print_function from six import iteritems +import shutil import lldb from lldbsuite.test.decorators import * @@ -18,15 +19,19 @@ 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 self.dbg.CreateTarget(None) self.target = self.dbg.GetSelectedTarget() 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 @@ 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 @@ 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,22 +79,75 @@ 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): frame = thread.GetFrameAtIndex(index) self.assertTrue(frame.IsValid()) 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.
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits