Remove the last manually constructed packet from gdb-remote register context + small refactor

[labath]

Summary

The tricky part here was that the exisiting implementation of WriteAllRegisters was expecting
hex-encoded data (as that was what the first implementation I replaced was using, but here we had
binary data to begin with. I thought the read/write register functions would be more useful if
they handled the hex-encoding themselves (all the other client functions provide the responses in
a more-or-less digested form). The read functions return a DataBuffer, so they can allocate as
much memory as they need to, while the write functions functions take an llvm::ArrayRef, as that
can be constructed from pretty much anything.

[clayborg]

Looks good!

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

 //===-- GDBRemoteCommunicationClient.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_GDBRemoteCommunicationClient_h_
 #define liblldb_GDBRemoteCommunicationClient_h_
 
+#include "GDBRemoteClientBase.h"
+
 // C Includes
 // C++ Includes
 #include <chrono>
 #include <map>
 #include <mutex>
 #include <string>
 #include <vector>
 
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/StructuredData.h"
 #include "lldb/Target/Process.h"
 
-#include "GDBRemoteClientBase.h"
-
 namespace lldb_private {
 namespace process_gdb_remote {
 
 class GDBRemoteCommunicationClient : public GDBRemoteClientBase
 {
 public:
     GDBRemoteCommunicationClient();
 
RunShellCommand(const char *command,           // Shouldn't be nullptr
                     const FileSpec &working_dir,   // Pass empty FileSpec to use the current working directory
                     int *status_ptr,               // Pass nullptr if you don't want the process exit status
                     int *signo_ptr,                // Pass nullptr if you don't want the signal that caused the process to exit
                     std::string *command_output,   // Pass nullptr if you don't want the command output
                     uint32_t timeout_sec);         // Timeout in seconds to wait for shell program to finish
 
     bool
     CalculateMD5 (const FileSpec& file_spec, uint64_t &high, uint64_t &low);
-    
-
-    bool
+    lldb::DataBufferSP
     ReadRegister(lldb::tid_t tid,
-                 uint32_t reg_num,   // Must be the eRegisterKindProcessPlugin register number, to be sent to the remote
-                 StringExtractorGDBRemote &response);
+                 uint32_t reg_num); // Must be the eRegisterKindProcessPlugin register number
 
-    bool
-    ReadAllRegisters (lldb::tid_t tid,
-                      StringExtractorGDBRemote &response);
+    lldb::DataBufferSP
+    ReadAllRegisters(lldb::tid_t tid);
 
     bool
     WriteRegister(lldb::tid_t tid, uint32_t reg_num, // eRegisterKindProcessPlugin register number
-                  llvm::StringRef data);
+                  llvm::ArrayRef<uint8_t> data);
 
     bool
-    WriteAllRegisters(lldb::tid_t tid, llvm::StringRef data);
+    WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef<uint8_t> data);
 
     bool
     SaveRegisterState(lldb::tid_t tid, uint32_t &save_id);
 
     bool
     RestoreRegisterState (lldb::tid_t tid, uint32_t save_id);
 
     bool

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

 #include <numeric>
 
 // Other libraries and framework includes
 #include "lldb/Core/Log.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/State.h"
 #include "lldb/Core/StreamGDBRemote.h"
 #include "lldb/Core/StreamString.h"
+#include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
 #include "lldb/Host/TimeValue.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/LLDBAssert.h"
if (m_avoid_g_packets == eLazyBoolCalculate)
                     }
                 }
             }
         }
     }
     return m_avoid_g_packets == eLazyBoolYes;
 }
 
-bool
-GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, StringExtractorGDBRemote &response)
+DataBufferSP
+GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg)
 {
     StreamString payload;
     payload.Printf("p%x", reg);
-    return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
-           PacketResult::Success;
-}
+    StringExtractorGDBRemote response;
+    if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success ||
+        !response.IsNormalResponse())
+        return nullptr;
 
