dvlahovski updated this revision to Diff 65957.
dvlahovski added a comment.
Checking for null pointer, in case of a corrupt core file
https://reviews.llvm.org/D22917
Files:
packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/elf-core/ProcessElfCore.h
source/Plugins/Process/elf-core/ThreadElfCore.cpp
source/Plugins/Process/elf-core/ThreadElfCore.h
Index: source/Plugins/Process/elf-core/ThreadElfCore.h
===================================================================
--- source/Plugins/Process/elf-core/ThreadElfCore.h
+++ source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -26,8 +26,10 @@
};
// PRSTATUS structure's size differs based on architecture.
-// Currently parsing done only for x86-64 architecture by
-// simply reading data from the buffer.
+// This is the layout in the x86-64 arch.
+// In the i386 case we parse it manually and fill it again
+// in the same structure
+// The gp registers are also a part of this struct, but they are handled separately
#undef si_signo
#undef si_code
@@ -56,25 +58,36 @@
ELFLinuxPrStatus();
- bool
+ lldb_private::Error
Parse(lldb_private::DataExtractor &data, 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(lldb_private::ArchSpec &arch)
{
switch(arch.GetCore())
{
case lldb_private::ArchSpec::eCore_s390x_generic:
case lldb_private::ArchSpec::eCore_x86_64_x86_64:
return sizeof(ELFLinuxPrStatus);
+ case lldb_private::ArchSpec::eCore_x86_32_i386:
+ case lldb_private::ArchSpec::eCore_x86_32_i486:
+ return 72;
default:
return 0;
}
}
};
static_assert(sizeof(ELFLinuxPrStatus) == 112, "sizeof ELFLinuxPrStatus 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
+// in the same structure
struct ELFLinuxPrPsInfo
{
char pr_state;
@@ -93,17 +106,24 @@
ELFLinuxPrPsInfo();
- bool
+ lldb_private::Error
Parse(lldb_private::DataExtractor &data, 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(lldb_private::ArchSpec &arch)
{
switch(arch.GetCore())
{
case lldb_private::ArchSpec::eCore_s390x_generic:
case lldb_private::ArchSpec::eCore_x86_64_x86_64:
return sizeof(ELFLinuxPrPsInfo);
+ case lldb_private::ArchSpec::eCore_x86_32_i386:
+ case lldb_private::ArchSpec::eCore_x86_32_i486:
+ return 124;
default:
return 0;
}
Index: source/Plugins/Process/elf-core/ThreadElfCore.cpp
===================================================================
--- source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -19,6 +19,7 @@
#include "Plugins/Process/Utility/RegisterContextLinux_arm.h"
#include "Plugins/Process/Utility/RegisterContextLinux_arm64.h"
#include "Plugins/Process/Utility/RegisterContextLinux_s390x.h"
+#include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
#include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
#include "Plugins/Process/Utility/RegisterContextFreeBSD_arm.h"
#include "Plugins/Process/Utility/RegisterContextFreeBSD_arm64.h"
@@ -144,6 +145,9 @@
case llvm::Triple::systemz:
reg_interface = new RegisterContextLinux_s390x(arch);
break;
+ case llvm::Triple::x86:
+ reg_interface = new RegisterContextLinux_i386(arch);
+ break;
case llvm::Triple::x86_64:
reg_interface = new RegisterContextLinux_x86_64(arch);
break;
@@ -219,20 +223,65 @@
memset(this, 0, sizeof(ELFLinuxPrStatus));
}
-bool
+Error
ELFLinuxPrStatus::Parse(DataExtractor &data, ArchSpec &arch)
{
+ Error error;
ByteOrder byteorder = data.GetByteOrder();
- size_t len;
+ if (GetSize(arch) > data.GetByteSize())
+ {
+ error.SetErrorStringWithFormat("NT_PRSTATUS size should be %lu, but the remaining bytes are: %lu",
+ GetSize(arch), data.GetByteSize());
+ return error;
+ }
+
switch(arch.GetCore())
{
case ArchSpec::eCore_s390x_generic:
case ArchSpec::eCore_x86_64_x86_64:
- len = data.ExtractBytes(0, sizeof(ELFLinuxPrStatus), byteorder, this);
- return len == sizeof(ELFLinuxPrStatus);
+ data.ExtractBytes(0, sizeof(ELFLinuxPrStatus), byteorder, this);
+ break;
+ 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);
+
+ pr_cursig = data.GetU16(&offset);
+ offset += 2; // pad
+
+ pr_sigpend = data.GetU32(&offset);
+ pr_sighold = data.GetU32(&offset);
+
+ pr_pid = data.GetU32(&offset);
+ pr_ppid = data.GetU32(&offset);
+ pr_pgrp = data.GetU32(&offset);
+ pr_sid = data.GetU32(&offset);
+
+ pr_utime.tv_sec = data.GetU32(&offset);
+ pr_utime.tv_usec = data.GetU32(&offset);
+
+ pr_stime.tv_sec = data.GetU32(&offset);
+ pr_stime.tv_usec = data.GetU32(&offset);
+
+ pr_cutime.tv_sec = data.GetU32(&offset);
+ pr_cutime.tv_usec = data.GetU32(&offset);
+
+ pr_cstime.tv_sec = data.GetU32(&offset);
+ pr_cstime.tv_usec = data.GetU32(&offset);
+
+ break;
+ }
default:
- return false;
+ error.SetErrorStringWithFormat("ELFLinuxPrStatus::%s Unknown architecture", __FUNCTION__);
+ break;
}
+
+ return error;
}
//----------------------------------------------------------------
@@ -243,19 +292,61 @@
memset(this, 0, sizeof(ELFLinuxPrPsInfo));
}
-bool
+Error
ELFLinuxPrPsInfo::Parse(DataExtractor &data, ArchSpec &arch)
{
+ Error error;
ByteOrder byteorder = data.GetByteOrder();
- size_t len;
+ if (GetSize(arch) > data.GetByteSize())
+ {
+ error.SetErrorStringWithFormat("NT_PRPSINFO size should be %lu, but the remaining bytes are: %lu",
+ GetSize(arch), data.GetByteSize());
+ return error;
+ }
+
switch(arch.GetCore())
{
case ArchSpec::eCore_s390x_generic:
case ArchSpec::eCore_x86_64_x86_64:
- len = data.ExtractBytes(0, sizeof(ELFLinuxPrPsInfo), byteorder, this);
- return len == sizeof(ELFLinuxPrPsInfo);
+ data.ExtractBytes(0, sizeof(ELFLinuxPrPsInfo), byteorder, this);
+ break;
+ 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
+ size_t size = 0;
+ offset_t offset = 0;
+
+ pr_state = data.GetU8(&offset);
+ pr_sname = data.GetU8(&offset);
+ pr_zomb = data.GetU8(&offset);
+ pr_nice = data.GetU8(&offset);
+
+ pr_flag = data.GetU32(&offset);
+ pr_uid = data.GetU16(&offset);
+ pr_gid = data.GetU16(&offset);
+
+ pr_pid = data.GetU32(&offset);
+ pr_ppid = data.GetU32(&offset);
+ pr_pgrp = data.GetU32(&offset);
+ pr_sid = data.GetU32(&offset);
+
+ size = 16;
+ data.ExtractBytes(offset, size, byteorder, pr_fname);
+ offset += size;
+
+ size = 80;
+ data.ExtractBytes(offset, size, byteorder, pr_psargs);
+ offset += size;
+
+ break;
+ }
default:
- return false;
+ error.SetErrorStringWithFormat("ELFLinuxPrPsInfo::%s Unknown architecture", __FUNCTION__);
+ break;
}
+
+ return error;
}
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===================================================================
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -166,9 +166,9 @@
std::vector<NT_FILE_Entry> m_nt_file_entries;
// Parse thread(s) data structures(prstatus, prpsinfo) from given NOTE segment
- void
- ParseThreadContextsFromNoteSegment (const elf::ELFProgramHeader *segment_header,
- lldb_private::DataExtractor segment_data);
+ lldb_private::Error
+ ParseThreadContextsFromNoteSegment(const elf::ELFProgramHeader *segment_header,
+ lldb_private::DataExtractor segment_data);
// Returns number of thread contexts stored in the core file
uint32_t
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===================================================================
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -223,8 +223,11 @@
// Parse thread contexts and auxv structure
if (header->p_type == llvm::ELF::PT_NOTE)
- ParseThreadContextsFromNoteSegment(header, data);
-
+ {
+ error = ParseThreadContextsFromNoteSegment(header, data);
+ if (error.Fail())
+ return error;
+ }
// PT_LOAD segments contains address map
if (header->p_type == llvm::ELF::PT_LOAD)
{
@@ -533,7 +536,7 @@
/// new thread when it finds NT_PRSTATUS or NT_PRPSINFO NOTE entry.
/// For case (b) there may be either one NT_PRPSINFO per thread, or a single
/// one that applies to all threads (depending on the platform type).
-void
+Error
ProcessElfCore::ParseThreadContextsFromNoteSegment(const elf::ELFProgramHeader *segment_header,
DataExtractor segment_data)
{
@@ -549,6 +552,7 @@
ELFLinuxPrStatus prstatus;
size_t header_size;
size_t len;
+ Error error;
// Loop through the NOTE entires in the segment
while (offset < segment_header->p_filesz)
@@ -610,7 +614,9 @@
{
case NT_PRSTATUS:
have_prstatus = true;
- prstatus.Parse(note_data, arch);
+ error = prstatus.Parse(note_data, arch);
+ if (error.Fail())
+ return error;
thread_data->signo = prstatus.pr_cursig;
thread_data->tid = prstatus.pr_pid;
header_size = ELFLinuxPrStatus::GetSize(arch);
@@ -622,7 +628,9 @@
break;
case NT_PRPSINFO:
have_prpsinfo = true;
- prpsinfo.Parse(note_data, arch);
+ error = prpsinfo.Parse(note_data, arch);
+ if (error.Fail())
+ return error;
thread_data->name = prpsinfo.pr_fname;
SetID(prpsinfo.pr_pid);
break;
@@ -663,6 +671,8 @@
{
m_thread_data.push_back(*thread_data);
}
+
+ return error;
}
uint32_t
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1479,7 +1479,7 @@
else if (note.n_name == LLDB_NT_OWNER_CORE)
{
// Parse the NT_FILE to look for stuff in paths to shared libraries
- // As the contents look like:
+ // As the contents look like this in a 64 bit ELF core file:
// count = 0x000000000000000a (10)
// page_size = 0x0000000000001000 (4096)
// Index start end file_ofs path
@@ -1494,14 +1494,24 @@
// [ 7] 0x00007fa79cdb2000 0x00007fa79cdd5000 0x0000000000000000 /lib/x86_64-linux-gnu/ld-2.19.so
// [ 8] 0x00007fa79cfd4000 0x00007fa79cfd5000 0x0000000000000022 /lib/x86_64-linux-gnu/ld-2.19.so
// [ 9] 0x00007fa79cfd5000 0x00007fa79cfd6000 0x0000000000000023 /lib/x86_64-linux-gnu/ld-2.19.so
+ // In the 32 bit ELFs the count, page_size, start, end, file_ofs are uint32_t
+ // For reference: see readelf source code (in binutils).
if (note.n_type == NT_FILE)
{
- uint64_t count = data.GetU64(&offset);
- offset += 8 + 3*8*count; // Skip page size and all start/end/file_ofs
+ uint64_t count = data.GetAddress(&offset);
+ const char *cstr;
+ data.GetAddress(&offset); // Skip page size
+ offset += count * 3 * data.GetAddressByteSize(); // Skip all start/end/file_ofs
for (size_t i=0; i<count; ++i)
{
- llvm::StringRef path(data.GetCStr(&offset));
- if (path.startswith("/lib/x86_64-linux-gnu"))
+ cstr = data.GetCStr(&offset);
+ if(cstr == nullptr)
+ {
+ error.SetErrorStringWithFormat("ObjectFileELF::%s trying to read at an offset after the end (GetCStr returned nullptr)", __FUNCTION__);
+ return error;
+ }
+ llvm::StringRef path(cstr);
+ if (path.startswith("/lib/x86_64-linux-gnu") || path.startswith("/lib/i386-linux-gnu"))
{
arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
break;
Index: packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
+++ packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
@@ -25,7 +25,6 @@
_x86_64_regions = 5
_s390x_regions = 2
- @skipIf(bugnumber="llvm.org/pr26947")
def test_i386(self):
"""Test that lldb can read the process information from an i386 linux core file."""
self.do_test("i386", self._i386_pid, self._i386_regions)
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
