[Lldb-commits] [PATCH] D44922: gdb-remote: Fix checksum verification for messages with escape chars

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL328693: gdb-remote: Fix checksum verification for messages 
with escape chars (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44922?vs=139905&id=140054#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44922

Files:
  lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt
  lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
  lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp


Index: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -37,12 +37,7 @@
   if (!IsConnected())
     return;
 
-  std::string response;
-  // Debugserver (non-conformingly?) sends a reply to the k packet instead of
-  // simply closing the connection.
-  PacketResult result =
-      IsDebugServer() ? PacketResult::Success : PacketResult::ErrorDisconnected;
-  EXPECT_THAT_ERROR(SendMessage("k", response, result), Succeeded());
+  EXPECT_THAT_ERROR(SendMessage("k"), Succeeded());
 }
 
 Expected<std::unique_ptr<TestClient>> TestClient::launch(StringRef Log) {
Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
===================================================================
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp
@@ -0,0 +1,67 @@
+//===-- GDBRemoteCommunicationTest.cpp --------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include "GDBRemoteTestUtils.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace lldb_private::process_gdb_remote;
+using namespace lldb_private;
+using namespace lldb;
+typedef GDBRemoteCommunication::PacketResult PacketResult;
+
+namespace {
+
+class TestClient : public GDBRemoteCommunication {
+public:
+  TestClient()
+      : GDBRemoteCommunication("test.client", "test.client.listener") {}
+
+  PacketResult ReadPacket(StringExtractorGDBRemote &response) {
+    return GDBRemoteCommunication::ReadPacket(response, std::chrono::seconds(1),
+                                              /*sync_on_timeout*/ false);
+  }
+};
+
+class GDBRemoteCommunicationTest : public GDBRemoteTest {
+public:
+  void SetUp() override {
+    ASSERT_THAT_ERROR(Connect(client, server), llvm::Succeeded());
+  }
+
+protected:
+  TestClient client;
+  MockServer server;
+
+  bool Write(llvm::StringRef packet) {
+    ConnectionStatus status;
+    return server.Write(packet.data(), packet.size(), status, nullptr) ==
+           packet.size();
+  }
+};
+} // end anonymous namespace
+
+TEST_F(GDBRemoteCommunicationTest, ReadPacket_checksum) {
+  struct TestCase {
+    llvm::StringLiteral Packet;
+    llvm::StringLiteral Payload;
+  };
+  static constexpr TestCase Tests[] = {
+      {{"$#00"}, {""}},
+      {{"$foobar#79"}, {"foobar"}},
+      {{"$}}#fa"}, {"]"}},
+      {{"$x*%#c7"}, {"xxxxxxxxx"}},
+  };
+  for (const auto &Test : Tests) {
+    SCOPED_TRACE(Test.Packet + " -> " + Test.Payload);
+    StringExtractorGDBRemote response;
+    ASSERT_TRUE(Write(Test.Packet));
+    ASSERT_EQ(PacketResult::Success, client.ReadPacket(response));
+    ASSERT_EQ(Test.Payload, response.GetStringRef());
+    ASSERT_EQ(PacketResult::Success, server.GetAck());
+  }
+}
Index: lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt
===================================================================
--- lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt
+++ lldb/trunk/unittests/Process/gdb-remote/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(ProcessGdbRemoteTests
   GDBRemoteClientBaseTest.cpp
   GDBRemoteCommunicationClientTest.cpp
+  GDBRemoteCommunicationTest.cpp
   GDBRemoteTestUtils.cpp
 
   LINK_LIBS
Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -914,7 +914,8 @@
           if (GetSendAcks()) {
             const char *packet_checksum_cstr = &m_bytes[checksum_idx];
             char packet_checksum = strtol(packet_checksum_cstr, NULL, 16);
-            char actual_checksum = CalculcateChecksum(packet_str);
+            char actual_checksum = CalculcateChecksum(
+                llvm::StringRef(m_bytes).slice(content_start, content_end));
             success = packet_checksum == actual_checksum;
             if (!success) {
               if (log)

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits