[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue May 28 01:17:04 PDT 2019

labath added a comment.

Thank you for working on this. This can speed up the handling of shared library loading by a lot. Also, thank you for splitting the work up into logical pieces. Please find my comments inline.

Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2697-2717
+  std::string xfer_object;
+  if (packet.GetStringBeforeChar(xfer_object, ':') == 0)
+    return SendIllFormedResponse(packet, "qXfer packet missing object name");
+  // Consume ':'
+  packet.GetChar();
+  // Parse action (read/write).
+  std::string xfer_action;
I think it would be simpler to just forget about the StringExtractor here, and do something like
SmallVector<StringRef, 4> fields;
StringRef(packet.GetStringRef()).split(fields, 3);
if (fields.size() != 4) return "Malformed packet";
// After this, "object" is in fields[0], "action" is in fields[1], annex is in fields[2], and the rest is in fields[3]
std::string buffer_key = fields[0] + fields[2];

Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2738
+  if (!memory_buffer_sp) {
+    if (xfer_object == "auxv") {
+// *BSD impls should be able to do this too.
Given that this function is going to grow, it would be good to split it in smaller chunks. Maybe move this code into something like `ErrorOr<xfer_map::iterator> ReadObject(StringRef object)`?

Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2740
+// *BSD impls should be able to do this too.
+#if defined(__linux__) || defined(__NetBSD__)
+      Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
I think we should just remove this ifdef. NetBSD and linux are the only platforms supported by lldb-server right now. And anyway, any implementation which does not support auxv reading can just return an error from `process->GetAuxvData`...

If we want to differentiate between SendErrorResponse and SendUnimplementedResponse we can just develop a convention that a specific error code (`llvm::errc::not_supported`) means "unimplemented".

Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:85
   lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
-  std::unique_ptr<llvm::MemoryBuffer> m_active_auxv_buffer_up;
+  std::unordered_map<std::string, std::shared_ptr<llvm::MemoryBuffer>>
+      m_xfer_buffer_map;
This could be an `llvm::StringMap`. Also, it should be enough to use `unique_ptr` here. You just need to make sure that you don't copy the value out of the map. I recommend a pattern like
iter = map.find("foo");
if (iter == end()) {
  auto buffer_or_error = getFoo();
  if (buffer_or_error)
    iter = map.insert("foo", std::move(*buffer_or_error))
StringRef buffer = iter->second->getBuffer();


if (done)

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list