[Lldb-commits] [lldb] r318903 - elf-core: Split up parsing code into os-specific functions

2017-11-23 Thread Pavel Labath via lldb-commits
Author: labath
Date: Thu Nov 23 02:50:34 2017
New Revision: 318903

URL: http://llvm.org/viewvc/llvm-project?rev=318903&view=rev
Log:
elf-core: Split up parsing code into os-specific functions

Summary:
We've had a single function responsible for splitting a core segment
into notes, and parsing the notes themselves, bearing in mind variations
between 4 supported OS types. This commit splits that code into 5
pieces:
- (os-independent) code for splitting a segment into individual notes
- per-os function for parsing the notes into thread information

Reviewers: clayborg, krytarowski, emaste, alexandreyy, kettenis

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp
lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h
lldb/trunk/source/Plugins/Process/elf-core/elf-core-enums.h

Modified: lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp?rev=318903&r1=318902&r2=318903&view=diff
==
--- lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp (original)
+++ lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp Thu Nov 23 
02:50:34 2017
@@ -102,10 +102,7 @@ bool ProcessElfCore::CanDebug(lldb::Targ
 ProcessElfCore::ProcessElfCore(lldb::TargetSP target_sp,
lldb::ListenerSP listener_sp,
const FileSpec &core_file)
-: Process(target_sp, listener_sp), m_core_module_sp(),
-  m_core_file(core_file), m_dyld_plugin_name(),
-  m_os(llvm::Triple::UnknownOS), m_thread_data_valid(false),
-  m_thread_data(), m_core_aranges() {}
+: Process(target_sp, listener_sp), m_core_file(core_file) {}
 
 //--
 // Destructor
@@ -194,9 +191,8 @@ Status ProcessElfCore::DoLoadCore() {
 
 // Parse thread contexts and auxv structure
 if (header->p_type == llvm::ELF::PT_NOTE) {
-  error = ParseThreadContextsFromNoteSegment(header, data);
-  if (error.Fail())
-return error;
+  if (llvm::Error error = ParseThreadContextsFromNoteSegment(header, data))
+return Status(std::move(error));
 }
 // PT_LOAD segments contains address map
 if (header->p_type == llvm::ELF::PT_LOAD) {
@@ -405,7 +401,6 @@ size_t ProcessElfCore::DoReadMemory(lldb
 
 void ProcessElfCore::Clear() {
   m_thread_list.Clear();
-  m_os = llvm::Triple::UnknownOS;
 
   SetUnixSignals(std::make_shared());
 }
@@ -429,8 +424,9 @@ lldb::addr_t ProcessElfCore::GetImageInf
 }
 
 // Parse a FreeBSD NT_PRSTATUS note - see FreeBSD sys/procfs.h for details.
-static void ParseFreeBSDPrStatus(ThreadData &thread_data, DataExtractor &data,
- ArchSpec &arch) {
+static void ParseFreeBSDPrStatus(ThreadData &thread_data,
+ const DataExtractor &data,
+ const ArchSpec &arch) {
   lldb::offset_t offset = 0;
   bool lp64 = (arch.GetMachine() == llvm::Triple::aarch64 ||
arch.GetMachine() == llvm::Triple::mips64 ||
@@ -459,12 +455,8 @@ static void ParseFreeBSDPrStatus(ThreadD
   thread_data.gpregset = DataExtractor(data, offset, len);
 }
 
-static void ParseFreeBSDThrMisc(ThreadData &thread_data, DataExtractor &data) {
-  lldb::offset_t offset = 0;
-  thread_data.name = data.GetCStr(&offset, 20);
-}
-
-static void ParseNetBSDProcInfo(ThreadData &thread_data, DataExtractor &data) {
+static void ParseNetBSDProcInfo(ThreadData &thread_data,
+const DataExtractor &data) {
   lldb::offset_t offset = 0;
 
   int version = data.GetU32(&offset);
@@ -475,7 +467,8 @@ static void ParseNetBSDProcInfo(ThreadDa
   thread_data.signo = data.GetU32(&offset);
 }
 
-static void ParseOpenBSDProcInfo(ThreadData &thread_data, DataExtractor &data) 
{
+static void ParseOpenBSDProcInfo(ThreadData &thread_data,
+ const DataExtractor &data) {
   lldb::offset_t offset = 0;
 
   int version = data.GetU32(&offset);
@@ -486,217 +479,290 @@ static void ParseOpenBSDProcInfo(ThreadD
   thread_data.signo = data.GetU32(&offset);
 }
 
-/// Parse Thread context from PT_NOTE segment and store it in the thread list
-/// Notes:
-/// 1) A PT_NOTE segment is composed of one or more NOTE entries.
-/// 2) NOTE Entry contains a standard header followed by variable size data.
-///   (see ELFNote structure)
-/// 3) A Thread Context in a core file usually described by 3 NOTE entries.
-///a) NT_PRSTATUS - Register context
-///b) NT_PRPSINFO - Process info(pid..)
-///c) NT_FPREGSET - Floating point registers
-/// 4) The NOTE entries can 

[Lldb-commits] [PATCH] D40311: elf-core: Split up parsing code into os-specific functions

2017-11-23 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318903: elf-core: Split up parsing code into os-specific 
functions (authored by labath).

Repository:
  rL LLVM

https://reviews.llvm.org/D40311

Files:
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.cpp
  lldb/trunk/source/Plugins/Process/elf-core/ProcessElfCore.h
  lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp
  lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h
  lldb/trunk/source/Plugins/Process/elf-core/elf-core-enums.h

Index: lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h
===
--- lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h
+++ lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -58,15 +58,15 @@
 
   ELFLinuxPrStatus();
 
-  lldb_private::Status Parse(lldb_private::DataExtractor &data,
- lldb_private::ArchSpec &arch);
+  lldb_private::Status Parse(const 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(lldb_private::ArchSpec &arch);
+  static size_t GetSize(const lldb_private::ArchSpec &arch);
 };
 
 static_assert(sizeof(ELFLinuxPrStatus) == 112,
@@ -79,7 +79,7 @@
 
   ELFLinuxSigInfo();
 
-  lldb_private::Status Parse(lldb_private::DataExtractor &data,
+  lldb_private::Status Parse(const lldb_private::DataExtractor &data,
  const lldb_private::ArchSpec &arch);
 
   // Return the bytesize of the structure
@@ -114,15 +114,15 @@
 
   ELFLinuxPrPsInfo();
 
-  lldb_private::Status Parse(lldb_private::DataExtractor &data,
- lldb_private::ArchSpec &arch);
+  lldb_private::Status Parse(const 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(lldb_private::ArchSpec &arch);
+  static size_t GetSize(const lldb_private::ArchSpec &arch);
 };
 
 static_assert(sizeof(ELFLinuxPrPsInfo) == 136,
Index: lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp
===
--- lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ lldb/trunk/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -259,7 +259,7 @@
   memset(this, 0, sizeof(ELFLinuxPrStatus));
 }
 
-size_t ELFLinuxPrStatus::GetSize(lldb_private::ArchSpec &arch) {
+size_t ELFLinuxPrStatus::GetSize(const lldb_private::ArchSpec &arch) {
   constexpr size_t mips_linux_pr_status_size_o32 = 96;
   constexpr size_t mips_linux_pr_status_size_n32 = 72;
   if (arch.IsMIPS()) {
@@ -285,7 +285,8 @@
   }
 }
 
-Status ELFLinuxPrStatus::Parse(DataExtractor &data, ArchSpec &arch) {
+Status ELFLinuxPrStatus::Parse(const DataExtractor &data,
+   const ArchSpec &arch) {
   Status error;
   if (GetSize(arch) > data.GetByteSize()) {
 error.SetErrorStringWithFormat(
@@ -334,7 +335,7 @@
   memset(this, 0, sizeof(ELFLinuxPrPsInfo));
 }
 
-size_t ELFLinuxPrPsInfo::GetSize(lldb_private::ArchSpec &arch) {
+size_t ELFLinuxPrPsInfo::GetSize(const lldb_private::ArchSpec &arch) {
   constexpr size_t mips_linux_pr_psinfo_size_o32_n32 = 128;
   if (arch.IsMIPS()) {
 uint8_t address_byte_size = arch.GetAddressByteSize();
@@ -355,7 +356,8 @@
   }
 }
 
-Status ELFLinuxPrPsInfo::Parse(DataExtractor &data, ArchSpec &arch) {
+Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
+   const ArchSpec &arch) {
   Status error;
   ByteOrder byteorder = data.GetByteOrder();
   if (GetSize(arch) > data.GetByteSize()) {
@@ -424,7 +426,7 @@
   }
 }
 
-Status ELFLinuxSigInfo::Parse(DataExtractor &data, const ArchSpec &arch) {
+Status ELFLinuxSigInfo::Parse(const DataExtractor &data, const ArchSpec &arch) {
   Status error;
   if (GetSize(arch) > data.GetByteSize()) {
 error.SetErrorStringWithFormat(
Index: lldb/trunk/source/Plugins/Process/elf-core/elf-core-enums.h
===
--- lldb/trunk/source/Plugins/Process/elf-core/elf-core-enums.h
+++ lldb/trunk/source/Plugins/Process/elf-core/elf-core-enums.h
@@ -10,6 +10,10 @@
 #ifndef LLDB_ELF_CORE_ENUMS_H
 #define LLDB_ELF_CORE_ENUMS_H
 
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "lldb/Utility/DataExtractor.h"
+
+namespace lldb_private {
 /// Core files PT_NOTE segment descriptor types
 
 namespace FREEBSD {
@@ -52,4 +56,11 @@
 };
 }
 
+struct CoreNote {
+  ELFNote info;
+ 

[Lldb-commits] [PATCH] D40133: elf-core: Convert remaining register context to use register set maps

2017-11-23 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 124064.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: mgorny.

This is a slight deviation from the initial approach. What I've done here is
that I've kept the register set indices in an os-specific form. I've done this
because it enables the parsing code in ProcessElfCore to be
architecture-agnostic -- it can just deal with OS specifics, and can just shove
any note it does not recognise to the side. These will be then parsed in the
corresponding register contexts, which can only deal with architecture
specifics, and ignore OS details, as the translation of os-specific note types
is provided by a separate lookup table.

This way, adding an register set, which is already supported on other OSes, to a
new OS, should in most cases be as simple as adding a new entry into the
register set description map.


https://reviews.llvm.org/D40133

Files:
  source/Plugins/Process/elf-core/CMakeLists.txt
  source/Plugins/Process/elf-core/ProcessElfCore.cpp
  source/Plugins/Process/elf-core/ProcessElfCore.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_mips64.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_mips64.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_powerpc.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_powerpc.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_ppc64le.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_ppc64le.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_s390x.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_s390x.h
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp
  source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.h
  source/Plugins/Process/elf-core/RegisterUtilities.cpp
  source/Plugins/Process/elf-core/RegisterUtilities.h
  source/Plugins/Process/elf-core/ThreadElfCore.cpp
  source/Plugins/Process/elf-core/ThreadElfCore.h
  source/Plugins/Process/elf-core/elf-core-enums.h
  source/Plugins/Process/minidump/ThreadMinidump.cpp

Index: source/Plugins/Process/minidump/ThreadMinidump.cpp
===
--- source/Plugins/Process/minidump/ThreadMinidump.cpp
+++ source/Plugins/Process/minidump/ThreadMinidump.cpp
@@ -17,8 +17,8 @@
 // Other libraries and framework includes
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
-
 #include "Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.h"
+#include "Plugins/Process/elf-core/RegisterUtilities.h"
 
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StopInfo.h"
@@ -74,20 +74,18 @@
   reg_interface = new RegisterContextLinux_i386(arch);
   lldb::DataBufferSP buf =
   ConvertMinidumpContext_x86_32(m_gpregset_data, reg_interface);
-  DataExtractor gpregs(buf, lldb::eByteOrderLittle, 4);
-  DataExtractor fpregs;
+  DataExtractor gpregset(buf, lldb::eByteOrderLittle, 4);
   m_thread_reg_ctx_sp.reset(new RegisterContextCorePOSIX_x86_64(
-  *this, reg_interface, gpregs, fpregs));
+  *this, reg_interface, gpregset, {}));
   break;
 }
 case llvm::Triple::x86_64: {
   reg_interface = new RegisterContextLinux_x86_64(arch);
   lldb::DataBufferSP buf =
   ConvertMinidumpContext_x86_64(m_gpregset_data, reg_interface);
-  DataExtractor gpregs(buf, lldb::eByteOrderLittle, 8);
-  DataExtractor fpregs;
+  DataExtractor gpregset(buf, lldb::eByteOrderLittle, 8);
   m_thread_reg_ctx_sp.reset(new RegisterContextCorePOSIX_x86_64(
-  *this, reg_interface, gpregs, fpregs));
+  *this, reg_interface, gpregset, {}));
   break;
 }
 default:
Index: source/Plugins/Process/elf-core/elf-core-enums.h
===
--- source/Plugins/Process/elf-core/elf-core-enums.h
+++ /dev/null
@@ -1,66 +0,0 @@
-//===-- elf-core-enums.h *- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef LLDB_ELF_CORE_ENUMS_H
-#define LLDB_ELF_CORE_ENUMS_H
-
-#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
-#include "lldb/Utility/DataExtractor.h"
-
-namespace lldb_private {
-/// Core files PT_NOTE segment descriptor types
-
-namespace FREEBSD {
-enum {
-  NT_PRSTATUS = 1,
-  NT_FPREGSET,
-  NT_PRPSINFO,
-  NT_THRMISC = 7,
-  NT

[Lldb-commits] [PATCH] D40212: refactor: Unify+simplify DWARFCompileUnit ctor+Clear() into in-class initializers + Extract()

2017-11-23 Thread Jan Kratochvil via Phabricator via lldb-commits
jankratochvil updated this revision to Diff 124103.
jankratochvil retitled this revision from "refactor: Unify+simplify 
DWARFCompileUnit ctor+Clear() into in-class initializers" to "refactor: 
Unify+simplify DWARFCompileUnit ctor+Clear() into in-class initializers + 
Extract()".
jankratochvil edited the summary of this revision.
Herald added a subscriber: aprantl.

https://reviews.llvm.org/D40212

Files:
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -61,8 +61,6 @@
   DWARFDebugAranges &GetCompileUnitAranges();
 
 protected:
-  typedef std::shared_ptr DWARFCompileUnitSP;
-
   static bool OffsetLessThanCompileUnitOffset(dw_offset_t offset,
   const DWARFCompileUnitSP &cu_sp);
 
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -97,17 +97,12 @@
   if (m_compile_units.empty()) {
 if (m_dwarf2Data != NULL) {
   lldb::offset_t offset = 0;
-  const DWARFDataExtractor &debug_info_data =
-  m_dwarf2Data->get_debug_info_data();
-  while (debug_info_data.ValidOffset(offset)) {
-DWARFCompileUnitSP cu_sp(new DWARFCompileUnit(m_dwarf2Data));
-// Out of memory?
+  for (;;) {
+DWARFCompileUnitSP cu_sp =
+DWARFCompileUnit::Extract(m_dwarf2Data, &offset);
 if (cu_sp.get() == NULL)
   break;
 
-if (cu_sp->Extract(debug_info_data, &offset) == false)
-  break;
-
 m_compile_units.push_back(cu_sp);
 
 offset = cu_sp->GetNextCompileUnitOffset();
@@ -248,12 +243,13 @@
   if (dwarf2Data) {
 lldb::offset_t offset = 0;
 uint32_t depth = 0;
-DWARFCompileUnitSP cu(new DWARFCompileUnit(dwarf2Data));
-if (cu.get() == NULL)
-  return;
 DWARFDebugInfoEntry die;
 
-while (cu->Extract(dwarf2Data->get_debug_info_data(), &offset)) {
+for (;;) {
+  DWARFCompileUnitSP cu = DWARFCompileUnit::Extract(dwarf2Data, &offset);
+  if (cu.get() == NULL)
+	break;
+
   const dw_offset_t next_cu_offset = cu->GetNextCompileUnitOffset();
 
   depth = 0;
@@ -288,12 +284,6 @@
   if (!dwarf2Data->get_debug_info_data().ValidOffset(offset))
 break;
 
-  // See if during the callback anyone retained a copy of the compile
-  // unit other than ourselves and if so, let whomever did own the object
-  // and create a new one for our own use!
-  if (!cu.unique())
-cu.reset(new DWARFCompileUnit(dwarf2Data));
-
   // Make sure we start on a proper
   offset = next_cu_offset;
 }
Index: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
@@ -18,6 +18,8 @@
 class SymbolFileDWARF;
 class SymbolFileDWARFDwo;
 
+typedef std::shared_ptr DWARFCompileUnitSP;
+
 class DWARFCompileUnit {
 public:
   enum Producer {
@@ -28,17 +30,15 @@
 eProcucerOther
   };
 
-  DWARFCompileUnit(SymbolFileDWARF *dwarf2Data);
+  static DWARFCompileUnitSP Extract(SymbolFileDWARF *dwarf2Data,
+  lldb::offset_t *offset_ptr);
   ~DWARFCompileUnit();
 
-  bool Extract(const lldb_private::DWARFDataExtractor &debug_info,
-   lldb::offset_t *offset_ptr);
   size_t ExtractDIEsIfNeeded(bool cu_die_only);
   DWARFDIE LookupAddress(const dw_addr_t address);
   size_t AppendDIEsWithTag(const dw_tag_t tag,
DWARFDIECollection &matching_dies,
uint32_t depth = UINT32_MAX) const;
-  void Clear();
   bool Verify(lldb_private::Stream *s) const;
   void Dump(lldb_private::Stream *s) const;
   // Offset of the initial length field.
@@ -163,7 +163,7 @@
   SymbolFileDWARF *m_dwarf2Data;
   std::unique_ptr m_dwo_symbol_file;
   const DWARFAbbreviationDeclarationSet *m_abbrevs;
-  void *m_user_data;
+  void *m_user_data = nullptr;
   DWARFDebugInfoEntry::collection
   m_die_array; // The compile unit debug information entry item
   std::unique_ptr m_func_aranges_ap; // A table similar to
@@ -172,24 +172,24 @@
 // points to the exact
 // DW_TAG_subprogram
 // DIEs
-  dw_addr_t m_base_addr;
+  dw_addr_t m_base_addr = 0;
   // Offset of the initial length field.
   dw_offset_t m_offset;