Adding a RegisterContextMinidump_x86_64 converter

[dvlahovski]

Summary

This is a register context converter from Minidump to Linux reg context.
This knows the layout of the register context in the Minidump file
(which is the same as in Windows FYI) and as a result emits a binary data
buffer that matches the Linux register context binary layout.
This way we can reuse the existing RegisterContextLinux_x86_64 and
RegisterContextCorePOSIX_x86_64 classes.

[dvlahovski]

I tested this on the yet-to-be-submitted plugin code and it works fine on Linux (should work fine everywhere for that matter). I can do a backtrace, go up/down the stack frames and, of course, see all of the registers with register read

[labath]

source/Plugins/Process/minidump/MinidumpParser.h

42

Replace these two functions with llvm::ArrayRef<uint8_t> GetData() const

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h

56

This function does not reflect the actual memory layout of the data, and you are not using it. I think it should go away.

162

This looks like a private utility function. It should not be in the header.

187

Could you add // end namespace minidump comments. Without them it's hard to tell what is going on here.

unittests/Process/minidump/MinidumpParserTest.cpp

176

static

Also i think having the register tests in this file is fine.

[zturner]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

47–48

Can this be a const MinidumpContext_x86_64_Flags*? Then you wouldn't have to do the uint32_t conversion each time since you could just & the flag you're checking directly against this value.

49

Can we use a sizeof() here?

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h

38

Should these be endian specific types like ulittle16_t? If someone writes a minidump on a little endian system and runs this code on a big endian system, it would fail to parse the minidump correctly.

162

The order of parameters here is confusing. the output buffer should be adjacent to its length. So it should be:

void writeRegister(ArrayRef<>, uint8_t*, size_t, const RegisterInfo*)

That said, passing around pointers and lengths is a pattern that needs to die. I would much rather this be:

void writeRegister(ArrayRef<uint8_t> &reg_src, MutableArrayRef<uint8_t> reg_dest, const RegisterInfo &reg);

unittests/Process/minidump/MinidumpParserTest.cpp

177

I would make the conversion a macro like this:

#define REG_VAL(x) *(reinterpret_cast<uint64_t *>(x))

Then write the asserts inline. The reason for this is that when you do it this way, when a test fails the test runner will report that the failure happened in registerEqualToVal, which won't give you any clue about what test it failed in. if the assertion is kept in the body of the test, the failure will report which test it was in, so you can more easily see.

[dvlahovski]

Updating the CL regarding Pavel's comments

[dvlahovski]

I will fix the comments that Zachary made in the next revision

[labath]

lgtm, after Zachary is happy.

[dvlahovski]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

48–49

If it is then when I do a & the result is an enum class of type MinidumpContext_x86_64_Flags. And the compiler complains that this is not convertible to bool

[dvlahovski]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

50

sizeof(uint16_t), sizeof(uint32_t), etc ?

[zturner]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

50

I think my comments got out of line and this is no longer at the position I suggested it. that said, I actually didn't notice all these constants being passed into writeRegister. I would propose changing this. Find the RegisterInfo class in lldb-private-types.h and add the following methods:

llvm::ArrayRef<uint8_t> data(const uint8_t *context_base) const { 
  return llvm::ArrayRef<uint8_t>(context_base + byte_offset, byte_size);
}

llvm::MutableArrayRef<uint8_t> mutable_data(uint8_t *context_base) const {
  return llvm::MutableArrayRef<uint8_t>(context_base + byte_offset, byte_size);
}

Then you can write:

writeRegister(source_data, reg_info[lldb_cs_x86_64].mutable_data(result_base));

[amccarth]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

2

Should match file name.

32

It might be better to put this comment with the declaration in the header file, since it explains what to pass in and what it does. Comments in .cpp files should contain implementation details that the callers don't necessarily need to know.

48–49

I think what Zach means is that you could locally define a uint32_t const, initialized with the value from the enum. Then each if statement could use that constant without a cast.

Also, is this right? MinidumpContext_x86_x64_Flags::Control has two bits set, so the condition will be true if either of them is set. Is that the intended behavior? Or should you be ensuring that they're both set like this:

const utin32_t ControlFlags = MinidumpContext_x86_64::Control;
if ((*context_flags & ControlFlags) == ControlFlags) {
  ...
}

?

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h

2

Please make this line match the file name.

unittests/Process/minidump/MinidumpParserTest.cpp

177

+1 to Zach's idea.

Also, consider whether EXPECT_EQ is more appropriate than ASSERT_EQ for these.