+    DataBufferSP buffer_sp(new DataBufferHeap(response.GetStringRef().size() / 2, 0));
+    response.GetHexBytes(buffer_sp->GetBytes(), buffer_sp->GetByteSize(), '\xcc');
+    return buffer_sp;
+}
 
-bool
-GDBRemoteCommunicationClient::ReadAllRegisters (lldb::tid_t tid, StringExtractorGDBRemote &response)
+DataBufferSP
+GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid)
 {
     StreamString payload;
     payload.PutChar('g');
-    return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
-           PacketResult::Success;
+    StringExtractorGDBRemote response;
+    if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success ||
+        !response.IsNormalResponse())
+        return nullptr;
+
+    DataBufferSP buffer_sp(new DataBufferHeap(response.GetStringRef().size() / 2, 0));
+    response.GetHexBytes(buffer_sp->GetBytes(), buffer_sp->GetByteSize(), '\xcc');
+    return buffer_sp;
 }
 
 bool
-GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::StringRef data)
+GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::ArrayRef<uint8_t> data)
 {
     StreamString payload;
     payload.Printf("P%x=", reg_num);
     payload.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder());
     StringExtractorGDBRemote response;
     return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
-           PacketResult::Success;
+               PacketResult::Success &&
+           response.IsOKResponse();
 }
 
 bool
-GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::StringRef data)
+GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef<uint8_t> data)
 {
     StreamString payload;
     payload.PutChar('G');
-    payload.Write(data.data(), data.size());
+    payload.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder());
     StringExtractorGDBRemote response;
     return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
-           PacketResult::Success;
+               PacketResult::Success &&
+           response.IsOKResponse();
 }
 
 bool
 GDBRemoteCommunicationClient::SaveRegisterState (lldb::tid_t tid, uint32_t &save_id)
 {
     save_id = 0; // Set to invalid save ID
     if (m_supports_QSaveRegisterState == eLazyBoolNo)
         return false;

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h

ReadRegisterBytes (const RegisterInfo *reg_info,
                        DataExtractor &data);
 
     bool
     WriteRegisterBytes (const RegisterInfo *reg_info,
                         DataExtractor &data, 
                         uint32_t data_offset);
 
     bool
-    PrivateSetRegisterValue (uint32_t reg, StringExtractor &response);
-    
+    PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data);
     bool
     PrivateSetRegisterValue (uint32_t reg, uint64_t val);
 
     void
     SetAllRegisterValid (bool b);
 
     bool
     GetRegisterIsValid (uint32_t reg) const

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

if (ReadRegisterBytes (reg_info, m_reg_data))
         const bool partial_data_ok = false;
         Error error (value.SetValueFromData(reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok));
         return error.Success();
     }
     return false;
 }
 
 bool
