Skip to content

Commit 1a2ac9b

Browse files
committedNov 24, 2016
Patch for lldb bug 26322 “core load hangs”
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
1 parent 1c7f07a commit 1a2ac9b

File tree

15 files changed

+477
-2
lines changed

15 files changed

+477
-2
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
"""
2+
Test signal reporting when debugging with linux core files.
3+
"""
4+
5+
from __future__ import print_function
6+
7+
import shutil
8+
import struct
9+
10+
import lldb
11+
from lldbsuite.test.decorators import *
12+
from lldbsuite.test.lldbtest import *
13+
from lldbsuite.test import lldbutil
14+
15+
16+
class GCoreTestCase(TestBase):
17+
NO_DEBUG_INFO_TESTCASE = True
18+
19+
mydir = TestBase.compute_mydir(__file__)
20+
_initial_platform = lldb.DBG.GetSelectedPlatform()
21+
22+
_i386_pid = 5586
23+
_x86_64_pid = 5669
24+
25+
@skipIf(oslist=['windows'])
26+
@skipIf(triple='^mips')
27+
def test_i386(self):
28+
"""Test that lldb can read the process information from an i386 linux core file."""
29+
self.do_test("linux-i386", self._i386_pid)
30+
31+
@skipIf(oslist=['windows'])
32+
@skipIf(triple='^mips')
33+
def test_x86_64(self):
34+
"""Test that lldb can read the process information from an x86_64 linux core file."""
35+
self.do_test("linux-x86_64", self._x86_64_pid)
36+
37+
def do_test(self, filename, pid):
38+
target = self.dbg.CreateTarget("")
39+
process = target.LoadCore(filename + ".core")
40+
self.assertTrue(process, PROCESS_IS_VALID)
41+
self.assertEqual(process.GetNumThreads(), 3)
42+
self.assertEqual(process.GetProcessID(), pid)
43+
44+
for thread in process:
45+
reason = thread.GetStopReason()
46+
self.assertEqual(reason, lldb.eStopReasonSignal)
47+
signal = thread.GetStopReasonDataAtIndex(1)
48+
# Check we got signal 19 (SIGSTOP)
49+
self.assertEqual(signal, 19)
50+
51+
self.dbg.DeleteTarget(target)
52+
lldb.DBG.SetSelectedPlatform(self._initial_platform)

‎lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-i386.core

Whitespace-only changes.