[dvlahovski]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

48–49

Now that I think about I should check the arch flag bit at the beggining of this.
But then this if's are OK I think - in them I only want to check if that specific bit is set.

[dvlahovski]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

50

This will make the code very clean indeed, but there is a problem.

From the RegisterInfo I only get the byte_offset and I don't use the byte_size because some of the registers in the Minidump context are smaller (e.g. cs is 16 bit). All of the registers in the Linux context are 64 bits in size. So that's why I basically hard-coded the size in each of the funciton calls. Now with the next revision I tried to make the code clearer.

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h

39

So I had the actual struct for the register context in the previous revision, but now I've removed it, because I actually don't use it. So added a comment describing it's layout.

For the endianness part - because this code only permutes the bytes in the layout the Linux register context expects them, I don't think that here is the place to handle that. I was hoping that the Linux register context will handle it, but I don't think that's the case when I look at the code in RegisterContextCorePOSIX_x86_64::ReadRegister.

Am I right - do the Linux context assume little endian? If so maybe I can also fix that.

unittests/Process/minidump/MinidumpParserTest.cpp

177

Probably EXPECT_EQ is better because if there is a mismatch in the reg values, one can see all of the values.

I still don't have the sense when to use EXPECT_EQ and when ASSERT_EQ.

[dvlahovski]

Addressing Zachary's and Adrian's comments

[zturner]

ASSERT_EQ causes the test method to return immediately if the condition fails. EXPECT_EQ will continue running the same test body. I like to use ASSERT_EQ, for example, if you are checking the value of a pointer returned by a function. First you want to check if it's null, then you want to check that its value is the value you expect. So I would do

ASSERT_NE(p, nullptr);
EXPECT_EQ(*p, 42);

Because if it's null, the rest of the statements in the test body are undefined.

Another example is if a method returns some value, say number of items in a list, and then you want to call another method to process that many items. Say:

int x = foo.GetItemCount();
const Item *items = foo.GetItems();
bool Success = ProcessItems(items, x);

If the number of items is not what you expect, it doesn't make sense to run the next statement. So you could write:

int x = foo.GetItemCount();
ASSERT_EQ(42, x);
const Item *items = foo.GetItems();
EXPECT_TRUE(ProcessItems(items, x));

On the other hand, if each thing you are testing is independent of the previous statement, everything can be expect. For example:

EXPECT_EQ("0", Foo(0));
EXPECT_EQ("1", Foo(1));
EXPECT_EQ("2", Foo(2));
EXPECT_EQ("3", Foo(3));

This way you can see as much diagnostic information as possible in the output (for example you might be able to spot a pattern in which values are failing.

[dvlahovski]

Thanks for the explanation and examples! :) Will keep this in mind in the future.

[amccarth]

unittests/Process/minidump/MinidumpParserTest.cpp

177

EXPECT_xxx will check the condition and report if it's wrong. But then the test proceeds.

ASSERT_xxx will check the condition, and, if it's wrong, it will report _and_ terminate the current test.

So my rule of thumb is:

1. Prefer EXPECT_xxx when checking that the code does what it's supposed to do.
2. Use ASSERT_xxx when the rest of the test depends on this condition.

[dvlahovski]

unittests/Process/minidump/MinidumpParserTest.cpp

177

Thanks :)

[zturner]

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

51

If they are just smaller (but never bigger), you can call take_front(N) on the result. So you could say:

writeRegister(source_data, reg_info[lldb_cs_x86_64].mutable_data(result_base).take_front(2));

Now the line is starting to get long so might look ugly if it wraps, so it's up to you. It is somewhat safer at least since you will assert if you accidentally specify an invalid size, and you don't have to worry about getting the computation right.

Perhaps you could combine this with the approach you've already taken here. Add the functions to RegisterInfo, then change getDestRegister to look like this:

llvm::MutableArrayRef<uint8_t> getDestRegister(const RegisterInfo &reg,
                                               uint8_t *context, uint32_t lldb_reg_num) {
  auto bytes = reg.mutable_data(context);

  switch (lldb_reg_num) {
  case lldb_cs_x86_64:
  case lldb_ds_x86_64:
  case lldb_es_x86_64:
  case lldb_fs_x86_64:
  case lldb_gs_x86_64:
  case lldb_ss_x86_64:
    return bytes.take_front(2);
    break;
  case lldb_rflags_x86_64:
    return bytes.take_front(4);
    break;
  default:
    return bytes.take_front(8);
}

then you could call writeRegister like this:

writeRegister(source_data, getDestRegister(reg_info, result_base, lldb_cs_x86_64));

At this point though I'm not sure what's better. So don't consider this a requirement, up to you :)

[dvlahovski]

I like the combined approach more. So this is the implementation

include/lldb/lldb-private-types.h

 //===-- lldb-private-types.h ------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_lldb_private_types_h_
 #define liblldb_lldb_private_types_h_
 
 #if defined(__cplusplus)
 
 #include "lldb/lldb-private.h"
 
+#include "llvm/ADT/ArrayRef.h"
+
 namespace llvm {
 namespace sys {
 class DynamicLibrary;
 }
 }
 
 namespace lldb_private {
 class Platform;
struct RegisterInfo {
   // null, all registers in this list will be invalidated when the value of this
   // register changes.  For example, the invalidate list for eax would be rax
   // ax, ah, and al.
   const uint8_t *dynamic_size_dwarf_expr_bytes; // A DWARF expression that when
                                                 // evaluated gives
   // the byte size of this register.
   size_t dynamic_size_dwarf_len; // The length of the DWARF expression in bytes
                                  // in the dynamic_size_dwarf_expr_bytes member.
+
+  llvm::ArrayRef<uint8_t> data(const uint8_t *context_base) const {
+    return llvm::ArrayRef<uint8_t>(context_base + byte_offset, byte_size);
+  }
+
+  llvm::MutableArrayRef<uint8_t> mutable_data(uint8_t *context_base) const {
+    return llvm::MutableArrayRef<uint8_t>(context_base + byte_offset,
+                                          byte_size);
+  }
 };
 
 //----------------------------------------------------------------------
 // Registers are grouped into register sets
 //----------------------------------------------------------------------
 struct RegisterSet {
   const char *name;          // Name of this register set
   const char *short_name;    // A short name for this register set

source/Plugins/Process/minidump/CMakeLists.txt

 include_directories(../Utility)
 
 add_lldb_library(lldbPluginProcessMinidump
   MinidumpTypes.cpp
   MinidumpParser.cpp
+  RegisterContextMinidump_x86_64.cpp
   )

source/Plugins/Process/minidump/MinidumpParser.h

 
 namespace minidump {
 
 class MinidumpParser {
 public:
   static llvm::Optional<MinidumpParser>
   Create(const lldb::DataBufferSP &data_buf_sp);
 
-  lldb::offset_t GetByteSize();
+  llvm::ArrayRef<uint8_t> GetData();
 
   llvm::ArrayRef<uint8_t> GetStream(MinidumpStreamType stream_type);
 
   llvm::Optional<std::string> GetMinidumpString(uint32_t rva);
 
   llvm::ArrayRef<MinidumpThread> GetThreads();
 
   const MinidumpSystemInfo *GetSystemInfo();
private:
   const MinidumpHeader *m_header;
   llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> m_directory_map;
 
   MinidumpParser(
       const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
       llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &&directory_map);
 };
 
-} // namespace minidump
-} // namespace lldb_private
+} // end namespace minidump
+} // end namespace lldb_private
 #endif // liblldb_MinidumpParser_h_

source/Plugins/Process/minidump/MinidumpParser.cpp

 }
 
 MinidumpParser::MinidumpParser(
     const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header,
     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::ArrayRef<uint8_t> MinidumpParser::GetData() {
+  return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(),
+                                 m_data_sp->GetByteSize());
 }
 
 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 {};
 

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h