-GDBRemoteRegisterContext::PrivateSetRegisterValue (uint32_t reg, StringExtractor &response)
+GDBRemoteRegisterContext::PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data)
 {
     const RegisterInfo *reg_info = GetRegisterInfoAtIndex (reg);
     if (reg_info == NULL)
         return false;
 
     // Invalidate if needed
     InvalidateIfNeeded(false);
 
-    const uint32_t reg_byte_size = reg_info->byte_size;
-    const size_t bytes_copied = response.GetHexBytes (const_cast<uint8_t*>(m_reg_data.PeekData(reg_info->byte_offset, reg_byte_size)), reg_byte_size, '\xcc');
-    bool success = bytes_copied == reg_byte_size;
+    const size_t reg_byte_size = reg_info->byte_size;
+    memcpy(const_cast<uint8_t *>(m_reg_data.PeekData(reg_info->byte_offset, reg_byte_size)), data.data(),
+           std::min(data.size(), reg_byte_size));
+    bool success = data.size() >= reg_byte_size;
     if (success)
     {
         SetRegisterIsValid(reg, true);
     }
-    else if (bytes_copied > 0)
+    else if (data.size() > 0)
     {
         // Only set register is valid to false if we copied some bytes, else
         // leave it as it was.
         SetRegisterIsValid(reg, false);
     }
     return success;
 }
 

 // Helper function for GDBRemoteRegisterContext::ReadRegisterBytes().
 bool
 GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info,
                                                 GDBRemoteCommunicationClient &gdb_comm)
 {
     const uint32_t lldb_reg = reg_info->kinds[eRegisterKindLLDB];
     const uint32_t remote_reg = reg_info->kinds[eRegisterKindProcessPlugin];
     StringExtractorGDBRemote response;
-    if (gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg, response))
-        return PrivateSetRegisterValue (lldb_reg, response);
+    if (DataBufferSP buffer_sp = gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg))
+        return PrivateSetRegisterValue(lldb_reg,
+                                       llvm::ArrayRef<uint8_t>(buffer_sp->GetBytes(), buffer_sp->GetByteSize()));
     return false;
 }
 
 bool
 GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data)
 {
     ExecutionContext exe_ctx (CalculateThread());
 
     Process *process = exe_ctx.GetProcessPtr();
     Thread *thread = exe_ctx.GetThreadPtr();
     if (process == NULL || thread == NULL)
         return false;
 
     GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
 
     InvalidateIfNeeded(false);
 
     const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
 
     if (!GetRegisterIsValid(reg))
     {
         if (m_read_all_at_once)
         {
-            StringExtractorGDBRemote response;
-            if (!gdb_comm.ReadAllRegisters(m_thread.GetProtocolID(), response))
-                return false;
-            if (response.IsNormalResponse())
-                if (response.GetHexBytes(const_cast<void *>(reinterpret_cast<const void *>(m_reg_data.GetDataStart())),
-                                         m_reg_data.GetByteSize(), '\xcc') == m_reg_data.GetByteSize())
-                    SetAllRegisterValid (true);
+            if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID()))
+            {
+                memcpy(const_cast<uint8_t *>(m_reg_data.GetDataStart()), buffer_sp->GetBytes(),
+                       std::min(buffer_sp->GetByteSize(), m_reg_data.GetByteSize()));
+                if (buffer_sp->GetByteSize() >= m_reg_data.GetByteSize())
+                {
+                    SetAllRegisterValid(true);
+                    return true;
                 }
-        else if (reg_info->value_regs)
+            }
+            return false;
+        }
+        if (reg_info->value_regs)
         {
             // Process this composite register request by delegating to the constituent
             // primordial registers.
             
             // Index of the primordial register.
             bool success = true;
             for (uint32_t idx = 0; success; ++idx)
             {
GDBRemoteRegisterContext::SetPrimordialRegister(const RegisterInfo *reg_info,
     StreamString packet;
     StringExtractorGDBRemote response;
     const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
     // Invalidate just this register
     SetRegisterIsValid(reg, false);
 
     return gdb_comm.WriteRegister(
         m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin],
-        llvm::StringRef(reinterpret_cast<const char *>(m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size)),
-                        reg_info->byte_size));
+        {m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size), reg_info->byte_size});
 }
 
 bool
 GDBRemoteRegisterContext::WriteRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data, uint32_t data_offset)
 {
     ExecutionContext exe_ctx (CalculateThread());
 
     Process *process = exe_ctx.GetProcessPtr();
if (data.CopyByteOrderedData (data_offset,                  // src offset
                                   reg_info->byte_size,          // src length
                                   dst,                          // dst
                                   reg_info->byte_size,          // dst length
                                   m_reg_data.GetByteOrder()))   // dst byte order
     {
         GDBRemoteClientBase::Lock lock(gdb_comm, false);
         if (lock)
         {
-            const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
-            ProcessSP process_sp (m_thread.GetProcess());
-            if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
-            {
-                StreamString packet;
-                StringExtractorGDBRemote response;
-                
                 if (m_read_all_at_once)
                 {
-                    // Set all registers in one packet
-                    packet.PutChar ('G');
-                    packet.PutBytesAsRawHex8 (m_reg_data.GetDataStart(),
-                                              m_reg_data.GetByteSize(),
-                                              endian::InlHostByteOrder(),
-                                              endian::InlHostByteOrder());
-
-                    if (thread_suffix_supported)
-                        packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetProtocolID());
-
                     // Invalidate all register values
                     InvalidateIfNeeded (true);
 
-                    if (gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(),
-                                                              packet.GetString().size(),
-                                                              response,
-                                                              false) == GDBRemoteCommunication::PacketResult::Success)
+                    // Set all registers in one packet
+                    if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(),
+                                                   {m_reg_data.GetDataStart(), m_reg_data.GetByteSize()}))
+
                     {
                         SetAllRegisterValid (false);
-                        if (response.IsOKResponse())
-                        {
-                            return true;
-                        }
-                    }
-                }
                 else
                 {
                     bool success = true;
 
                     if (reg_info->value_regs)
                     {
                         // This register is part of another register. In this case we read the actual
                         // register data for any "value_regs", and once all that data is read, we will
if (data.CopyByteOrderedData (data_offset,                  // src offset
                              reg = reg_info->invalidate_regs[++idx])
                         {
                             SetRegisterIsValid(reg, false);
                         }
                     }
                     
                     return success;
                 }
-            }
-        }
         else
         {
             Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_THREAD | GDBR_LOG_PACKETS));
             if (log)
             {
                 if (log->GetVerbose())
                 {
                     StreamString strm;
GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
 
     Process *process = exe_ctx.GetProcessPtr();
     Thread *thread = exe_ctx.GetThreadPtr();
     if (process == NULL || thread == NULL)
         return false;
 
     GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
 
-    StringExtractorGDBRemote response;
-
     const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false;
 
     GDBRemoteClientBase::Lock lock(gdb_comm, false);
     if (lock)
     {
         if (gdb_comm.SyncThreadState(m_thread.GetProtocolID()))
             InvalidateAllRegisters();
 
-        if (use_g_packet && gdb_comm.ReadAllRegisters(m_thread.GetProtocolID(), response))
-        {
-            if (response.IsErrorResponse())
-                return false;
-
-            std::string &response_str = response.GetStringRef();
-            if (!isxdigit(response_str[0]))
-                return false;
-
-            data_sp.reset(new DataBufferHeap(response_str.c_str(), response_str.size()));
+        if (use_g_packet && (data_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID())))
             return true;
-        }
-        else
-        {
-            // For the use_g_packet == false case, we're going to read each register
-            // individually and store them as binary data in a buffer instead of as ascii
-            // characters.
-            const RegisterInfo *reg_info;
 
-            // data_sp will take ownership of this DataBufferHeap pointer soon.
-            DataBufferSP reg_ctx(new DataBufferHeap(m_reg_info.GetRegisterDataByteSize(), 0));
+        // We're going to read each register
+        // individually and store them as binary data in a buffer.
+        const RegisterInfo *reg_info;
 
-            for (uint32_t i = 0; (reg_info = GetRegisterInfoAtIndex(i)) != NULL; i++)
-            {
-                if (reg_info->value_regs) // skip registers that are slices of real registers
-                    continue;
-                ReadRegisterBytes(reg_info, m_reg_data);
-                // ReadRegisterBytes saves the contents of the register in to the m_reg_data buffer
-            }
-            memcpy(reg_ctx->GetBytes(), m_reg_data.GetDataStart(), m_reg_info.GetRegisterDataByteSize());
-
-            data_sp = reg_ctx;
-            return true;
-        }
-    }
+        data_sp.reset(new DataBufferHeap(m_reg_data.GetDataStart(), m_reg_info.GetRegisterDataByteSize()));
     else
     {
 
         Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_THREAD | GDBR_LOG_PACKETS));
         if (log)
         {
             if (log->GetVerbose())
             {
GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data_sp)
     Thread *thread = exe_ctx.GetThreadPtr();
     if (process == NULL || thread == NULL)
         return false;
 
     GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
 
     const bool use_g_packet = gdb_comm.AvoidGPackets ((ProcessGDBRemote *)process) == false;
 
-    StringExtractorGDBRemote response;
     GDBRemoteClientBase::Lock lock(gdb_comm, false);
     if (lock)
     {
         // The data_sp contains the G response packet.
-        llvm::StringRef data(reinterpret_cast<const char *>(data_sp->GetBytes()), data_sp->GetByteSize());
         if (use_g_packet)
         {
-            if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), data))
+            if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), {data_sp->GetBytes(), data_sp->GetByteSize()}))
                 return true;
 
             uint32_t num_restored = 0;
             // We need to manually go through all of the registers and
             // restore them manually