‎lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/gcore/linux-x86_64.core

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//===-- main.cpp ------------------------------------------------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
// This test verifies the correct handling of child thread exits.
11+
12+
#include <atomic>
13+
#include <thread>
14+
#include <csignal>
15+
16+
pseudo_barrier_t g_barrier1;
17+
pseudo_barrier_t g_barrier2;
18+
19+
void *
20+
thread1 ()
21+
{
22+
// Synchronize with the main thread.
23+
pseudo_barrier_wait(g_barrier1);
24+
25+
// Synchronize with the main thread and thread2.
26+
pseudo_barrier_wait(g_barrier2);
27+
28+
// Return
29+
return NULL;
30+
}
31+
32+
void *
33+
thread2 ()
34+
{
35+
36+
// Synchronize with thread1 and the main thread.
37+
pseudo_barrier_wait(g_barrier2); // Should not reach here.
38+
39+
// Return
40+
return NULL;
41+
}
42+
43+
int main ()
44+
{
45+
46+
pseudo_barrier_init(g_barrier1, 2);
47+
pseudo_barrier_init(g_barrier2, 3);
48+
49+
// Create a thread.
50+
std::thread thread_1(thread1);
51+
52+
// Wait for thread1 to start.
53+
pseudo_barrier_wait(g_barrier1);
54+
55+
// Wait for thread1 to start.
56+
std::thread thread_2(thread2);
57+
58+
// Thread 2 is waiting for another thread to reach the barrier.
59+
// This should have for ever. (So we can run gcore against this process.)
60+
thread_2.join();
61+
62+
return 0;
63+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
LEVEL = ../../../../make
2+
3+
CXX_SOURCES := main.cpp
4+
ENABLE_THREADS := YES
5+
include $(LEVEL)/Makefile.rules
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#! /bin/sh
2+
3+
linux_check_ptrace_scope()
4+
{
5+
if grep -q '1' </proc/sys/kernel/yama/ptrace_scope; then
6+
cat <<EOF
7+
Your system prevents the use of PTRACE to attach to non-child processes. The core file
8+
cannot be generated. Please reset /proc/sys/kernel/yama/ptrace_scope to 0 (requires root
9+
privileges) to enable core generation via gcore.
10+
EOF
11+
exit 1
12+
fi
13+
}
14+
15+
set -e -x
16+
17+
OS=$(uname -s)
18+
if [ "$OS" = Linux ]; then
19+
linux_check_ptrace_scope
20+
fi
21+
22+
rm -f a.out
23+
make -f main.mk
24+
25+
cat <<EOF
26+
Executable file is in a.out.
27+
Core file will be saved as core.<pid>.
28+
EOF
29+
30+
stack_size=`ulimit -s`
31+
32+
# Decrease stack size to 16k => smaller core files.
33+
# gcore won't run with the smaller stack
34+
ulimit -Ss 16
35+
36+
core_dump_filter=`cat /proc/self/coredump_filter`
37+
echo 0 > /proc/self/coredump_filter
38+
39+
./a.out &
40+
41+
pid=$!
42+
43+
echo $core_dump_filter > /proc/self/coredump_filter
44+
45+
# Reset stack size as so there's enough space to run gcore.
46+
ulimit -s $stack_size
47+
48+
echo "Sleeping for 5 seconds to wait for $pid"
49+
50+
sleep 5
51+
echo "Taking core from process $pid"
52+
53+
gcore -o core $pid
54+
55+
echo "Killing process $pid"
56+
kill -9 $pid
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
"""
2+
Test signal reporting when debugging with linux core files.
3+
"""
4+
5+
from __future__ import print_function
6+
7+
import shutil
8+
import struct
9+
10+
import lldb
11+
from lldbsuite.test.decorators import *
12+
from lldbsuite.test.lldbtest import *
13+
from lldbsuite.test import lldbutil
14+
15+
16+
class LinuxCoreThreadsTestCase(TestBase):
17+
NO_DEBUG_INFO_TESTCASE = True
18+
19+
mydir = TestBase.compute_mydir(__file__)
20+
_initial_platform = lldb.DBG.GetSelectedPlatform()
21+
22+
_i386_pid = 5193
23+
_x86_64_pid = 5222
24+
25+
# Thread id for the failing thread.
26+
_i386_tid = 5195
27+
_x86_64_tid = 5250
28+
29+
@skipIf(oslist=['windows'])
30+
@skipIf(triple='^mips')
31+
def test_i386(self):
32+
"""Test that lldb can read the process information from an i386 linux core file."""
33+
self.do_test("linux-i386", self._i386_pid, self._i386_tid)
34+
35+
@skipIf(oslist=['windows'])
36+
@skipIf(triple='^mips')
37+
def test_x86_64(self):
38+
"""Test that lldb can read the process information from an x86_64 linux core file."""
39+
self.do_test("linux-x86_64", self._x86_64_pid, self._x86_64_tid)
40+
41+
def do_test(self, filename, pid, tid):
42+
target = self.dbg.CreateTarget("")
43+
process = target.LoadCore(filename + ".core")
44+
self.assertTrue(process, PROCESS_IS_VALID)
45+
self.assertEqual(process.GetNumThreads(), 3)
46+
self.assertEqual(process.GetProcessID(), pid)
47+
48+
for thread in process:
49+
reason = thread.GetStopReason()
50+
if( thread.GetThreadID() == tid ):
51+
self.assertEqual(reason, lldb.eStopReasonSignal)
52+
signal = thread.GetStopReasonDataAtIndex(1)
53+
# Check we got signal 4 (SIGILL)
54+
self.assertEqual(signal, 4)
55+
else:
56+
signal = thread.GetStopReasonDataAtIndex(1)
57+
# Check we got no signal on the other threads
58+
self.assertEqual(signal, 0)
59+
60+
self.dbg.DeleteTarget(target)
61+
lldb.DBG.SetSelectedPlatform(self._initial_platform)

‎lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-i386.core

Whitespace-only changes.

‎lldb/packages/Python/lldbsuite/test/functionalities/postmortem/elf-core/thread_crash/linux-x86_64.core

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//===-- main.cpp ------------------------------------------------*- C++ -*-===//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
10+
// This test verifies the correct handling of child thread exits.
11+
12+
#include <atomic>
13+
#include <thread>
14+
#include <csignal>
15+
16+
pseudo_barrier_t g_barrier1;
17+
pseudo_barrier_t g_barrier2;
18+
19+
void *
20+
thread1 ()
21+
{
22+
// Synchronize with the main thread.
23+
pseudo_barrier_wait(g_barrier1);
24+
25+
// Synchronize with the main thread and thread2.
26+
pseudo_barrier_wait(g_barrier2);
27+
28+
// Return
29+
return NULL; // Should not reach here. (thread2 should raise SIGILL)
30+
}
31+
32+
void *
33+
thread2 ()
34+
{
35+
raise(SIGILL); // Raise SIGILL
36+
37+
// Synchronize with thread1 and the main thread.
38+
pseudo_barrier_wait(g_barrier2); // Should not reach here.
39+
40+
// Return
41+
return NULL;
42+
}
43+
44+
int main ()
45+
{
46+
pseudo_barrier_init(g_barrier1, 2);
47+
pseudo_barrier_init(g_barrier2, 3);
48+
49+
// Create a thread.
50+
std::thread thread_1(thread1);
51+
52+
// Wait for thread1 to start.
53+
pseudo_barrier_wait(g_barrier1);
54+
55+
// Create another thread.
56+
std::thread thread_2(thread2);
57+
58+
// Wait for thread2 to start.
59+
// Second thread should crash but first thread and main thread may reach here.
60+
pseudo_barrier_wait(g_barrier2);
61+
62+
return 0;
63+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
LEVEL = ../../../../make
2+
3+
CXX_SOURCES := main.cpp
4+
ENABLE_THREADS := YES
5+
include $(LEVEL)/Makefile.rules
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#! /bin/sh
2+
3+
linux_check_core_pattern()
4+
{
5+
if grep -q '^|' </proc/sys/kernel/core_pattern; then
6+
cat <<EOF
7+
Your system uses a crash report tool ($(cat /proc/sys/kernel/core_pattern)). Core files
8+
will not be generated. Please reset /proc/sys/kernel/core_pattern (requires root
9+
privileges) to enable core generation.
10+
EOF
11+
exit 1
12+
fi
13+
}
14+
15+
OS=$(uname -s)
16+
case "$OS" in
17+
FreeBSD)
18+
core_pattern=$(sysctl -n kern.corefile)
19+
;;
20+
Linux)
21+
core_pattern=$(cat /proc/sys/kernel/core_pattern)
22+
;;
23+
*)
24+
echo "OS $OS not supported" >&2
25+
exit 1
26+
;;
27+
esac
28+
29+
set -e -x
30+
31+
if [ "$OS" = Linux ]; then
32+
linux_check_core_pattern
33+
fi
34+
35+
ulimit -c 1000
36+
real_limit=$(ulimit -c)
37+
if [ $real_limit -lt 100 ]; then
38+
cat <<EOF
39+
Unable to increase the core file limit. Core file may be truncated!
40+
To fix this, increase HARD core file limit (ulimit -H -c 1000). This may require root
41+
privileges.
42+
EOF
43+
fi
44+
45+
rm -f a.out
46+
make -f main.mk
47+
48+
cat <<EOF
49+
Executable file is in a.out.
50+
Core file will be saved according to pattern $core_pattern.
51+
EOF
52+
53+
# Save stack size and core_dump_filter
54+
stack_size=`ulimit -s`
55+
ulimit -Ss 32 # Decrease stack size to 32k => smaller core files.
56+
57+
core_dump_filter=`cat /proc/self/coredump_filter`
58+
echo 0 > /proc/self/coredump_filter
59+
60+
exec ./a.out
61+
62+
# Reset stack size and core_dump_filter
63+
echo core_dump_filter > /proc/self/coredump_filter
64+
ulimit -s $stack_size