+//===-- RegisterContextMinidump_x86_64.h ------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef liblldb_RegisterContextMinidump_h_
+#define liblldb_RegisterContextMinidump_h_
+
+// Project includes
+#include "MinidumpTypes.h"
+
+// Other libraries and framework includes
+#include "Plugins/Process/Utility/RegisterInfoInterface.h"
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+
+#include "lldb/Target/RegisterContext.h"
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitmaskEnum.h"
+
+// C includes
+// C++ includes
+
+namespace lldb_private {
+
+namespace minidump {
+
+// The content of the Minidump register context is as follows:
+// (for reference see breakpad's source or WinNT.h)
+// Register parameter home addresses: (p1_home .. p6_home)
+// - uint64_t p1_home
+// - uint64_t p2_home
+// - uint64_t p3_home
+// - uint64_t p4_home
+// - uint64_t p5_home
+// - uint64_t p6_home
+//
+// - uint32_t context_flags - field that determines the layout of the structure
+//     and which parts of it are populated
+// - uint32_t mx_csr
+//
+// - uint16_t cs - included if MinidumpContext_x86_64_Flags::Control
+//
+// - uint16_t ds - included if MinidumpContext_x86_64_Flags::Segments
+// - uint16_t es - included if MinidumpContext_x86_64_Flags::Segments
+// - uint16_t fs - included if MinidumpContext_x86_64_Flags::Segments
+// - uint16_t gs - included if MinidumpContext_x86_64_Flags::Segments
+//
+// - uint16_t ss     - included if MinidumpContext_x86_64_Flags::Control
+// - uint32_t rflags - included if MinidumpContext_x86_64_Flags::Control
+//
+// Debug registers: (included if MinidumpContext_x86_64_Flags::DebugRegisters)
+// - uint64_t dr0
+// - uint64_t dr1
+// - uint64_t dr2
+// - uint64_t dr3
+// - uint64_t dr6
+// - uint64_t dr7
+//
+// The next 4 registers are included if MinidumpContext_x86_64_Flags::Integer
+// - uint64_t rax
+// - uint64_t rcx
+// - uint64_t rdx
+// - uint64_t rbx
+//
+// - uint64_t rsp - included if MinidumpContext_x86_64_Flags::Control
+//
+// The next 11 registers are included if MinidumpContext_x86_64_Flags::Integer
+// - uint64_t rbp
+// - uint64_t rsi
+// - uint64_t rdi
+// - uint64_t r8
+// - uint64_t r9
+// - uint64_t r10
+// - uint64_t r11
+// - uint64_t r12
+// - uint64_t r13
+// - uint64_t r14
+// - uint64_t r15
+//
+// - uint64_t rip - included if MinidumpContext_x86_64_Flags::Control
+//
+// TODO: add floating point registers here
+
+// This function receives an ArrayRef pointing to the bytes of the Minidump
+// register context and returns a DataBuffer that's ordered by the offsets
+// specified in the RegisterInfoInterface argument
+// This way we can reuse the already existing register contexts
+lldb::DataBufferSP
+ConvertMinidumpContextToRegIface(llvm::ArrayRef<uint8_t> source_data,
+                                 RegisterInfoInterface *target_reg_interface);
+
+// For context_flags. These values indicate the type of
+// context stored in the structure.  The high 24 bits identify the CPU, the
+// low 8 bits identify the type of context saved.
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+enum class MinidumpContext_x86_64_Flags : uint32_t {
+  x86_64_Flag = 0x00100000,
+  Control = x86_64_Flag | 0x00000001,
+  Integer = x86_64_Flag | 0x00000002,
+  Segments = x86_64_Flag | 0x00000004,
+  FloatingPoint = x86_64_Flag | 0x00000008,
+  DebugRegisters = x86_64_Flag | 0x00000010,
+  XState = x86_64_Flag | 0x00000040,
+
+  Full = Control | Integer | FloatingPoint,
+  All = Full | Segments | DebugRegisters,
+
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ All)
+};
+
+} // end namespace minidump
+} // end namespace lldb_private
+#endif // liblldb_RegisterContextMinidump_h_

source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp

