MinidumpParsing: pid, modules, exceptions

[dvlahovski]

Summary

Added parsing of the MiscInfo data stream.
The main member of it that we care about is the process_id
On Linux generated Minidump (from breakpad) we don't have
the MiscInfo, we have the /proc/$pid/status from where we can get the
pid.
Also parsing the module list - the list of all of the loaded
modules/shared libraries.
Finally - parsing the exception stream.

I have unit tests for all of that.
Also added some tests using a Minidump generated from Windows tools (not
from breakpad)

[dvlahovski]

Forgot to run clang-format

[dvlahovski]

Also added parsing code for Minidump strings - the strings in the Minidump format are UTF-16 encoded. I used the code from the WinMiniDump plugin and it can extract a UTF-16 string and convert it to a UTF-8 one.

[labath]

A collection of small remarks, mostly to do with style. Mainly, I'd like to make the tests more strict about what they accept as reasonable values.

A higher level question: Given that we are already in the minidump namespace, do we want all (most) of these symbols to be prefixed with Minidump ? Thoughts?

source/Plugins/Process/minidump/MinidumpParser.cpp

65 ↗

(On Diff #70816)

this will still invoke the copy-constructor of the map. For that to work, your constructor needs to take a rvalue reference to the map.

154 ↗

(On Diff #70816)

How hard would it be to actually detect the os properly here?

Since you've already started processing windows minidumps, it would be great if we could have a GetArchitecture() test for both OSs.

Update: So I see you already have that test, but it does not check the OS part of the triple. If it's not too hard for some reason, let's set that as a part of this CL.

source/Plugins/Process/minidump/MinidumpTypes.cpp

21 ↗

(On Diff #70816)

This is not consistent with the consumeObject function, which also drops the consumed bytes from the buffer.

127 ↗

(On Diff #70816)

pid_t is a host-specific type. Use lldb::pid_t.

148 ↗

(On Diff #70816)

This can end up adding 43 elements to the vector. It will still work, but you'll be doubling your memory usage for no reason. If you really want it, make the vector larger, but as this is not performance critical, maybe you could just use SmallVector<StringRef, 0> ?

source/Plugins/Process/minidump/MinidumpTypes.h

223 ↗

(On Diff #70816)

MinidumpString is just a wrapper around the std::string. Why not return the string directly? (Also the "const" there is unnecessary).

299 ↗

(On Diff #70816)

this is oddly formatted now.

307 ↗

(On Diff #70816)

It looks like this should be a normal method instead of a static one.

318 ↗

(On Diff #70816)

It looks like this should be a normal method instead of a static one.

unittests/Process/minidump/MinidumpParserTest.cpp

95 ↗

(On Diff #70816)

Also check whether the returned ProcStatus object has the contents you expect?

111 ↗

(On Diff #70816)

Fix the todo.

118 ↗

(On Diff #70816)

Check the contents of the returned object.

138 ↗

(On Diff #70816)

Check MiscInfo contents.

[zturner]

source/Plugins/Process/minidump/MinidumpParser.cpp

88–89 ↗

(On Diff #70816)

This is fine, but I prefer to avoid the math wherever possible since it makes code slightly more difficult to read. I would write this as:

auto arr_ref = m_data_sp->GetData().drop_front(rva);

171 ↗

(On Diff #70816)

Same as before, an Optional<ArrayRef> seems a bit redundant. I would just return an empty ArrayRef to represent it not being there. I dont' think there's a valid case for a strema to actually be present, but be 0 bytes in length.

202 ↗

(On Diff #70816)

It will do this parsing every time you ask for the module list. How about storing a std::vector<const MinidumpModule *> in the MinidumpParser class and then you can return an ArrayRef<const MinidumpModule*> to it. So it's lazy evaluation, only done once, with the side benefit of allowing you to return an ArrayRef<T> instead of a more complicated Optional<vector<T>>

source/Plugins/Process/minidump/MinidumpParser.h

47 ↗

(On Diff #70816)

There's a lot of Optional's here. Is it really possible for any subset of these to be optional? Or is it more like if you find one, you will find them all?

61 ↗

(On Diff #70816)

I would drop the Optional here and return ArrayRef<const MinidumpModule*>. Returning an entire vector by value is wasteful on the stack. Using an ArrayRef makes it very lightweight, while still providing value-like semantics in that you won't be able to modify the underlyign container. Also, if its size is 0, you can treat that the same as if the Optional did not have a value.

source/Plugins/Process/minidump/MinidumpTypes.cpp

21 ↗

(On Diff #70816)

Is this logic correct? A buffer may be arbitrarily large and have more data in it besides the string. Perhaps you need to search forward for a null terminator, then only return that portion of the string, then drop that many bytes (plus the null terminator) from the input buffer?

source/Plugins/Process/minidump/MinidumpTypes.h

223 ↗

(On Diff #70816)

Agree, the MinidumpString class seems unnecessary to me. Just make parseMinidumpString be a file static global function and return an std::string.

299 ↗

(On Diff #70816)

Probably because the comment passed 80 columns. I would put the comment on a new line above the field to fix this.

[amccarth]

source/Plugins/Process/minidump/MinidumpTypes.cpp

21 ↗

(On Diff #70816)

Minidump strings aren't zero-terminated. They're counted (in UTF16 code units). So this would have to read the count and "consume" the appropriate number of bytes.

Oh, but this isn't used for minidump strings. It looks like it's for these Linux proc status fields.

[dvlahovski]

Changes regarding all of the comments.

Removed llvm::Optionals where it was unneeded.
Parsing also the OS from the Minidump.

Refined old and added new test cases.

[dvlahovski]

source/Plugins/Process/minidump/MinidumpParser.cpp

222 ↗

(On Diff #71026)

Now that I return an ArrayRef<ModuleList>, which is basically a reinterpret cast of a piece of memory to a list of modules, do you think that this is still something good to do?

At least for now, I'm calling this method only once in the DoLoadCore initializing method of the plugin (still unsubmitted code) and then creating the ModuleSpecs for each module.

source/Plugins/Process/minidump/MinidumpParser.h

46 ↗

(On Diff #71026)

Specifically for the string - I think it's best to return llvm::Optional<std::string> in order to distinguish between empty string and error. And there are some conditions that need to be met for the string to parsed correctly.

60 ↗

(On Diff #71026)

I am returning an ArrayRef<MinidumpModule>. The Modules are consecutive in memory so this seemed like a good idea

source/Plugins/Process/minidump/MinidumpTypes.cpp

21 ↗

(On Diff #71026)

So, breakpad specific string in the Minidump files are not UTF-16 string, but just ascii strings in the file (not null terminated).
When I get the stream "LinuxProcStatus" I know how big it is and set it in the ArrayRef.
So basically it is my intention here to parse the entire ArrayRef to a string

[zturner]

source/Plugins/Process/minidump/MinidumpParser.cpp

222 ↗

(On Diff #71026)

Yea I suppose this is fine now.

source/Plugins/Process/minidump/MinidumpTypes.cpp

21 ↗

(On Diff #71026)

In that case it seems like this function is unnecessary. If all it does is reinterpret an entire ArrayRef as a StringRef, it seems better to do that at each callsite. A generic name like consumeString makes people think of either zero terminated string or length prefixed strength, and this one is neither.

Another option might be to add an additional length argument to the consumeString function, and then call it like llvm::StringRef S = consumeString(Buffer, Buffer.size()) with a comment about why you're treating the entire thing as a string.

[labath]

Just a couple of tiny comments from me.

source/Plugins/Process/minidump/MinidumpParser.cpp

86 ↗

(On Diff #71026)

What should the function do if we call it with an out-of-bounds index? Right now it will crash in drop_front()...

unittests/Process/minidump/MinidumpParserTest.cpp

76 ↗

(On Diff #71026)

ASSERT_EQ(0, thread_list.size())

136 ↗

(On Diff #71026)

ASSERT_NE

[dvlahovski]

Removed consumeString; Fixed comparisons in unittests; out-of-bounds check in MinidumpString parsing

[dvlahovski]

Making the LinuxProcStatus a smarter class

by not parsing the pid each time when GetPid is called

[labath]

Looks good as far as I am concerned.

lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h

 // Other libraries and framework includes
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataBuffer.h"
 #include "lldb/Core/Error.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
 
 // C includes
 
 // C++ includes
 #include <cstring>
 #include <unordered_map>
 
 namespace lldb_private {
 
 namespace minidump {
 
 class MinidumpParser {
 public:
   static llvm::Optional<MinidumpParser>
   Create(const lldb::DataBufferSP &data_buf_sp);
 
   lldb::offset_t GetByteSize();
 
-  llvm::Optional<llvm::ArrayRef<uint8_t>>
-  GetStream(MinidumpStreamType stream_type);
+  llvm::ArrayRef<uint8_t> GetStream(MinidumpStreamType stream_type);
 
-  llvm::Optional<std::vector<const MinidumpThread *>> GetThreads();
+  llvm::Optional<std::string> GetMinidumpString(uint32_t rva);
+
+  llvm::ArrayRef<MinidumpThread> GetThreads();
 
   const MinidumpSystemInfo *GetSystemInfo();
 
   ArchSpec GetArchitecture();
 
   const MinidumpMiscInfo *GetMiscInfo();
 
+  llvm::Optional<LinuxProcStatus> GetLinuxProcStatus();
+
+  llvm::Optional<lldb::pid_t> GetPid();
+
+  llvm::ArrayRef<MinidumpModule> GetModuleList();
+
+  const MinidumpExceptionStream *GetExceptionStream();
+
 private:
   lldb::DataBufferSP m_data_sp;
   const MinidumpHeader *m_header;
   llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> m_directory_map;
 
-  MinidumpParser(const lldb::DataBufferSP &data_buf_sp,
-                 const MinidumpHeader *header,
-                 const llvm::DenseMap<uint32_t, MinidumpLocationDescriptor>
-                     &directory_map);
+  MinidumpParser(
+      const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
+      llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map);
 };
 
 } // namespace minidump
 } // namespace lldb_private
 #endif // liblldb_MinidumpParser_h_

lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp

-//===-- MinidumpParser.cpp ---------------------------------------*- C++
-//-*-===//
+//===-- MinidumpParser.cpp ---------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
for (uint32_t i = 0; i < header->streams_count; ++i) {
     error = consumeObject(directory_data, directory);
     if (error.Fail()) {
       return llvm::None;
     }
     directory_map[static_cast<const uint32_t>(directory->stream_type)] =
         directory->location;
   }
 
-  MinidumpParser parser(data_buf_sp, header, directory_map);
-  return llvm::Optional<MinidumpParser>(parser);
+  return MinidumpParser(data_buf_sp, header, std::move(directory_map));
 }
 
 MinidumpParser::MinidumpParser(
     const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
-    const llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &directory_map)
+    llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map)
     : m_data_sp(data_buf_sp), m_header(header), m_directory_map(directory_map) {
 }
 
 lldb::offset_t MinidumpParser::GetByteSize() {
   return m_data_sp->GetByteSize();
 }
 
-llvm::Optional<llvm::ArrayRef<uint8_t>>
+llvm::ArrayRef<uint8_t>
 MinidumpParser::GetStream(MinidumpStreamType stream_type) {
   auto iter = m_directory_map.find(static_cast<uint32_t>(stream_type));
   if (iter == m_directory_map.end())
-    return llvm::None;
+    return {};
 
   // check if there is enough data
   if (iter->second.rva + iter->second.data_size > m_data_sp->GetByteSize())
-    return llvm::None;
+    return {};
 
-  llvm::ArrayRef<uint8_t> arr_ref(m_data_sp->GetBytes() + iter->second.rva,
-                                  iter->second.data_size);
-  return llvm::Optional<llvm::ArrayRef<uint8_t>>(arr_ref);
+  return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes() + iter->second.rva,
 }
 
-llvm::Optional<std::vector<const MinidumpThread *>>
-MinidumpParser::GetThreads() {
-  llvm::Optional<llvm::ArrayRef<uint8_t>> data =
-      GetStream(MinidumpStreamType::ThreadList);
+llvm::Optional<std::string> MinidumpParser::GetMinidumpString(uint32_t rva) {
+  auto arr_ref = m_data_sp->GetData();
+  if (rva > arr_ref.size())
+    return llvm::None;
+  arr_ref = arr_ref.drop_front(rva);
+  return parseMinidumpString(arr_ref);
+}
+
+llvm::ArrayRef<MinidumpThread> MinidumpParser::GetThreads() {
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::ThreadList);
 
-  if (!data)
+  if (data.size() == 0)
     return llvm::None;
 
-  return MinidumpThread::ParseThreadList(data.getValue());
+  return MinidumpThread::ParseThreadList(data);
 }
 
 const MinidumpSystemInfo *MinidumpParser::GetSystemInfo() {
-  llvm::Optional<llvm::ArrayRef<uint8_t>> data =
-      GetStream(MinidumpStreamType::SystemInfo);
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::SystemInfo);
 
-  if (!data)
+  if (data.size() == 0)
     return nullptr;
 
-  return MinidumpSystemInfo::Parse(data.getValue());
+  return MinidumpSystemInfo::Parse(data);
 }
 
 ArchSpec MinidumpParser::GetArchitecture() {
   ArchSpec arch_spec;
-  arch_spec.GetTriple().setOS(llvm::Triple::OSType::UnknownOS);
-  arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
-  arch_spec.GetTriple().setArch(llvm::Triple::ArchType::UnknownArch);
-
-  // TODO should we add the OS type here, or somewhere else ?
-
   const MinidumpSystemInfo *system_info = GetSystemInfo();
 
   if (!system_info)
     return arch_spec;
 
   // TODO what to do about big endiand flavors of arm ?
   // TODO set the arm subarch stuff if the minidump has info about it
 
+  llvm::Triple triple;
+  triple.setVendor(llvm::Triple::VendorType::UnknownVendor);
+
   const MinidumpCPUArchitecture arch =
       static_cast<const MinidumpCPUArchitecture>(
           static_cast<const uint32_t>(system_info->processor_arch));
+
   switch (arch) {
   case MinidumpCPUArchitecture::X86:
-    arch_spec.GetTriple().setArch(llvm::Triple::ArchType::x86);
+    triple.setArch(llvm::Triple::ArchType::x86);
     break;
   case MinidumpCPUArchitecture::AMD64:
-    arch_spec.GetTriple().setArch(llvm::Triple::ArchType::x86_64);
+    triple.setArch(llvm::Triple::ArchType::x86_64);
     break;
   case MinidumpCPUArchitecture::ARM:
-    arch_spec.GetTriple().setArch(llvm::Triple::ArchType::arm);
+    triple.setArch(llvm::Triple::ArchType::arm);
     break;
   case MinidumpCPUArchitecture::ARM64:
-    arch_spec.GetTriple().setArch(llvm::Triple::ArchType::aarch64);
+    triple.setArch(llvm::Triple::ArchType::aarch64);
+    break;
+  default:
+    triple.setArch(llvm::Triple::ArchType::UnknownArch);
     break;
   default:
     break;
   }
 
+  const MinidumpOSPlatform os = static_cast<const MinidumpOSPlatform>(
+      static_cast<const uint32_t>(system_info->platform_id));
+
+  // TODO add all of the OSes that Minidump/breakpad distinguishes?
+  switch (os) {
+  case MinidumpOSPlatform::Win32S:
+  case MinidumpOSPlatform::Win32Windows:
+  case MinidumpOSPlatform::Win32NT:
+  case MinidumpOSPlatform::Win32CE:
+    triple.setOS(llvm::Triple::OSType::Win32);
+    break;
+  case MinidumpOSPlatform::Linux:
+    triple.setOS(llvm::Triple::OSType::Linux);
+    break;
+  case MinidumpOSPlatform::MacOSX:
+    triple.setOS(llvm::Triple::OSType::MacOSX);
+    break;
+  case MinidumpOSPlatform::Android:
+    triple.setOS(llvm::Triple::OSType::Linux);
+    triple.setEnvironment(llvm::Triple::EnvironmentType::Android);
+    break;
+  default:
+    triple.setOS(llvm::Triple::OSType::UnknownOS);
+    break;
+  }
+
+  arch_spec.SetTriple(triple);
+
   return arch_spec;
 }
 
 const MinidumpMiscInfo *MinidumpParser::GetMiscInfo() {
-  llvm::Optional<llvm::ArrayRef<uint8_t>> data =
-      GetStream(MinidumpStreamType::MiscInfo);
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::MiscInfo);
+
+  if (data.size() == 0)
+    return nullptr;
+
+  return MinidumpMiscInfo::Parse(data);
+}
+
+llvm::Optional<LinuxProcStatus> MinidumpParser::GetLinuxProcStatus() {
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::LinuxProcStatus);
+
+  if (data.size() == 0)
+    return llvm::None;
+
+  return LinuxProcStatus::Parse(data);
+}
+
+llvm::Optional<lldb::pid_t> MinidumpParser::GetPid() {
+  const MinidumpMiscInfo *misc_info = GetMiscInfo();
+  if (misc_info != nullptr) {
+    return misc_info->GetPid();
+  }
+
+  llvm::Optional<LinuxProcStatus> proc_status = GetLinuxProcStatus();
+  if (proc_status.hasValue()) {
+    return proc_status->GetPid();
+  }
+
+  return llvm::None;
+}
+
+llvm::ArrayRef<MinidumpModule> MinidumpParser::GetModuleList() {
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::ModuleList);
+
+  if (data.size() == 0)
+    return {};
+
+  return MinidumpModule::ParseModuleList(data);
+}
+
+const MinidumpExceptionStream *MinidumpParser::GetExceptionStream() {
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::Exception);
 
-  if (!data)
+  if (data.size() == 0)
     return nullptr;
 
-  return MinidumpMiscInfo::Parse(data.getValue());
+  return MinidumpExceptionStream::Parse(data);
 }

lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.h

 // Project includes
 
 // Other libraries and framework includes
 #include "lldb/Core/Error.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Endian.h"
 
 // C includes
 // C++ includes
 
 // Reference:
 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms679293(v=vs.85).aspx
 // https://chromium.googlesource.com/breakpad/breakpad/
enum class MinidumpPCPUInformationARMElfHwCaps : uint32_t {
   VFPv3D16 = (1 << 14),
   TLS = (1 << 15),
   VFPv4 = (1 << 16),
   IDIVA = (1 << 17),
   IDIVT = (1 << 18),
   LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ IDIVT)
 };
 
+enum class MinidumpMiscInfoFlags : uint32_t {
+  ProcessID = (1 << 0),
+  ProcessTimes = (1 << 1),
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ ProcessTimes)
+};
+
 template <typename T>
 Error consumeObject(llvm::ArrayRef<uint8_t> &Buffer, const T *&Object) {
   Error error;
   if (Buffer.size() < sizeof(T)) {
     error.SetErrorString("Insufficient buffer!");
     return error;
   }
 
   Object = reinterpret_cast<const T *>(Buffer.data());
   Buffer = Buffer.drop_front(sizeof(T));
   return error;
 }
 
+// parse a MinidumpString which is with UTF-16
+// Reference:
+// https://msdn.microsoft.com/en-us/library/windows/desktop/ms680395(v=vs.85).aspx
+llvm::Optional<std::string> parseMinidumpString(llvm::ArrayRef<uint8_t> &data);
+
 // Reference:
 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680378(v=vs.85).aspx
 struct MinidumpHeader {
   llvm::support::ulittle32_t signature;
   llvm::support::ulittle32_t
       version; // The high 16 bits of version field are implementation specific
   llvm::support::ulittle32_t streams_count;
   llvm::support::ulittle32_t
struct MinidumpThread {
   llvm::support::ulittle32_t priority_class;
   llvm::support::ulittle32_t priority;
   llvm::support::ulittle64_t teb;
   MinidumpMemoryDescriptor stack;
   MinidumpLocationDescriptor thread_context;
 
   static const MinidumpThread *Parse(llvm::ArrayRef<uint8_t> &data);
 
-  static llvm::Optional<std::vector<const MinidumpThread *>>
+  static llvm::ArrayRef<MinidumpThread>
   ParseThreadList(llvm::ArrayRef<uint8_t> &data);
 };
 static_assert(sizeof(MinidumpThread) == 48,
               "sizeof MinidumpThread is not correct!");
 
 // Reference:
 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680396(v=vs.85).aspx
 union MinidumpCPUInfo {
struct MinidumpSystemInfo {
 
   MinidumpCPUInfo cpu;
 
   static const MinidumpSystemInfo *Parse(llvm::ArrayRef<uint8_t> &data);
 };
 static_assert(sizeof(MinidumpSystemInfo) == 56,
               "sizeof MinidumpSystemInfo is not correct!");
 
-// TODO check flags to see what's valid
 // TODO misc2, misc3 ?
 // Reference:
 // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680389(v=vs.85).aspx
 struct MinidumpMiscInfo {
   llvm::support::ulittle32_t size;
+  // flags1 represents what info in the struct is valid
   llvm::support::ulittle32_t flags1;
   llvm::support::ulittle32_t process_id;
   llvm::support::ulittle32_t process_create_time;
   llvm::support::ulittle32_t process_user_time;
   llvm::support::ulittle32_t process_kernel_time;
 
   static const MinidumpMiscInfo *Parse(llvm::ArrayRef<uint8_t> &data);
+
+  llvm::Optional<lldb::pid_t> GetPid() const;
 };
 static_assert(sizeof(MinidumpMiscInfo) == 24,
               "sizeof MinidumpMiscInfo is not correct!");
 
+// The /proc/pid/status is saved as an ascii string in the file
+class LinuxProcStatus {
+public:
+  llvm::StringRef proc_status;
+  lldb::pid_t pid;
+
+  static llvm::Optional<LinuxProcStatus> Parse(llvm::ArrayRef<uint8_t> &data);
+
+  lldb::pid_t GetPid() const;
+
+private:
+  LinuxProcStatus() = default;
+};
+
+// MinidumpModule stuff
+struct MinidumpVSFixedFileInfo {
+  llvm::support::ulittle32_t signature;
+  llvm::support::ulittle32_t struct_version;
+  llvm::support::ulittle32_t file_version_hi;
+  llvm::support::ulittle32_t file_version_lo;
+  llvm::support::ulittle32_t product_version_hi;
+  llvm::support::ulittle32_t product_version_lo;
+  // file_flags_mask - identifies valid bits in fileFlags
+  llvm::support::ulittle32_t file_flags_mask;
+  llvm::support::ulittle32_t file_flags;
+  llvm::support::ulittle32_t file_os;
+  llvm::support::ulittle32_t file_type;
+  llvm::support::ulittle32_t file_subtype;
+  llvm::support::ulittle32_t file_date_hi;
+  llvm::support::ulittle32_t file_date_lo;
+};
+static_assert(sizeof(MinidumpVSFixedFileInfo) == 52,
+              "sizeof MinidumpVSFixedFileInfo is not correct!");
+
+struct MinidumpModule {
+  llvm::support::ulittle64_t base_of_image;
+  llvm::support::ulittle32_t size_of_image;
+  llvm::support::ulittle32_t checksum;
+  llvm::support::ulittle32_t time_date_stamp;
+  llvm::support::ulittle32_t module_name_rva;
+  MinidumpVSFixedFileInfo version_info;
+  MinidumpLocationDescriptor CV_record;
+  MinidumpLocationDescriptor misc_record;
+  llvm::support::ulittle32_t reserved0[2];
+  llvm::support::ulittle32_t reserved1[2];
+
+  static const MinidumpModule *Parse(llvm::ArrayRef<uint8_t> &data);
+
+  static llvm::ArrayRef<MinidumpModule>
+  ParseModuleList(llvm::ArrayRef<uint8_t> &data);
+};
+static_assert(sizeof(MinidumpModule) == 108,
+              "sizeof MinidumpVSFixedFileInfo is not correct!");
+
+// Exception stuff
+struct MinidumpException {
+  enum {
+    MaxParams = 15,
+  };
+
+  llvm::support::ulittle32_t exception_code;
+  llvm::support::ulittle32_t exception_flags;
+  llvm::support::ulittle64_t exception_record;
+  llvm::support::ulittle64_t exception_address;
+  llvm::support::ulittle32_t number_parameters;
+  llvm::support::ulittle32_t unused_alignment;
+  llvm::support::ulittle64_t exception_information[MaxParams];
+};
+static_assert(sizeof(MinidumpException) == 152,
+              "sizeof MinidumpException is not correct!");
+
+struct MinidumpExceptionStream {
+  llvm::support::ulittle32_t thread_id;
+  llvm::support::ulittle32_t alignment;
+  MinidumpException exception_record;
+  MinidumpLocationDescriptor thread_context;
+
+  static const MinidumpExceptionStream *Parse(llvm::ArrayRef<uint8_t> &data);
+};
+static_assert(sizeof(MinidumpExceptionStream) == 168,
+              "sizeof MinidumpExceptionStream is not correct!");
+
 } // namespace minidump
 } // namespace lldb_private
 #endif // liblldb_MinidumpTypes_h_

lldb/trunk/source/Plugins/Process/minidump/MinidumpTypes.cpp

 //===-- MinidumpTypes.cpp ---------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // Project includes
 #include "MinidumpTypes.h"
-#include "MinidumpParser.h"
 
 // Other libraries and framework includes
 // C includes
 // C++ includes
 
 using namespace lldb_private;
 using namespace minidump;
 
if (error.Fail() || signature != MinidumpHeaderConstants::Signature ||
     return nullptr;
 
   // TODO check for max number of streams ?
   // TODO more sanity checks ?
 
   return header;
 }
 
+// Minidump string
+llvm::Optional<std::string>
+lldb_private::minidump::parseMinidumpString(llvm::ArrayRef<uint8_t> &data) {
+  std::string result;
+
+  const uint32_t *source_length;
+  Error error = consumeObject(data, source_length);
+  if (error.Fail() || *source_length > data.size() || *source_length % 2 != 0)
+    return llvm::None;
+
+  auto source_start = reinterpret_cast<const UTF16 *>(data.data());
+  // source_length is the length of the string in bytes
+  // we need the length of the string in UTF-16 characters/code points (16 bits
+  // per char)
+  // that's why it's divided by 2
+  const auto source_end = source_start + (*source_length) / 2;
+  // resize to worst case length
+  result.resize(UNI_MAX_UTF8_BYTES_PER_CODE_POINT * (*source_length) / 2);
+  auto result_start = reinterpret_cast<UTF8 *>(&result[0]);
+  const auto result_end = result_start + result.size();
+  ConvertUTF16toUTF8(&source_start, source_end, &result_start, result_end,
+                     strictConversion);
+  const auto result_size =
+      std::distance(reinterpret_cast<UTF8 *>(&result[0]), result_start);
+  result.resize(result_size); // shrink to actual length
+
+  return result;
+}
+
 // MinidumpThread
 const MinidumpThread *MinidumpThread::Parse(llvm::ArrayRef<uint8_t> &data) {
   const MinidumpThread *thread = nullptr;
   Error error = consumeObject(data, thread);
   if (error.Fail())
     return nullptr;
 
   return thread;
 }
 
-llvm::Optional<std::vector<const MinidumpThread *>>
+llvm::ArrayRef<MinidumpThread>
 MinidumpThread::ParseThreadList(llvm::ArrayRef<uint8_t> &data) {
-  std::vector<const MinidumpThread *> thread_list;
-
   const llvm::support::ulittle32_t *thread_count;
   Error error = consumeObject(data, thread_count);
-  if (error.Fail())
-    return llvm::None;
-
-  const MinidumpThread *thread;
-  for (uint32_t i = 0; i < *thread_count; ++i) {
-    thread = MinidumpThread::Parse(data);
-    if (thread == nullptr)
-      return llvm::None;
-    thread_list.push_back(thread);
-  }
+  if (error.Fail() || *thread_count * sizeof(MinidumpThread) > data.size())
+    return {};
 
-  return llvm::Optional<std::vector<const MinidumpThread *>>(thread_list);
+  return llvm::ArrayRef<MinidumpThread>(
+      reinterpret_cast<const MinidumpThread *>(data.data()), *thread_count);
 }
 
 // MinidumpSystemInfo
 const MinidumpSystemInfo *
 MinidumpSystemInfo::Parse(llvm::ArrayRef<uint8_t> &data) {
   const MinidumpSystemInfo *system_info;
   Error error = consumeObject(data, system_info);
   if (error.Fail())
     return nullptr;
 
   return system_info;
 }
 
 // MinidumpMiscInfo
 const MinidumpMiscInfo *MinidumpMiscInfo::Parse(llvm::ArrayRef<uint8_t> &data) {
   const MinidumpMiscInfo *misc_info;
   Error error = consumeObject(data, misc_info);
   if (error.Fail())
     return nullptr;
 
   return misc_info;
 }
+
+llvm::Optional<lldb::pid_t> MinidumpMiscInfo::GetPid() const {
+  uint32_t pid_flag =
+      static_cast<const uint32_t>(MinidumpMiscInfoFlags::ProcessID);
+  if (flags1 & pid_flag)
+    return llvm::Optional<lldb::pid_t>(process_id);
+
+  return llvm::None;
+}
+
+// Linux Proc Status
+// it's stored as an ascii string in the file
+llvm::Optional<LinuxProcStatus>
+LinuxProcStatus::Parse(llvm::ArrayRef<uint8_t> &data) {
+  LinuxProcStatus result;
+  result.proc_status =
+      llvm::StringRef(reinterpret_cast<const char *>(data.data()), data.size());
+  data = data.drop_front(data.size());
+
+  llvm::SmallVector<llvm::StringRef, 0> lines;
+  result.proc_status.split(lines, '\n', 42);
+  // /proc/$pid/status has 41 lines, but why not use 42?
+  for (auto line : lines) {
+    if (line.consume_front("Pid:")) {
+      line = line.trim();
+      if (!line.getAsInteger(10, result.pid))
+        return result;
+    }
+  }
+
+  return llvm::None;
+}
+
+lldb::pid_t LinuxProcStatus::GetPid() const { return pid; }
+
+// Module stuff
+const MinidumpModule *MinidumpModule::Parse(llvm::ArrayRef<uint8_t> &data) {
+  const MinidumpModule *module = nullptr;
+  Error error = consumeObject(data, module);
+  if (error.Fail())
+    return nullptr;
+
+  return module;
+}
+
+llvm::ArrayRef<MinidumpModule>
+MinidumpModule::ParseModuleList(llvm::ArrayRef<uint8_t> &data) {
+
+  const llvm::support::ulittle32_t *modules_count;
+  Error error = consumeObject(data, modules_count);
+  if (error.Fail() || *modules_count * sizeof(MinidumpModule) > data.size())
+    return {};
+
+  return llvm::ArrayRef<MinidumpModule>(
+      reinterpret_cast<const MinidumpModule *>(data.data()), *modules_count);
+}
+
+// Exception stuff
+const MinidumpExceptionStream *
+MinidumpExceptionStream::Parse(llvm::ArrayRef<uint8_t> &data) {
+  const MinidumpExceptionStream *exception_stream = nullptr;
+  Error error = consumeObject(data, exception_stream);
+  if (error.Fail())
+    return nullptr;
+
+  return exception_stream;
+}

lldb/trunk/unittests/Process/minidump/CMakeLists.txt

 add_lldb_unittest(LLDBMinidumpTests
   MinidumpParserTest.cpp
   )
 
 set(test_inputs
-   linux-x86_64.dmp)
+   linux-x86_64.dmp
+   fizzbuzz_no_heap.dmp)
 
 add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")

lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

public:
   }
 
   llvm::SmallString<128> inputs_folder;
   std::unique_ptr<MinidumpParser> parser;
 };
 
 TEST_F(MinidumpParserTest, GetThreads) {
   SetUpData("linux-x86_64.dmp");
-  llvm::Optional<std::vector<const MinidumpThread *>> thread_list;
+  llvm::ArrayRef<MinidumpThread> thread_list;
 
   thread_list = parser->GetThreads();
-  ASSERT_TRUE(thread_list.hasValue());
-  ASSERT_EQ(1UL, thread_list->size());
+  ASSERT_EQ(1UL, thread_list.size());
 
-  const MinidumpThread *thread = thread_list.getValue()[0];
-  ASSERT_EQ(16001UL, thread->thread_id);
+  const MinidumpThread thread = thread_list[0];
+  ASSERT_EQ(16001UL, thread.thread_id);
 }
 
 TEST_F(MinidumpParserTest, GetThreadsTruncatedFile) {
   SetUpData("linux-x86_64.dmp", 200);
-  llvm::Optional<std::vector<const MinidumpThread *>> thread_list;
+  llvm::ArrayRef<MinidumpThread> thread_list;
 
   thread_list = parser->GetThreads();
-  ASSERT_FALSE(thread_list.hasValue());
+  ASSERT_EQ(0UL, thread_list.size());
 }
 
 TEST_F(MinidumpParserTest, GetArchitecture) {
   SetUpData("linux-x86_64.dmp");
   ASSERT_EQ(llvm::Triple::ArchType::x86_64,
-            parser->GetArchitecture().GetTriple().getArch());
+            parser->GetArchitecture().GetMachine());
+  ASSERT_EQ(llvm::Triple::OSType::Linux,
+            parser->GetArchitecture().GetTriple().getOS());
 }
 
 TEST_F(MinidumpParserTest, GetMiscInfo) {
   SetUpData("linux-x86_64.dmp");
   const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
   ASSERT_EQ(nullptr, misc_info);
-  // linux breakpad generated minidump files don't have misc info stream
+}
+
+TEST_F(MinidumpParserTest, GetLinuxProcStatus) {
+  SetUpData("linux-x86_64.dmp");
+  llvm::Optional<LinuxProcStatus> proc_status = parser->GetLinuxProcStatus();
+  ASSERT_TRUE(proc_status.hasValue());
+  lldb::pid_t pid = proc_status->GetPid();
+  ASSERT_EQ(16001UL, pid);
+}
+
+TEST_F(MinidumpParserTest, GetPid) {
+  SetUpData("linux-x86_64.dmp");
+  llvm::Optional<lldb::pid_t> pid = parser->GetPid();
+  ASSERT_TRUE(pid.hasValue());
+  ASSERT_EQ(16001UL, pid.getValue());
+}
+
+TEST_F(MinidumpParserTest, GetModuleList) {
+  SetUpData("linux-x86_64.dmp");
+  llvm::ArrayRef<MinidumpModule> modules = parser->GetModuleList();
+  ASSERT_EQ(8UL, modules.size());
+  std::string module_names[8] = {
+      "/usr/local/google/home/dvlahovski/projects/test_breakpad/a.out",
+      "/lib/x86_64-linux-gnu/libm-2.19.so",
+      "/lib/x86_64-linux-gnu/libc-2.19.so",
+      "/lib/x86_64-linux-gnu/libgcc_s.so.1",
+      "/usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.19",
+      "/lib/x86_64-linux-gnu/libpthread-2.19.so",
+      "/lib/x86_64-linux-gnu/ld-2.19.so",
+      "linux-gate.so",
+  };
+
+  for (int i = 0; i < 8; ++i) {
+    llvm::Optional<std::string> name =
+        parser->GetMinidumpString(modules[i].module_name_rva);
+    ASSERT_TRUE(name.hasValue());
+    ASSERT_EQ(module_names[i], name.getValue());
+  }
+}
+
+TEST_F(MinidumpParserTest, GetExceptionStream) {
+  SetUpData("linux-x86_64.dmp");
+  const MinidumpExceptionStream *exception_stream =
+      parser->GetExceptionStream();
+  ASSERT_NE(nullptr, exception_stream);
+  ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
+}
+
+// Windows Minidump tests
+// fizzbuzz_no_heap.dmp is copied from the WinMiniDump tests
+TEST_F(MinidumpParserTest, GetArchitectureWindows) {
+  SetUpData("fizzbuzz_no_heap.dmp");
+  ASSERT_EQ(llvm::Triple::ArchType::x86,
+            parser->GetArchitecture().GetMachine());
+  ASSERT_EQ(llvm::Triple::OSType::Win32,
+            parser->GetArchitecture().GetTriple().getOS());
+}
+
+TEST_F(MinidumpParserTest, GetLinuxProcStatusWindows) {
+  SetUpData("fizzbuzz_no_heap.dmp");
+  llvm::Optional<LinuxProcStatus> proc_status = parser->GetLinuxProcStatus();
+  ASSERT_FALSE(proc_status.hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetMiscInfoWindows) {
+  SetUpData("fizzbuzz_no_heap.dmp");
+  const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
+  ASSERT_NE(nullptr, misc_info);
+  llvm::Optional<lldb::pid_t> pid = misc_info->GetPid();
+  ASSERT_TRUE(pid.hasValue());
+  ASSERT_EQ(4440UL, pid.getValue());
+}
+
+TEST_F(MinidumpParserTest, GetPidWindows) {
+  SetUpData("fizzbuzz_no_heap.dmp");
+  llvm::Optional<lldb::pid_t> pid = parser->GetPid();
+  ASSERT_TRUE(pid.hasValue());
+  ASSERT_EQ(4440UL, pid.getValue());
 }