[Lldb-commits] [lldb] r208058 - Change ProcessGDBRemote::DoReadMemory to use the x packet to read
Jason Molenda
jmolenda at apple.com
Mon May 5 21:34:53 PDT 2014
Author: jmolenda
Date: Mon May 5 23:34:52 2014
New Revision: 208058
URL: http://llvm.org/viewvc/llvm-project?rev=208058&view=rev
Log:
Change ProcessGDBRemote::DoReadMemory to use the x packet to read
data if it is available.
Change ProcessGDBRemote's maximum read/write packet size from a
fixed 512 byte value to asking the remote gdb stub what its maximum
is, using up to 128kbyte sizes if that's allowed, and falling back
to 512 if the remote gdb stub doesn't advertise a max packet size.
Add a new "process plugin packet xfer-size" command that can be used
to override the maximum packet size (although not exceeding any packet
size maximum published by the remote gdb stub).
<rdar://problem/16032150>
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/trunk/source/Target/Memory.cpp
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=208058&r1=208057&r2=208058&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Mon May 5 23:34:52 2014
@@ -277,7 +277,8 @@ ProcessGDBRemote::ProcessGDBRemote(Targe
m_continue_C_tids (),
m_continue_s_tids (),
m_continue_S_tids (),
- m_max_memory_size (512),
+ m_max_memory_size (0),
+ m_remote_stub_max_memory_size (0),
m_addr_to_mmap_size (),
m_thread_create_bp_sp (),
m_waiting_for_attach (false),
@@ -2122,6 +2123,7 @@ ProcessGDBRemote::GetImageInfoAddress()
size_t
ProcessGDBRemote::DoReadMemory (addr_t addr, void *buf, size_t size, Error &error)
{
+ GetMaxMemorySize ();
if (size > m_max_memory_size)
{
// Keep memory read sizes down to a sane limit. This function will be
@@ -2131,7 +2133,16 @@ ProcessGDBRemote::DoReadMemory (addr_t a
}
char packet[64];
- const int packet_len = ::snprintf (packet, sizeof(packet), "m%" PRIx64 ",%" PRIx64, (uint64_t)addr, (uint64_t)size);
+ int packet_len;
+ bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+ if (binary_memory_read)
+ {
+ packet_len = ::snprintf (packet, sizeof(packet), "x0x%" PRIx64 ",0x%" PRIx64, (uint64_t)addr, (uint64_t)size);
+ }
+ else
+ {
+ packet_len = ::snprintf (packet, sizeof(packet), "m%" PRIx64 ",%" PRIx64, (uint64_t)addr, (uint64_t)size);
+ }
assert (packet_len + 1 < (int)sizeof(packet));
StringExtractorGDBRemote response;
if (m_gdb_comm.SendPacketAndWaitForResponse(packet, packet_len, response, true) == GDBRemoteCommunication::PacketResult::Success)
@@ -2139,7 +2150,25 @@ ProcessGDBRemote::DoReadMemory (addr_t a
if (response.IsNormalResponse())
{
error.Clear();
- return response.GetHexBytes(buf, size, '\xdd');
+ if (binary_memory_read)
+ {
+ // The lower level GDBRemoteCommunication packet receive layer has already de-quoted any
+ // 0x7d character escaping that was present in the packet
+
+ size_t data_received_size = response.GetBytesLeft();
+ if (data_received_size > size)
+ {
+ // Don't write past the end of BUF if the remote debug server gave us too
+ // much data for some reason.
+ data_received_size = size;
+ }
+ memcpy (buf, response.GetStringRef().data(), data_received_size);
+ return data_received_size;
+ }
+ else
+ {
+ return response.GetHexBytes(buf, size, '\xdd');
+ }
}
else if (response.IsErrorResponse())
error.SetErrorStringWithFormat("memory read failed for 0x%" PRIx64, addr);
@@ -2158,6 +2187,7 @@ ProcessGDBRemote::DoReadMemory (addr_t a
size_t
ProcessGDBRemote::DoWriteMemory (addr_t addr, const void *buf, size_t size, Error &error)
{
+ GetMaxMemorySize ();
if (size > m_max_memory_size)
{
// Keep memory read sizes down to a sane limit. This function will be
@@ -3118,6 +3148,71 @@ ProcessGDBRemote::GetAuxvData()
return buf;
}
+// Establish the largest memory read/write payloads we should use.
+// If the remote stub has a max packet size, stay under that size.
+//
+// If the remote stub's max packet size is crazy large, use a
+// reasonable largeish default.
+//
+// If the remote stub doesn't advertise a max packet size, use a
+// conservative default.
+
+void
+ProcessGDBRemote::GetMaxMemorySize()
+{
+ const uint64_t reasonable_largeish_default = 128 * 1024;
+ const uint64_t conservative_default = 512;
+
+ if (m_max_memory_size == 0)
+ {
+ uint64_t stub_max_size = m_gdb_comm.GetRemoteMaxPacketSize();
+ if (stub_max_size != UINT64_MAX && stub_max_size != 0)
+ {
+ // Save the stub's claimed maximum packet size
+ m_remote_stub_max_memory_size = stub_max_size;
+
+ // Even if the stub says it can support ginormous packets,
+ // don't exceed our resonable largeish default packet size.
+ if (stub_max_size > reasonable_largeish_default)
+ {
+ stub_max_size = reasonable_largeish_default;
+ }
+
+ m_max_memory_size = stub_max_size;
+ }
+ else
+ {
+ m_max_memory_size = conservative_default;
+ }
+ }
+}
+
+void
+ProcessGDBRemote::SetUserSpecifiedMaxMemoryTransferSize (uint64_t user_specified_max)
+{
+ if (user_specified_max != 0)
+ {
+ GetMaxMemorySize ();
+
+ if (m_remote_stub_max_memory_size != 0)
+ {
+ if (m_remote_stub_max_memory_size < user_specified_max)
+ {
+ m_max_memory_size = m_remote_stub_max_memory_size; // user specified a packet size too big, go as big
+ // as the remote stub says we can go.
+ }
+ else
+ {
+ m_max_memory_size = user_specified_max; // user's packet size is good
+ }
+ }
+ else
+ {
+ m_max_memory_size = user_specified_max; // user's packet size is probably fine
+ }
+ }
+}
+
class CommandObjectProcessGDBRemotePacketHistory : public CommandObjectParsed
{
private:
@@ -3158,6 +3253,53 @@ public:
}
};
+class CommandObjectProcessGDBRemotePacketXferSize : public CommandObjectParsed
+{
+private:
+
+public:
+ CommandObjectProcessGDBRemotePacketXferSize(CommandInterpreter &interpreter) :
+ CommandObjectParsed (interpreter,
+ "process plugin packet xfer-size",
+ "Maximum size that lldb will try to read/write one one chunk.",
+ NULL)
+ {
+ }
+
+ ~CommandObjectProcessGDBRemotePacketXferSize ()
+ {
+ }
+
+ bool
+ DoExecute (Args& command, CommandReturnObject &result)
+ {
+ const size_t argc = command.GetArgumentCount();
+ if (argc == 0)
+ {
+ result.AppendErrorWithFormat ("'%s' takes an argument to specify the max amount to be transferred when reading/writing", m_cmd_name.c_str());
+ result.SetStatus (eReturnStatusFailed);
+ return false;
+ }
+
+ ProcessGDBRemote *process = (ProcessGDBRemote *)m_interpreter.GetExecutionContext().GetProcessPtr();
+ if (process)
+ {
+ const char *packet_size = command.GetArgumentAtIndex(0);
+ errno = 0;
+ uint64_t user_specified_max = strtoul (packet_size, NULL, 10);
+ if (errno == 0 && user_specified_max != 0)
+ {
+ process->SetUserSpecifiedMaxMemoryTransferSize (user_specified_max);
+ result.SetStatus (eReturnStatusSuccessFinishResult);
+ return true;
+ }
+ }
+ result.SetStatus (eReturnStatusFailed);
+ return false;
+ }
+};
+
+
class CommandObjectProcessGDBRemotePacketSend : public CommandObjectParsed
{
private:
@@ -3283,6 +3425,7 @@ public:
LoadSubCommand ("history", CommandObjectSP (new CommandObjectProcessGDBRemotePacketHistory (interpreter)));
LoadSubCommand ("send", CommandObjectSP (new CommandObjectProcessGDBRemotePacketSend (interpreter)));
LoadSubCommand ("monitor", CommandObjectSP (new CommandObjectProcessGDBRemotePacketMonitor (interpreter)));
+ LoadSubCommand ("xfer-size", CommandObjectSP (new CommandObjectProcessGDBRemotePacketXferSize (interpreter)));
}
~CommandObjectProcessGDBRemotePacket ()
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=208058&r1=208057&r2=208058&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Mon May 5 23:34:52 2014
@@ -230,6 +230,9 @@ public:
virtual bool
SetExitStatus (int exit_status, const char *cstr);
+ void
+ SetUserSpecifiedMaxMemoryTransferSize (uint64_t user_specified_max);
+
protected:
friend class ThreadGDBRemote;
friend class GDBRemoteCommunicationClient;
@@ -304,6 +307,9 @@ protected:
virtual const lldb::DataBufferSP
GetAuxvData();
+ void
+ GetMaxMemorySize();
+
//------------------------------------------------------------------
/// Broadcaster event bits definitions.
//------------------------------------------------------------------
@@ -339,7 +345,8 @@ protected:
tid_sig_collection m_continue_C_tids; // 'C' for continue with signal
tid_collection m_continue_s_tids; // 's' for step
tid_sig_collection m_continue_S_tids; // 'S' for step with signal
- size_t m_max_memory_size; // The maximum number of bytes to read/write when reading and writing memory
+ uint64_t m_max_memory_size; // The maximum number of bytes to read/write when reading and writing memory
+ uint64_t m_remote_stub_max_memory_size; // The maximum memory size the remote gdb stub can handle
MMapMap m_addr_to_mmap_size;
lldb::BreakpointSP m_thread_create_bp_sp;
bool m_waiting_for_attach;
Modified: lldb/trunk/source/Target/Memory.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Memory.cpp?rev=208058&r1=208057&r2=208058&view=diff
==============================================================================
--- lldb/trunk/source/Target/Memory.cpp (original)
+++ lldb/trunk/source/Target/Memory.cpp Mon May 5 23:34:52 2014
@@ -120,6 +120,19 @@ MemoryCache::Read (addr_t addr,
Error &error)
{
size_t bytes_left = dst_len;
+
+ // If this memory read request is larger than the cache line size, then
+ // we (1) try to read as much of it at once as possible, and (2) don't
+ // add the data to the memory cache. We don't want to split a big read
+ // up into more separate reads than necessary, and with a large memory read
+ // request, it is unlikely that the caller function will ask for the next
+ // 4 bytes after the large memory read - so there's little benefit to saving
+ // it in the cache.
+ if (dst && dst_len > m_cache_line_byte_size)
+ {
+ return m_process.ReadMemoryFromInferior (addr, dst, dst_len, error);
+ }
+
if (dst && bytes_left > 0)
{
const uint32_t cache_line_byte_size = m_cache_line_byte_size;
More information about the lldb-commits
mailing list