labath created this revision.
Herald added subscribers: mgorny, srhines.

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


https://reviews.llvm.org/D35311

Files:
  unittests/tools/CMakeLists.txt
  unittests/tools/lldb-server/CMakeLists.txt
  unittests/tools/lldb-server/tests/MessageObjects.cpp
  unittests/tools/lldb-server/tests/MessageObjects.h
  unittests/tools/lldb-server/tests/TestClient.cpp
  unittests/tools/lldb-server/tests/TestClient.h

Index: unittests/tools/lldb-server/tests/TestClient.h
===================================================================
--- unittests/tools/lldb-server/tests/TestClient.h
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -22,6 +22,9 @@
     : 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();
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -28,6 +28,16 @@
 namespace llgs_tests {
 void TestClient::Initialize() { HostInfo::Initialize(); }
 
+bool TestClient::IsDebugServer() {
+#ifdef __APPLE__
+  return true;
+#else
+  return false;
+#endif
+}
+
+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 +49,16 @@
   const ArchSpec &arch_spec = HostInfo::GetArchitecture();
   Args args;
   args.AppendArgument(LLDB_SERVER);
-  args.AppendArgument("gdbserver");
-  args.AppendArgument("--log-channels=gdb-remote packets");
+  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 +96,11 @@
 
 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 +209,7 @@
       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;
Index: unittests/tools/lldb-server/tests/MessageObjects.h
===================================================================
--- unittests/tools/lldb-server/tests/MessageObjects.h
+++ unittests/tools/lldb-server/tests/MessageObjects.h
@@ -89,7 +89,10 @@
 
 // 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) {
Index: unittests/tools/lldb-server/tests/MessageObjects.cpp
===================================================================
--- unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -19,7 +19,7 @@
 
 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 @@
 
 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 = 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];
 
-  auto elements = *elements_or_error;
-  stop_reply.m_name = elements["name"];
-  stop_reply.m_reason = elements["reason"];
+  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,25 +186,25 @@
 
   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();
     }
   }
 
   return stop_reply;
 }
 
 //====== 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 @@
     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;
Index: unittests/tools/lldb-server/CMakeLists.txt
===================================================================
--- unittests/tools/lldb-server/CMakeLists.txt
+++ unittests/tools/lldb-server/CMakeLists.txt
@@ -7,7 +7,12 @@
 
 add_lldb_test_executable(thread_inferior inferior/thread_inferior.cpp)
 
-add_definitions(-DLLDB_SERVER="$<TARGET_FILE:lldb-server>")
+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)
Index: unittests/tools/CMakeLists.txt
===================================================================
--- unittests/tools/CMakeLists.txt
+++ 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-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to