-
-            response.GetStringRef() = data;
-            DataBufferHeap buffer(data.size() / 2, 0);
-
-            const uint32_t bytes_extracted = response.GetHexBytes(buffer.GetBytes(), buffer.GetByteSize(), '\xcc');
-
-            DataExtractor restore_data(buffer.GetBytes(), buffer.GetByteSize(), m_reg_data.GetByteOrder(),
-                                       m_reg_data.GetAddressByteSize());
-
-            if (bytes_extracted < restore_data.GetByteSize())
-                restore_data.SetData(restore_data.GetDataStart(), bytes_extracted, m_reg_data.GetByteOrder());
+            DataExtractor restore_data(data_sp, m_reg_data.GetByteOrder(), m_reg_data.GetAddressByteSize());
 
             const RegisterInfo *reg_info;
 
             // The g packet contents may either include the slice registers (registers defined in
             // terms of other registers, e.g. eax is a subset of rax) or not.  The slice registers
             // should NOT be in the g packet, but some implementations may incorrectly include them.
             //
             // If the slice registers are included in the packet, we must step over the slice registers
if (lock)
                 }
                 else
                 {
                     register_offset = running_byte_offset;
                 }
 
                 const uint32_t reg_byte_size = reg_info->byte_size;
 
-                const char *restore_src = (const char *)restore_data.PeekData(register_offset, reg_byte_size);
+                const uint8_t *restore_src = restore_data.PeekData(register_offset, reg_byte_size);
                 if (restore_src)
                 {
                     SetRegisterIsValid(reg, false);
                     if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin],