‎lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

+32-1
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,29 @@ Error ProcessElfCore::DoLoadCore() {
214214

215215
SetUnixSignals(UnixSignals::Create(GetArchitecture()));
216216

217+
// Ensure we found at least one thread that was stopped on a signal.
218+
bool siginfo_signal_found = false;
219+
bool prstatus_signal_found = false;
220+
// Check we found a signal in a SIGINFO note.
221+
for (const auto &thread_data: m_thread_data) {
222+
if (thread_data.signo != 0)
223+
siginfo_signal_found = true;
224+
if (thread_data.prstatus_sig != 0)
225+
prstatus_signal_found = true;
226+
}
227+
if (!siginfo_signal_found) {
228+
// If we don't have signal from SIGINFO use the signal from each threads
229+
// PRSTATUS note.
230+
if (prstatus_signal_found) {
231+
for (auto &thread_data: m_thread_data)
232+
thread_data.signo = thread_data.prstatus_sig;
233+
} else if (m_thread_data.size() > 0) {
234+
// If all else fails force the first thread to be SIGSTOP
235+
m_thread_data.begin()->signo =
236+
GetUnixSignals()->GetSignalNumberFromName("SIGSTOP");
237+
}
238+
}
239+
217240
// Core files are useless without the main executable. See if we can locate
218241
// the main
219242
// executable using data we found in the core file notes.
@@ -402,6 +425,7 @@ enum {
402425
NT_AUXV,
403426
NT_FILE = 0x46494c45,
404427
NT_PRXFPREG = 0x46e62b7f,
428+
NT_SIGINFO = 0x53494749,
405429
};
406430

407431
namespace FREEBSD {
@@ -485,6 +509,7 @@ Error ProcessElfCore::ParseThreadContextsFromNoteSegment(
485509
ArchSpec arch = GetArchitecture();
486510
ELFLinuxPrPsInfo prpsinfo;
487511
ELFLinuxPrStatus prstatus;
512+
ELFLinuxSigInfo siginfo;
488513
size_t header_size;
489514
size_t len;
490515
Error error;
@@ -546,7 +571,7 @@ Error ProcessElfCore::ParseThreadContextsFromNoteSegment(
546571
error = prstatus.Parse(note_data, arch);
547572
if (error.Fail())
548573
return error;
549-
thread_data->signo = prstatus.pr_cursig;
574+
thread_data->prstatus_sig = prstatus.pr_cursig;
550575
thread_data->tid = prstatus.pr_pid;
551576
header_size = ELFLinuxPrStatus::GetSize(arch);
552577
len = note_data.GetByteSize() - header_size;
@@ -588,6 +613,12 @@ Error ProcessElfCore::ParseThreadContextsFromNoteSegment(
588613
m_nt_file_entries[i].path.SetCString(path);
589614
}
590615
} break;
616+
case NT_SIGINFO: {
617+
error = siginfo.Parse(note_data, arch);
618+
if (error.Fail())
619+
return error;
620+
thread_data->signo = siginfo.si_signo;
621+
} break;
591622
default:
592623
break;
593624
}

‎lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp

+42
Original file line numberDiff line numberDiff line change
@@ -320,3 +320,45 @@ Error ELFLinuxPrPsInfo::Parse(DataExtractor &data, ArchSpec &arch) {
320320

321321
return error;
322322
}
323+
324+
//----------------------------------------------------------------
325+
// Parse SIGINFO from NOTE entry
326+
//----------------------------------------------------------------
327+
ELFLinuxSigInfo::ELFLinuxSigInfo() {
328+
memset(this, 0, sizeof(ELFLinuxSigInfo));
329+
}
330+
331+
Error ELFLinuxSigInfo::Parse(DataExtractor &data, const ArchSpec &arch) {
332+
Error error;
333+
ByteOrder byteorder = data.GetByteOrder();
334+
if (GetSize(arch) > data.GetByteSize()) {
335+
error.SetErrorStringWithFormat(
336+
"NT_SIGINFO size should be %zu, but the remaining bytes are: %" PRIu64,
337+
GetSize(arch), data.GetByteSize());
338+
return error;
339+
}
340+
341+
switch (arch.GetCore()) {
342+
case ArchSpec::eCore_x86_64_x86_64:
343+
data.ExtractBytes(0, sizeof(ELFLinuxPrStatus), byteorder, this);
344+
break;
345+
case ArchSpec::eCore_s390x_generic:
346+
case ArchSpec::eCore_x86_32_i386:
347+
case ArchSpec::eCore_x86_32_i486: {
348+
// Parsing from a 32 bit ELF core file, and populating/reusing the structure
349+
// properly, because the struct is for the 64 bit version
350+
offset_t offset = 0;
351+
si_signo = data.GetU32(&offset);
352+
si_code = data.GetU32(&offset);
353+
si_errno = data.GetU32(&offset);
354+
355+
break;
356+
}
357+
default:
358+
error.SetErrorStringWithFormat("ELFLinuxSigInfo::%s Unknown architecture",
359+
__FUNCTION__);
360+
break;
361+
}
362+
363+
return error;
364+
}

‎lldb/source/Plugins/Process/elf-core/ThreadElfCore.h

+34-1
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,38 @@ struct ELFLinuxPrStatus {
8282
static_assert(sizeof(ELFLinuxPrStatus) == 112,
8383
"sizeof ELFLinuxPrStatus is not correct!");
8484

85+
struct ELFLinuxSigInfo {
86+
int32_t si_signo;
87+
int32_t si_code;
88+
int32_t si_errno;
89+
90+
ELFLinuxSigInfo();
91+
92+
lldb_private::Error Parse(lldb_private::DataExtractor &data,
93+
const lldb_private::ArchSpec &arch);
94+
95+
// Return the bytesize of the structure
96+
// 64 bit - just sizeof
97+
// 32 bit - hardcoded because we are reusing the struct, but some of the
98+
// members are smaller -
99+
// so the layout is not the same
100+
static size_t GetSize(const lldb_private::ArchSpec &arch) {
101+
switch (arch.GetCore()) {
102+
case lldb_private::ArchSpec::eCore_x86_64_x86_64:
103+
return sizeof(ELFLinuxSigInfo);
104+
case lldb_private::ArchSpec::eCore_s390x_generic:
105+
case lldb_private::ArchSpec::eCore_x86_32_i386:
106+
case lldb_private::ArchSpec::eCore_x86_32_i486:
107+
return 12;
108+
default:
109+
return 0;
110+
}
111+
}
112+
};
113+
114+
static_assert(sizeof(ELFLinuxSigInfo) == 12,
115+
"sizeof ELFLinuxSigInfo is not correct!");
116+
85117
// PRPSINFO structure's size differs based on architecture.
86118
// This is the layout in the x86-64 arch case.
87119
// In the i386 case we parse it manually and fill it again
@@ -133,7 +165,8 @@ struct ThreadData {
133165
lldb_private::DataExtractor fpregset;
134166
lldb_private::DataExtractor vregset;
135167
lldb::tid_t tid;
136-
int signo;
168+
int signo = 0;
169+
int prstatus_sig = 0;
137170
std::string name;
138171
};
139172

0 commit comments

Comments
 (0)
Please sign in to comment.