lldb-server tests: Add support for testing debugserver

Summary:
This adds support for running the lldb-server test suite (currently consisting
of only one test) against the debugserver. Currently, the choice which binary
to test is based on the host system. This will need to be replaced by something
more elaborate if/when lldb-server starts supporting debugging on darwin.

I need to make a couple of tweaks to the test client to work with debugserver:
- debugserver has different command-line arguments - launching code adjusted to
  handle that
- debugserver sends duplicate "medata" fields in the stop reply packet -
  adjusted stop-reply parsing code to handle that
- debugserver replies to the k packet instead of just dropping the connection -
  stopping code adjusted, although we should probably consider aligning the
  behavior of the two stubs in this case

Reviewers: jmajors, beanz

Subscribers: srhines, mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D35311

llvm-svn: 316010

lldb/unittests/tools/CMakeLists.txt

@@ -1,3 +1,3 @@
-if(CMAKE_SYSTEM_NAME MATCHES "Android|Linux|NetBSD")
+if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD")
   add_subdirectory(lldb-server)
 endif()

lldb/unittests/tools/lldb-server/CMakeLists.txt

@@ -7,7 +7,12 @@ endfunction()
 
 add_lldb_test_executable(thread_inferior inferior/thread_inferior.cpp)
 
+if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
+  add_definitions(-DLLDB_SERVER="$<TARGET_FILE:debugserver>")
+else()
   add_definitions(-DLLDB_SERVER="$<TARGET_FILE:lldb-server>")
+endif()
+
 add_definitions(-DTHREAD_INFERIOR="${CMAKE_CURRENT_BINARY_DIR}/thread_inferior")
 add_subdirectory(tests)
 add_dependencies(LLDBServerTests thread_inferior)

lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