+//===-- RegisterContextMinidump_x86_64.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 "RegisterContextMinidump_x86_64.h"
+
+// Other libraries and framework includes
+#include "lldb/Core/DataBufferHeap.h"
+
+// C includes
+// C++ includes
+
+using namespace lldb_private;
+using namespace minidump;
+
+void writeRegister(llvm::ArrayRef<uint8_t> &reg_src,
+                   llvm::MutableArrayRef<uint8_t> reg_dest) {
+  memcpy(reg_dest.data(), reg_src.data(), reg_dest.size());
+  reg_src = reg_src.drop_front(reg_dest.size());
+}
+
+llvm::MutableArrayRef<uint8_t> getDestRegister(uint8_t *context,
+                                               uint32_t lldb_reg_num,
+                                               const RegisterInfo &reg) {
+  auto bytes = reg.mutable_data(context);
+
+  switch (lldb_reg_num) {
+  case lldb_cs_x86_64:
+  case lldb_ds_x86_64:
+  case lldb_es_x86_64:
+  case lldb_fs_x86_64:
+  case lldb_gs_x86_64:
+  case lldb_ss_x86_64:
+    return bytes.take_front(2);
+    break;
+  case lldb_rflags_x86_64:
+    return bytes.take_front(4);
+    break;
+  default:
+    return bytes.take_front(8);
+    break;
+  }
+}
+
+lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContextToRegIface(
+    llvm::ArrayRef<uint8_t> source_data,
+    RegisterInfoInterface *target_reg_interface) {
+
+  const RegisterInfo *reg_info = target_reg_interface->GetRegisterInfo();
+
+  lldb::DataBufferSP result_context_buf(
+      new DataBufferHeap(target_reg_interface->GetGPRSize(), 0));
+  uint8_t *result_base = result_context_buf->GetBytes();
+
+  source_data = source_data.drop_front(6 * 8); // p[1-6] home registers
+  const uint32_t *context_flags;
+  consumeObject(source_data, context_flags);
+  const uint32_t x86_64_Flag =
+      static_cast<uint32_t>(MinidumpContext_x86_64_Flags::x86_64_Flag);
+  const uint32_t ControlFlag =
+      static_cast<uint32_t>(MinidumpContext_x86_64_Flags::Control);
+  const uint32_t IntegerFlag =
+      static_cast<uint32_t>(MinidumpContext_x86_64_Flags::Integer);
+  const uint32_t SegmentsFlag =
+      static_cast<uint32_t>(MinidumpContext_x86_64_Flags::Segments);
+  const uint32_t DebugRegistersFlag =
+      static_cast<uint32_t>(MinidumpContext_x86_64_Flags::DebugRegisters);
+
+  if (!(*context_flags & x86_64_Flag)) {
+    return result_context_buf; // error
+  }
+
+  source_data = source_data.drop_front(4); // mx_csr
+
+  if (*context_flags & ControlFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_cs_x86_64,
+                                               reg_info[lldb_cs_x86_64]));
+  }
+
+  if (*context_flags & SegmentsFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_ds_x86_64,
+                                               reg_info[lldb_ds_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_es_x86_64,
+                                               reg_info[lldb_es_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_fs_x86_64,
+                                               reg_info[lldb_fs_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_gs_x86_64,
+                                               reg_info[lldb_gs_x86_64]));
+  }
+
+  if (*context_flags & ControlFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_ss_x86_64,
+                                               reg_info[lldb_ss_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_rflags_x86_64,
+                                               reg_info[lldb_rflags_x86_64]));
+  }
+
+  if (*context_flags & DebugRegistersFlag) {
+    source_data =
+        source_data.drop_front(6 * 8); // 6 debug registers 64 bit each
+  }
+
+  if (*context_flags & IntegerFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_rax_x86_64,
+                                               reg_info[lldb_rax_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_rcx_x86_64,
+                                               reg_info[lldb_rcx_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_rdx_x86_64,
+                                               reg_info[lldb_rdx_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_rbx_x86_64,
+                                               reg_info[lldb_rbx_x86_64]));
+  }
+
+  if (*context_flags & ControlFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_rsp_x86_64,
+                                               reg_info[lldb_rsp_x86_64]));
+  }
+
+  if (*context_flags & IntegerFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_rbp_x86_64,
+                                               reg_info[lldb_rbp_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_rsi_x86_64,
+                                               reg_info[lldb_rsi_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_rdi_x86_64,
+                                               reg_info[lldb_rdi_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r8_x86_64,
+                                               reg_info[lldb_r8_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r9_x86_64,
+                                               reg_info[lldb_r9_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r10_x86_64,
+                                               reg_info[lldb_r10_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r11_x86_64,
+                                               reg_info[lldb_r11_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r12_x86_64,
+                                               reg_info[lldb_r12_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r13_x86_64,
+                                               reg_info[lldb_r13_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r14_x86_64,
+                                               reg_info[lldb_r14_x86_64]));
+    writeRegister(source_data, getDestRegister(result_base, lldb_r15_x86_64,
+                                               reg_info[lldb_r15_x86_64]));
+  }
+
+  if (*context_flags & ControlFlag) {
+    writeRegister(source_data, getDestRegister(result_base, lldb_rip_x86_64,
+                                               reg_info[lldb_rip_x86_64]));
+  }
+
+  // TODO parse the floating point registers
+
+  return result_context_buf;
+}

unittests/Process/minidump/MinidumpParserTest.cpp

 //===-- MinidumpTypesTest.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 "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 #include "Plugins/Process/minidump/MinidumpParser.h"
 #include "Plugins/Process/minidump/MinidumpTypes.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
 
 // Other libraries and framework includes
 #include "gtest/gtest.h"
 
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/DataExtractor.h"
 #include "lldb/Host/FileSpec.h"
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 // C includes
 
 // C++ includes
 #include <memory>
void SetUpData(const char *minidump_filename, size_t load_size = SIZE_MAX) {
     llvm::sys::path::append(filename, minidump_filename);
     FileSpec minidump_file(filename.c_str(), false);
     lldb::DataBufferSP data_sp(
         minidump_file.MemoryMapFileContents(0, load_size));
     llvm::Optional<MinidumpParser> optional_parser =
         MinidumpParser::Create(data_sp);
     ASSERT_TRUE(optional_parser.hasValue());
     parser.reset(new MinidumpParser(optional_parser.getValue()));
-    ASSERT_GT(parser->GetByteSize(), 0UL);
+    ASSERT_GT(parser->GetData().size(), 0UL);
   }
 
   llvm::SmallString<128> inputs_folder;
   std::unique_ptr<MinidumpParser> parser;
 };
 
 TEST_F(MinidumpParserTest, GetThreads) {
   SetUpData("linux-x86_64.dmp");

 }
 
 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());
 }
+
+// Register stuff
+// TODO probably split register stuff tests into different file?
+#define REG_VAL(x) *(reinterpret_cast<uint64_t *>(x))
+
+TEST_F(MinidumpParserTest, ConvertRegisterContext) {
+  SetUpData("linux-x86_64.dmp");
+  llvm::ArrayRef<MinidumpThread> thread_list = parser->GetThreads();
+  const MinidumpThread thread = thread_list[0];
+  llvm::ArrayRef<uint8_t> registers(parser->GetData().data() +
+                                        thread.thread_context.rva,
+                                    thread.thread_context.data_size);
+
+  ArchSpec arch = parser->GetArchitecture();
+  RegisterInfoInterface *reg_interface = new RegisterContextLinux_x86_64(arch);
+  lldb::DataBufferSP buf =
+      ConvertMinidumpContextToRegIface(registers, reg_interface);
+  ASSERT_EQ(reg_interface->GetGPRSize(), buf->GetByteSize());
+
+  const RegisterInfo *reg_info = reg_interface->GetRegisterInfo();
+
+  std::map<uint64_t, uint64_t> reg_values;
+
+  // clang-format off
+  reg_values[lldb_rax_x86_64]    =  0x0000000000000000;
+  reg_values[lldb_rbx_x86_64]    =  0x0000000000000000;
+  reg_values[lldb_rcx_x86_64]    =  0x0000000000000010;
+  reg_values[lldb_rdx_x86_64]    =  0x0000000000000000;
+  reg_values[lldb_rdi_x86_64]    =  0x00007ffceb349cf0;
+  reg_values[lldb_rsi_x86_64]    =  0x0000000000000000;
+  reg_values[lldb_rbp_x86_64]    =  0x00007ffceb34a210;
+  reg_values[lldb_rsp_x86_64]    =  0x00007ffceb34a210;
+  reg_values[lldb_r8_x86_64]     =  0x00007fe9bc1aa9c0;
+  reg_values[lldb_r9_x86_64]     =  0x0000000000000000;
+  reg_values[lldb_r10_x86_64]    =  0x00007fe9bc3f16a0;
+  reg_values[lldb_r11_x86_64]    =  0x0000000000000246;
+  reg_values[lldb_r12_x86_64]    =  0x0000000000401c92;
+  reg_values[lldb_r13_x86_64]    =  0x00007ffceb34a430;
+  reg_values[lldb_r14_x86_64]    =  0x0000000000000000;
+  reg_values[lldb_r15_x86_64]    =  0x0000000000000000;
+  reg_values[lldb_rip_x86_64]    =  0x0000000000401dc6;
+  reg_values[lldb_rflags_x86_64] =  0x0000000000010206;
+  reg_values[lldb_cs_x86_64]     =  0x0000000000000033;
+  reg_values[lldb_fs_x86_64]     =  0x0000000000000000;
+  reg_values[lldb_gs_x86_64]     =  0x0000000000000000;
+  reg_values[lldb_ss_x86_64]     =  0x0000000000000000;
+  reg_values[lldb_ds_x86_64]     =  0x0000000000000000;
+  reg_values[lldb_es_x86_64]     =  0x0000000000000000;
+  // clang-format on
+
+  for (uint32_t reg_index = 0; reg_index < reg_interface->GetRegisterCount();
+       ++reg_index) {
+    if (reg_values.find(reg_index) != reg_values.end()) {
+      EXPECT_EQ(reg_values[reg_index],
+                REG_VAL(buf->GetBytes() + reg_info[reg_index].byte_offset));
+    }
+  }
+}