From 1a2ac9bda573919f52123d93baf91501023b5ba8 Mon Sep 17 00:00:00 2001 From: Howard Hellyer Date: Thu, 24 Nov 2016 08:56:37 +0000 Subject: [PATCH] =?utf8?q?Patch=20for=20lldb=20bug=2026322=20=E2=80=9Ccore?= =?utf8?q?=20load=20hangs=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Summary: This patch changes the way ProcessElfCore.cpp handles signal information. The patch changes ProcessElfCore.cpp to use the signal from si_signo in SIGINFO notes in preference to the value of cursig in PRSTATUS notes. The value from SIGINFO seems to be more thread specific. The value from PRSTATUS is usually the same for all threads even if only one thread received a signal. If it cannot find any SIGINFO blocks it reverts to the old behaviour and uses the value from cursig in PRSTATUS. If after that no thread appears to have been stopped it forces the status of the first thread to be SIGSTOP to prevent lldb hanging waiting for any thread from the core file to change state. The order is: - If one or more threads have a non-zero si_signo in SIGINFO that will be used. - If no threads had a SIGINFO block with a non-zero si_signo set all threads signals to the value in cursig in their PRSTATUS notes. - If no thread has a signal set to a non-zero value set the signal for only the first thread to SIGSTOP. This resolves two issues. The first was identified in bug 26322, the second became apparent while investigating this problem and looking at the signal values reported for each thread via “thread list”. Firstly lldb is able to load core dumps generated by gcore where each thread has a SIGINFO note containing a signal number but cursig in the PRSTATUS block for each thread is 0. Secondly if a SIGINFO note was found the “thread list” command will no longer show the same signal number for all threads. At the moment if a process crashes, for example with SIGILL, all threads will show “stop reason = signal SIGILL”. With this patch only the thread that executed the illegal instruction shows that stop reason. The other threads show “stop reason = signal 0”. Reviewers: jingham, clayborg Subscribers: sas, labath, lldb-commits Differential Revision: https://reviews.llvm.org/D26676 llvm-svn: 287858 --- .../postmortem/elf-core/gcore/TestGCore.py | 52 ++++++++++++++++++ .../postmortem/elf-core/gcore/linux-i386.core | 0 .../postmortem/elf-core/gcore/linux-x86_64.core | 0 .../postmortem/elf-core/gcore/main.cpp | 63 +++++++++++++++++++++ .../postmortem/elf-core/gcore/main.mk | 5 ++ .../postmortem/elf-core/gcore/make-core.sh | 56 +++++++++++++++++++ .../elf-core/thread_crash/TestLinuxCoreThreads.py | 61 +++++++++++++++++++++ .../elf-core/thread_crash/linux-i386.core | 0 .../elf-core/thread_crash/linux-x86_64.core | 0 .../postmortem/elf-core/thread_crash/main.cpp | 63 +++++++++++++++++++++ .../postmortem/elf-core/thread_crash/main.mk | 5 ++ .../postmortem/elf-core/thread_crash/make-core.sh | 64 ++++++++++++++++++++++ .../Plugins/Process/elf-core/ProcessElfCore.cpp | 33 ++++++++++- .../Plugins/Process/elf-core/ThreadElfCore.cpp | 42 ++++++++++++++ .../Plugins/Process/elf-core/ThreadElfCore.h | 35 +++++++++++- 15 files changed, 477 insertions(+), 2 deletions(-) create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/TestGCore.py create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-i386.core create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-x86_64.core create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.cpp create mode 100755 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.mk create mode 100755 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/make-core.sh create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-i386.core create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-x86_64.core create mode 100644 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.cpp create mode 100755 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.mk create mode 100755 lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/make-core.sh diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/TestGCore.py b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/TestGCore.py new file mode 100644 index 0000000..5a11a52 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/TestGCore.py @@ -0,0 +1,52 @@ +""" +Test signal reporting when debugging with linux core files. +""" + +from __future__ import print_function + +import shutil +import struct + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class GCoreTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + mydir = TestBase.compute_mydir(__file__) + _initial_platform = lldb.DBG.GetSelectedPlatform() + + _i386_pid = 5586 + _x86_64_pid = 5669 + + @skipIf(oslist=['windows']) + @skipIf(triple='^mips') + def test_i386(self): + """Test that lldb can read the process information from an i386 linux core file.""" + self.do_test("linux-i386", self._i386_pid) + + @skipIf(oslist=['windows']) + @skipIf(triple='^mips') + def test_x86_64(self): + """Test that lldb can read the process information from an x86_64 linux core file.""" + self.do_test("linux-x86_64", self._x86_64_pid) + + def do_test(self, filename, pid): + target = self.dbg.CreateTarget("") + process = target.LoadCore(filename + ".core") + self.assertTrue(process, PROCESS_IS_VALID) + self.assertEqual(process.GetNumThreads(), 3) + self.assertEqual(process.GetProcessID(), pid) + + for thread in process: + reason = thread.GetStopReason() + self.assertEqual(reason, lldb.eStopReasonSignal) + signal = thread.GetStopReasonDataAtIndex(1) + # Check we got signal 19 (SIGSTOP) + self.assertEqual(signal, 19) + + self.dbg.DeleteTarget(target) + lldb.DBG.SetSelectedPlatform(self._initial_platform) diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-i386.core b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-i386.core new file mode 100644 index 0000000..e69de29 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-x86_64.core b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-x86_64.core new file mode 100644 index 0000000..e69de29 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.cpp new file mode 100644 index 0000000..9908faf --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.cpp @@ -0,0 +1,63 @@ +//===-- main.cpp ------------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +// This test verifies the correct handling of child thread exits. + +#include +#include +#include + +pseudo_barrier_t g_barrier1; +pseudo_barrier_t g_barrier2; + +void * +thread1 () +{ + // Synchronize with the main thread. + pseudo_barrier_wait(g_barrier1); + + // Synchronize with the main thread and thread2. + pseudo_barrier_wait(g_barrier2); + + // Return + return NULL; +} + +void * +thread2 () +{ + + // Synchronize with thread1 and the main thread. + pseudo_barrier_wait(g_barrier2); // Should not reach here. + + // Return + return NULL; +} + +int main () +{ + + pseudo_barrier_init(g_barrier1, 2); + pseudo_barrier_init(g_barrier2, 3); + + // Create a thread. + std::thread thread_1(thread1); + + // Wait for thread1 to start. + pseudo_barrier_wait(g_barrier1); + + // Wait for thread1 to start. + std::thread thread_2(thread2); + + // Thread 2 is waiting for another thread to reach the barrier. + // This should have for ever. (So we can run gcore against this process.) + thread_2.join(); + + return 0; +} diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.mk b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.mk new file mode 100755 index 0000000..ff874a2 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/main.mk @@ -0,0 +1,5 @@ +LEVEL = ../../../../make + +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES +include $(LEVEL)/Makefile.rules diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/make-core.sh b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/make-core.sh new file mode 100755 index 0000000..b6979c7 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/make-core.sh @@ -0,0 +1,56 @@ +#! /bin/sh + +linux_check_ptrace_scope() +{ + if grep -q '1' . +EOF + +stack_size=`ulimit -s` + +# Decrease stack size to 16k => smaller core files. +# gcore won't run with the smaller stack +ulimit -Ss 16 + +core_dump_filter=`cat /proc/self/coredump_filter` +echo 0 > /proc/self/coredump_filter + +./a.out & + +pid=$! + +echo $core_dump_filter > /proc/self/coredump_filter + +# Reset stack size as so there's enough space to run gcore. +ulimit -s $stack_size + +echo "Sleeping for 5 seconds to wait for $pid" + +sleep 5 +echo "Taking core from process $pid" + +gcore -o core $pid + +echo "Killing process $pid" +kill -9 $pid diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py new file mode 100644 index 0000000..7cc3c07 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/TestLinuxCoreThreads.py @@ -0,0 +1,61 @@ +""" +Test signal reporting when debugging with linux core files. +""" + +from __future__ import print_function + +import shutil +import struct + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class LinuxCoreThreadsTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + mydir = TestBase.compute_mydir(__file__) + _initial_platform = lldb.DBG.GetSelectedPlatform() + + _i386_pid = 5193 + _x86_64_pid = 5222 + + # Thread id for the failing thread. + _i386_tid = 5195 + _x86_64_tid = 5250 + + @skipIf(oslist=['windows']) + @skipIf(triple='^mips') + def test_i386(self): + """Test that lldb can read the process information from an i386 linux core file.""" + self.do_test("linux-i386", self._i386_pid, self._i386_tid) + + @skipIf(oslist=['windows']) + @skipIf(triple='^mips') + def test_x86_64(self): + """Test that lldb can read the process information from an x86_64 linux core file.""" + self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_tid) + + def do_test(self, filename, pid, tid): + target = self.dbg.CreateTarget("") + process = target.LoadCore(filename + ".core") + self.assertTrue(process, PROCESS_IS_VALID) + self.assertEqual(process.GetNumThreads(), 3) + self.assertEqual(process.GetProcessID(), pid) + + for thread in process: + reason = thread.GetStopReason() + if( thread.GetThreadID() == tid ): + self.assertEqual(reason, lldb.eStopReasonSignal) + signal = thread.GetStopReasonDataAtIndex(1) + # Check we got signal 4 (SIGILL) + self.assertEqual(signal, 4) + else: + signal = thread.GetStopReasonDataAtIndex(1) + # Check we got no signal on the other threads + self.assertEqual(signal, 0) + + self.dbg.DeleteTarget(target) + lldb.DBG.SetSelectedPlatform(self._initial_platform) diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-i386.core b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-i386.core new file mode 100644 index 0000000..e69de29 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-x86_64.core b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-x86_64.core new file mode 100644 index 0000000..e69de29 diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.cpp b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.cpp new file mode 100644 index 0000000..826d9d3 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.cpp @@ -0,0 +1,63 @@ +//===-- main.cpp ------------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +// This test verifies the correct handling of child thread exits. + +#include +#include +#include + +pseudo_barrier_t g_barrier1; +pseudo_barrier_t g_barrier2; + +void * +thread1 () +{ + // Synchronize with the main thread. + pseudo_barrier_wait(g_barrier1); + + // Synchronize with the main thread and thread2. + pseudo_barrier_wait(g_barrier2); + + // Return + return NULL; // Should not reach here. (thread2 should raise SIGILL) +} + +void * +thread2 () +{ + raise(SIGILL); // Raise SIGILL + + // Synchronize with thread1 and the main thread. + pseudo_barrier_wait(g_barrier2); // Should not reach here. + + // Return + return NULL; +} + +int main () +{ + pseudo_barrier_init(g_barrier1, 2); + pseudo_barrier_init(g_barrier2, 3); + + // Create a thread. + std::thread thread_1(thread1); + + // Wait for thread1 to start. + pseudo_barrier_wait(g_barrier1); + + // Create another thread. + std::thread thread_2(thread2); + + // Wait for thread2 to start. + // Second thread should crash but first thread and main thread may reach here. + pseudo_barrier_wait(g_barrier2); + + return 0; +} diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.mk b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.mk new file mode 100755 index 0000000..ff874a2 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/main.mk @@ -0,0 +1,5 @@ +LEVEL = ../../../../make + +CXX_SOURCES := main.cpp +ENABLE_THREADS := YES +include $(LEVEL)/Makefile.rules diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/make-core.sh b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/make-core.sh new file mode 100755 index 0000000..ea263c8 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/make-core.sh @@ -0,0 +1,64 @@ +#! /bin/sh + +linux_check_core_pattern() +{ + if grep -q '^|' &2 + exit 1 + ;; +esac + +set -e -x + +if [ "$OS" = Linux ]; then + linux_check_core_pattern +fi + +ulimit -c 1000 +real_limit=$(ulimit -c) +if [ $real_limit -lt 100 ]; then + cat < smaller core files. + +core_dump_filter=`cat /proc/self/coredump_filter` +echo 0 > /proc/self/coredump_filter + +exec ./a.out + +# Reset stack size and core_dump_filter +echo core_dump_filter > /proc/self/coredump_filter +ulimit -s $stack_size diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index f03f8db..6ac308f 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -214,6 +214,29 @@ Error ProcessElfCore::DoLoadCore() { SetUnixSignals(UnixSignals::Create(GetArchitecture())); + // Ensure we found at least one thread that was stopped on a signal. + bool siginfo_signal_found = false; + bool prstatus_signal_found = false; + // Check we found a signal in a SIGINFO note. + for (const auto &thread_data: m_thread_data) { + if (thread_data.signo != 0) + siginfo_signal_found = true; + if (thread_data.prstatus_sig != 0) + prstatus_signal_found = true; + } + if (!siginfo_signal_found) { + // If we don't have signal from SIGINFO use the signal from each threads + // PRSTATUS note. + if (prstatus_signal_found) { + for (auto &thread_data: m_thread_data) + thread_data.signo = thread_data.prstatus_sig; + } else if (m_thread_data.size() > 0) { + // If all else fails force the first thread to be SIGSTOP + m_thread_data.begin()->signo = + GetUnixSignals()->GetSignalNumberFromName("SIGSTOP"); + } + } + // Core files are useless without the main executable. See if we can locate // the main // executable using data we found in the core file notes. @@ -402,6 +425,7 @@ enum { NT_AUXV, NT_FILE = 0x46494c45, NT_PRXFPREG = 0x46e62b7f, + NT_SIGINFO = 0x53494749, }; namespace FREEBSD { @@ -485,6 +509,7 @@ Error ProcessElfCore::ParseThreadContextsFromNoteSegment( ArchSpec arch = GetArchitecture(); ELFLinuxPrPsInfo prpsinfo; ELFLinuxPrStatus prstatus; + ELFLinuxSigInfo siginfo; size_t header_size; size_t len; Error error; @@ -546,7 +571,7 @@ Error ProcessElfCore::ParseThreadContextsFromNoteSegment( error = prstatus.Parse(note_data, arch); if (error.Fail()) return error; - thread_data->signo = prstatus.pr_cursig; + thread_data->prstatus_sig = prstatus.pr_cursig; thread_data->tid = prstatus.pr_pid; header_size = ELFLinuxPrStatus::GetSize(arch); len = note_data.GetByteSize() - header_size; @@ -588,6 +613,12 @@ Error ProcessElfCore::ParseThreadContextsFromNoteSegment( m_nt_file_entries[i].path.SetCString(path); } } break; + case NT_SIGINFO: { + error = siginfo.Parse(note_data, arch); + if (error.Fail()) + return error; + thread_data->signo = siginfo.si_signo; + } break; default: break; } diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp index e40ec17..fd545cd 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp @@ -320,3 +320,45 @@ Error ELFLinuxPrPsInfo::Parse(DataExtractor &data, ArchSpec &arch) { return error; } + +//---------------------------------------------------------------- +// Parse SIGINFO from NOTE entry +//---------------------------------------------------------------- +ELFLinuxSigInfo::ELFLinuxSigInfo() { + memset(this, 0, sizeof(ELFLinuxSigInfo)); +} + +Error ELFLinuxSigInfo::Parse(DataExtractor &data, const ArchSpec &arch) { + Error error; + ByteOrder byteorder = data.GetByteOrder(); + if (GetSize(arch) > data.GetByteSize()) { + error.SetErrorStringWithFormat( + "NT_SIGINFO size should be %zu, but the remaining bytes are: %" PRIu64, + GetSize(arch), data.GetByteSize()); + return error; + } + + switch (arch.GetCore()) { + case ArchSpec::eCore_x86_64_x86_64: + data.ExtractBytes(0, sizeof(ELFLinuxPrStatus), byteorder, this); + break; + case ArchSpec::eCore_s390x_generic: + case ArchSpec::eCore_x86_32_i386: + case ArchSpec::eCore_x86_32_i486: { + // Parsing from a 32 bit ELF core file, and populating/reusing the structure + // properly, because the struct is for the 64 bit version + offset_t offset = 0; + si_signo = data.GetU32(&offset); + si_code = data.GetU32(&offset); + si_errno = data.GetU32(&offset); + + break; + } + default: + error.SetErrorStringWithFormat("ELFLinuxSigInfo::%s Unknown architecture", + __FUNCTION__); + break; + } + + return error; +} diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h index c144516..1957ac2 100644 --- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h @@ -82,6 +82,38 @@ struct ELFLinuxPrStatus { static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus is not correct!"); +struct ELFLinuxSigInfo { + int32_t si_signo; + int32_t si_code; + int32_t si_errno; + + ELFLinuxSigInfo(); + + lldb_private::Error Parse(lldb_private::DataExtractor &data, + const lldb_private::ArchSpec &arch); + + // Return the bytesize of the structure + // 64 bit - just sizeof + // 32 bit - hardcoded because we are reusing the struct, but some of the + // members are smaller - + // so the layout is not the same + static size_t GetSize(const lldb_private::ArchSpec &arch) { + switch (arch.GetCore()) { + case lldb_private::ArchSpec::eCore_x86_64_x86_64: + return sizeof(ELFLinuxSigInfo); + case lldb_private::ArchSpec::eCore_s390x_generic: + case lldb_private::ArchSpec::eCore_x86_32_i386: + case lldb_private::ArchSpec::eCore_x86_32_i486: + return 12; + default: + return 0; + } + } +}; + +static_assert(sizeof(ELFLinuxSigInfo) == 12, + "sizeof ELFLinuxSigInfo is not correct!"); + // PRPSINFO structure's size differs based on architecture. // This is the layout in the x86-64 arch case. // In the i386 case we parse it manually and fill it again @@ -133,7 +165,8 @@ struct ThreadData { lldb_private::DataExtractor fpregset; lldb_private::DataExtractor vregset; lldb::tid_t tid; - int signo; + int signo = 0; + int prstatus_sig = 0; std::string name; }; -- 2.7.4