@@ -19,7 +19,7 @@ namespace llgs_tests {
 
 Expected<ProcessInfo> ProcessInfo::Create(StringRef response) {
   ProcessInfo process_info;
-  auto elements_or_error = SplitPairList("ProcessInfo", response);
+  auto elements_or_error = SplitUniquePairList("ProcessInfo", response);
   if (!elements_or_error)
     return elements_or_error.takeError();
 
@@ -140,21 +140,36 @@ const U64Map &StopReply::GetThreadPcs() const { return m_thread_pcs; }
 
 Expected<StopReply> StopReply::Create(StringRef response,
                                       llvm::support::endianness endian) {
+  if (response.size() < 3 || !response.consume_front("T"))
+    return make_parsing_error("StopReply: Invalid packet");
+
   StopReply stop_reply;
 
-  auto elements_or_error = SplitPairList("StopReply", response);
-  if (auto split_error = elements_or_error.takeError()) {
-    return std::move(split_error);
-  }
+  StringRef signal = response.take_front(2);
+  response = response.drop_front(2);
+  if (!llvm::to_integer(signal, stop_reply.m_signal, 16))
+    return make_parsing_error("StopReply: stop signal");
 
-  auto elements = *elements_or_error;
-  stop_reply.m_name = elements["name"];
-  stop_reply.m_reason = elements["reason"];
+  auto elements = SplitPairList(response);
+  for (StringRef field :
+       {"name", "reason", "thread", "threads", "thread-pcs"}) {
+    // This will insert an empty field if there is none. In the future, we
+    // should probably differentiate between these fields not being present and
+    // them being empty, but right now no tests depends on this.
+    if (elements.insert({field, {""}}).first->second.size() != 1)
+      return make_parsing_error(
+          "StopReply: got multiple responses for the {0} field", field);
+  }
+  stop_reply.m_name = elements["name"][0];
+  stop_reply.m_reason = elements["reason"][0];
+
+  if (!llvm::to_integer(elements["thread"][0], stop_reply.m_thread, 16))
+    return make_parsing_error("StopReply: thread");
 
   SmallVector<StringRef, 20> threads;
   SmallVector<StringRef, 20> pcs;
-  elements["threads"].split(threads, ',');
-  elements["thread-pcs"].split(pcs, ',');
+  elements["threads"][0].split(threads, ',');
+  elements["thread-pcs"][0].split(pcs, ',');
   if (threads.size() != pcs.size())
     return make_parsing_error("StopReply: thread/PC count mismatch");
 
@@ -171,17 +186,16 @@ Expected<StopReply> StopReply::Create(StringRef response,
 
   for (auto i = elements.begin(); i != elements.end(); i++) {
     StringRef key = i->getKey();
-    StringRef val = i->getValue();
-    if (key.size() >= 9 && key[0] == 'T' && key.substr(3, 6) == "thread") {
-      if (val.getAsInteger(16, stop_reply.m_thread))
-        return make_parsing_error("StopReply: thread id");
-      if (key.substr(1, 2).getAsInteger(16, stop_reply.m_signal))
-        return make_parsing_error("StopReply: stop signal");
-    } else if (key.size() == 2) {
+    const auto &val = i->getValue();
+    if (key.size() == 2) {
       unsigned int reg;
-      if (!key.getAsInteger(16, reg)) {
-        stop_reply.m_registers[reg] = val.str();
-      }
+      if (key.getAsInteger(16, reg))
+        continue;
+      if (val.size() != 1)
+        return make_parsing_error(
+            "StopReply: multiple entries for register field [{0:x}]", reg);
+
+      stop_reply.m_registers[reg] = val[0].str();
     }
   }
 
@@ -189,7 +203,8 @@ Expected<StopReply> StopReply::Create(StringRef response,
 }
 
 //====== Globals ===============================================================
-Expected<StringMap<StringRef>> SplitPairList(StringRef caller, StringRef str) {
+Expected<StringMap<StringRef>> SplitUniquePairList(StringRef caller,
+                                                   StringRef str) {
   SmallVector<StringRef, 20> elements;
   str.split(elements, ';');
 
@@ -199,7 +214,20 @@ Expected<StringMap<StringRef>> SplitPairList(StringRef caller, StringRef str) {
     if (pairs.count(pair.first))
       return make_parsing_error("{0}: Duplicate Key: {1}", caller, pair.first);
 
-    pairs.insert(s.split(':'));
+    pairs.insert(pair);
+  }
+
+  return pairs;
+}
+
+StringMap<SmallVector<StringRef, 2>> SplitPairList(StringRef str) {
+  SmallVector<StringRef, 20> elements;
+  str.split(elements, ';');
+
+  StringMap<SmallVector<StringRef, 2>> pairs;
+  for (StringRef s : elements) {
+    std::pair<StringRef, StringRef> pair = s.split(':');
+    pairs[pair.first].push_back(pair.second);
   }
 
   return pairs;

lldb/unittests/tools/lldb-server/tests/MessageObjects.h

@@ -89,7 +89,10 @@ private:
 
 // Common functions for parsing packet data.
 llvm::Expected<llvm::StringMap<llvm::StringRef>>
-SplitPairList(llvm::StringRef caller, llvm::StringRef s);
+SplitUniquePairList(llvm::StringRef caller, llvm::StringRef s);
+
+llvm::StringMap<llvm::SmallVector<llvm::StringRef, 2>>
+SplitPairList(llvm::StringRef s);
 
 template <typename... Args>
 llvm::Error make_parsing_error(llvm::StringRef format, Args &&... args) {

lldb/unittests/tools/lldb-server/tests/TestClient.cpp

@@ -15,6 +15,7 @@
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 #include <cstdlib>
 #include <future>
@@ -28,6 +29,12 @@ using namespace llvm;
 namespace llgs_tests {
 void TestClient::Initialize() { HostInfo::Initialize(); }
 
+bool TestClient::IsDebugServer() {
+  return sys::path::filename(LLDB_SERVER).contains("debugserver");
+}
+
+bool TestClient::IsLldbServer() { return !IsDebugServer(); }
+
 TestClient::TestClient(const std::string &test_name,
                        const std::string &test_case_name)
     : m_test_name(test_name), m_test_case_name(test_case_name),
@@ -39,13 +46,16 @@ bool TestClient::StartDebugger() {
   const ArchSpec &arch_spec = HostInfo::GetArchitecture();
   Args args;
   args.AppendArgument(LLDB_SERVER);
+  if (IsLldbServer()) {
     args.AppendArgument("gdbserver");
     args.AppendArgument("--log-channels=gdb-remote packets");
+  } else {
+    args.AppendArgument("--log-flags=0x800000");
+  }
   args.AppendArgument("--reverse-connect");
   std::string log_file_name = GenerateLogFileName(arch_spec);
-  if (log_file_name.size()) {
+  if (log_file_name.size())
     args.AppendArgument("--log-file=" + log_file_name);
-  }
 
   Status error;
   TCPSocket listen_socket(true, false);
@@ -83,7 +93,11 @@ bool TestClient::StartDebugger() {
 
 bool TestClient::StopDebugger() {
   std::string response;
-  return SendMessage("k", response, PacketResult::ErrorDisconnected);
+  // Debugserver (non-conformingly?) sends a reply to the k packet instead of
+  // simply closing the connection.
+  PacketResult result =
+      IsDebugServer() ? PacketResult::Success : PacketResult::ErrorDisconnected;
+  return SendMessage("k", response, result);
 }
 
 bool TestClient::SetInferior(llvm::ArrayRef<std::string> inferior_args) {
@@ -192,7 +206,7 @@ unsigned int TestClient::GetPcRegisterId() {
       return UINT_MAX;
     }
 
-    auto elements_or_error = SplitPairList("GetPcRegisterId", response);
+    auto elements_or_error = SplitUniquePairList("GetPcRegisterId", response);
     if (auto split_error = elements_or_error.takeError()) {
       GTEST_LOG_(ERROR) << "GetPcRegisterId: Error splitting response: "
                         << response;

lldb/unittests/tools/lldb-server/tests/TestClient.h

@@ -22,6 +22,9 @@ class TestClient
     : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
 public:
   static void Initialize();
+  static bool IsDebugServer();
+  static bool IsLldbServer();
+
   TestClient(const std::string &test_name, const std::string &test_case_name);
   virtual ~TestClient();
   LLVM_NODISCARD bool StartDebugger();