-                                               llvm::StringRef(restore_src, reg_byte_size)))
+                                               {restore_src, reg_byte_size}))
                         ++num_restored;
                 }
             }
             return num_restored > 0;
         }
         else
         {
             // For the use_g_packet == false case, we're going to write each register
if (lock)
                 // work around a bug in an older version of debugserver that would lead to
                 // register context corruption when writing fpsr/fpcr.
                 if (arm64_debugserver && (strcmp(reg_info->name, "fpsr") == 0 || strcmp(reg_info->name, "fpcr") == 0))
                 {
                     continue;
                 }
 
                 SetRegisterIsValid(reg_info, false);
-                if (gdb_comm.WriteRegister(
-                        m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin],
-                        llvm::StringRef(reinterpret_cast<const char *>(data_sp->GetBytes() + reg_info->byte_offset),
-                                        reg_info->byte_size)))
-                {
+                if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin],
+                                           {data_sp->GetBytes() + reg_info->byte_offset, reg_info->byte_size}))
                     ++num_restored;
-                }
-            }
             return num_restored > 0;
         }
     }
     else
     {
         Log *log (ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet (GDBR_LOG_THREAD | GDBR_LOG_PACKETS));
         if (log)
         {

lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

if (tid != LLDB_INVALID_THREAD_ID)
         {
             ThreadGDBRemote *gdb_thread = static_cast<ThreadGDBRemote *> (thread_sp.get());
             gdb_thread->GetRegisterContext()->InvalidateIfNeeded(true);
 
             for (const auto &pair : expedited_register_map)
             {
                 StringExtractor reg_value_extractor;
                 reg_value_extractor.GetStringRef() = pair.second;
-                gdb_thread->PrivateSetRegisterValue (pair.first, reg_value_extractor);
+                DataBufferSP buffer_sp(new DataBufferHeap(reg_value_extractor.GetStringRef().size() / 2, 0));
+                reg_value_extractor.GetHexBytes(buffer_sp->GetBytes(), buffer_sp->GetByteSize(), '\xcc');
+                gdb_thread->PrivateSetRegisterValue(
+                    pair.first, llvm::ArrayRef<uint8_t>(buffer_sp->GetBytes(), buffer_sp->GetByteSize()));
             }
 
             thread_sp->SetName (thread_name.empty() ? NULL : thread_name.c_str());
 
             gdb_thread->SetThreadDispatchQAddr (thread_dispatch_qaddr);
             // Check if the GDB server was able to provide the queue name, kind and serial number
             if (queue_vars_valid)
                 gdb_thread->SetQueueInfo(std::move(queue_name), queue_kind, queue_serial, dispatch_queue_t, associated_with_dispatch_queue);

lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h

protected:
     std::string m_dispatch_queue_name;
     lldb::addr_t m_thread_dispatch_qaddr;
     lldb::addr_t m_dispatch_queue_t;
     lldb::QueueKind m_queue_kind;     // Queue info from stop reply/stop info for thread
     uint64_t m_queue_serial_number;   // Queue info from stop reply/stop info for thread
     lldb_private::LazyBool m_associated_with_libdispatch_queue;
 
     bool
-    PrivateSetRegisterValue (uint32_t reg, 
-                             StringExtractor &response);
+    PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data);
 
     bool
     PrivateSetRegisterValue (uint32_t reg, 
                              uint64_t regval);
 
     bool
     CachedQueueInfoIsValid() const
     {

lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp

else
         Unwind *unwinder = GetUnwinder ();
         if (unwinder)
             reg_ctx_sp = unwinder->CreateRegisterContextForFrame (frame);
     }
     return reg_ctx_sp;
 }
 
 bool
-ThreadGDBRemote::PrivateSetRegisterValue (uint32_t reg, StringExtractor &response)
+ThreadGDBRemote::PrivateSetRegisterValue(uint32_t reg, llvm::ArrayRef<uint8_t> data)
 {
     GDBRemoteRegisterContext *gdb_reg_ctx = static_cast<GDBRemoteRegisterContext *>(GetRegisterContext ().get());
     assert (gdb_reg_ctx);
-    return gdb_reg_ctx->PrivateSetRegisterValue (reg, response);
+    return gdb_reg_ctx->PrivateSetRegisterValue(reg, data);
 }
 
 bool
 ThreadGDBRemote::PrivateSetRegisterValue (uint32_t reg, uint64_t regval)
 {
     GDBRemoteRegisterContext *gdb_reg_ctx = static_cast<GDBRemoteRegisterContext *>(GetRegisterContext ().get());
     assert (gdb_reg_ctx);
     return gdb_reg_ctx->PrivateSetRegisterValue (reg, regval);

lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

 #include <eh.h>
 #endif
 #include <future>
 
 #include "GDBRemoteTestUtils.h"
 #include "gtest/gtest.h"
 
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
+#include "lldb/Core/DataBuffer.h"
+
+#include "llvm/ADT/ArrayRef.h"
 
 using namespace lldb_private::process_gdb_remote;
 using namespace lldb_private;
 using namespace lldb;
 
 namespace
 {
 

 HandlePacket(MockServer &server, llvm::StringRef expected, llvm::StringRef response)
 {
     StringExtractorGDBRemote request;
     ASSERT_EQ(PacketResult::Success, server.GetPacket(request));
     ASSERT_EQ(expected, request.GetStringRef());
     ASSERT_EQ(PacketResult::Success, server.SendPacket(response));
 }
 
-const char all_registers[] = "404142434445464748494a4b4c4d4e4f";
+uint8_t all_registers[] = {'@', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O'};
+std::string all_registers_hex = "404142434445464748494a4b4c4d4e4f";
+uint8_t one_register[] = {'A', 'B', 'C', 'D'};
+std::string one_register_hex = "41424344";
 
 } // end anonymous namespace
 
 class GDBRemoteCommunicationClientTest : public GDBRemoteTest
 {
 };
 
 TEST_F(GDBRemoteCommunicationClientTest, WriteRegister)
 {
     TestClient client;
     MockServer server;
     Connect(client, server);
     if (HasFailure())
         return;
 
     const lldb::tid_t tid = 0x47;
     const uint32_t reg_num = 4;
     std::future<bool> write_result =
-        std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, "ABCD"); });
+        std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register); });
 
     Handle_QThreadSuffixSupported(server, true);
 
-    HandlePacket(server, "P4=41424344;thread:0047;", "OK");
+    HandlePacket(server, "P4=" + one_register_hex + ";thread:0047;", "OK");
     ASSERT_TRUE(write_result.get());
 
     write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers); });
 
-    HandlePacket(server, std::string("G") + all_registers + ";thread:0047;", "OK");
+    HandlePacket(server, "G" + all_registers_hex + ";thread:0047;", "OK");
     ASSERT_TRUE(write_result.get());
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, WriteRegisterNoSuffix)
 {
     TestClient client;
     MockServer server;
     Connect(client, server);
     if (HasFailure())
         return;
 
     const lldb::tid_t tid = 0x47;
     const uint32_t reg_num = 4;
     std::future<bool> write_result =
-        std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, "ABCD"); });
+        std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register); });
 
     Handle_QThreadSuffixSupported(server, false);
     HandlePacket(server, "Hg47", "OK");
-    HandlePacket(server, "P4=41424344", "OK");
+    HandlePacket(server, "P4=" + one_register_hex, "OK");
     ASSERT_TRUE(write_result.get());
 
     write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers); });
 
-    HandlePacket(server, std::string("G") + all_registers, "OK");
+    HandlePacket(server, "G" + all_registers_hex, "OK");
     ASSERT_TRUE(write_result.get());
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, ReadRegister)
 {
     TestClient client;
     MockServer server;
     Connect(client, server);
     if (HasFailure())
         return;
 
     const lldb::tid_t tid = 0x47;
     const uint32_t reg_num = 4;
     std::future<bool> async_result = std::async(std::launch::async, [&] { return client.GetpPacketSupported(tid); });
     Handle_QThreadSuffixSupported(server, true);
-    HandlePacket(server, "p0;thread:0047;", "41424344");
+    HandlePacket(server, "p0;thread:0047;", one_register_hex);
     ASSERT_TRUE(async_result.get());
 
-    StringExtractorGDBRemote response;
-    async_result = std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num, response); });
+    std::future<DataBufferSP> read_result =
+        std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num); });
     HandlePacket(server, "p4;thread:0047;", "41424344");
-    ASSERT_TRUE(async_result.get());
-    ASSERT_EQ("41424344", response.GetStringRef());
-
-    async_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid, response); });
-    HandlePacket(server, "g;thread:0047;", all_registers);
-    ASSERT_TRUE(async_result.get());
-    ASSERT_EQ(all_registers, response.GetStringRef());
+    auto buffer_sp = read_result.get();
+    ASSERT_TRUE(bool(buffer_sp));
+    ASSERT_EQ(0, memcmp(buffer_sp->GetBytes(), one_register, sizeof one_register));
+
+    read_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid); });
+    HandlePacket(server, "g;thread:0047;", all_registers_hex);
+    buffer_sp = read_result.get();
+    ASSERT_TRUE(bool(buffer_sp));
+    ASSERT_EQ(0, memcmp(buffer_sp->GetBytes(), all_registers, sizeof all_registers));
 }
 
 TEST_F(GDBRemoteCommunicationClientTest, SaveRestoreRegistersNoSuffix)
 {
     TestClient client;
     MockServer server;
     Connect(client, server);
     if (HasFailure())