Fix regression in jModulesInfo packet handling

The recent UUID cleanups exposed a bug in the parsing code for the
jModulesInfo response, which was passing wrong value for the second
argument to UUID::SetFromStringRef (it passed the length of the string,
whereas the correct value should be the number of decoded bytes we
expect to receive).

This was not picked up by tests, because they test with 16-byte uuids,
for which the function happens to do the right thing even if the length
does not match (if the length does not match, the function does not
update m_num_uuid_bytes member, but that member is already 16 to begin
with).

I fix that and add a test with 20-byte uuid to catch if this regresses.
I have also added more safeguards into the parsing code to fail if we
cannot parse the entire uuid field we recieve. While testing the latter
part, I noticed that the "negative" jModulesInfo tests were succeeding
because we were sending malformed json (and not because the json
contents was invalid), so I make those tests a bit more robuts as well.

llvm-svn: 320985

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index e79f5d0..867f57c 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3442,7 +3442,9 @@ ParseModuleSpec(StructuredData::Dictionary *dict) {
 
   if (!dict->GetValueForKeyAsString("uuid", string))
     return llvm::None;
-  result.GetUUID().SetFromStringRef(string, string.size());
+  if (result.GetUUID().SetFromStringRef(string, string.size() / 2) !=
+      string.size())
+    return llvm::None;
 
   if (!dict->GetValueForKeyAsInteger("file_offset", integer))
     return llvm::None;

diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 90a9c7e..1d7632a 100644 (file)
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -197,27 +197,53 @@ TEST_F(GDBRemoteCommunicationClientTest, GetModulesInfo) {
   EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
 }
 
+TEST_F(GDBRemoteCommunicationClientTest, GetModulesInfo_UUID20) {
+  llvm::Triple triple("i386-pc-linux");
+
+  FileSpec file_spec("/foo/bar.so", false, FileSpec::ePathSyntaxPosix);
+  std::future<llvm::Optional<std::vector<ModuleSpec>>> async_result =
+      std::async(std::launch::async,
+                 [&] { return client.GetModulesInfo(file_spec, triple); });
+  HandlePacket(
+      server,
+      "jModulesInfo:["
+      R"({"file":"/foo/bar.so","triple":"i386-pc-linux"}])",
+      R"([{"uuid":"404142434445464748494a4b4c4d4e4f50515253","triple":"i386-pc-linux",)"
+      R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])");
+
+  auto result = async_result.get();
+  ASSERT_TRUE(result.hasValue());
+  ASSERT_EQ(1u, result->size());
+  EXPECT_EQ("/foo/bar.so", result.getValue()[0].GetFileSpec().GetPath());
+  EXPECT_EQ(triple, result.getValue()[0].GetArchitecture().GetTriple());
+  EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20), result.getValue()[0].GetUUID());
+  EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset());
+  EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
+}
+
 TEST_F(GDBRemoteCommunicationClientTest, GetModulesInfoInvalidResponse) {
   llvm::Triple triple("i386-pc-linux");
   FileSpec file_spec("/foo/bar.so", false, FileSpec::ePathSyntaxPosix);
 
   const char *invalid_responses[] = {
-      "OK", "E47", "[]",
       // no UUID
       R"([{"triple":"i386-pc-linux",)"
-      R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}])",
+      R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])",
+      // invalid UUID
+      R"([{"uuid":"XXXXXX","triple":"i386-pc-linux",)"
+      R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])",
       // no triple
       R"([{"uuid":"404142434445464748494a4b4c4d4e4f",)"
-      R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}])",
+      R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])",
       // no file_path
       R"([{"uuid":"404142434445464748494a4b4c4d4e4f","triple":"i386-pc-linux",)"
-      R"("file_offset":0,"file_size":1234}])",
+      R"("file_offset":0,"file_size":1234}]])",
       // no file_offset
       R"([{"uuid":"404142434445464748494a4b4c4d4e4f","triple":"i386-pc-linux",)"
-      R"("file_path":"/foo/bar.so","file_size":1234}])",
+      R"("file_path":"/foo/bar.so","file_size":1234}]])",
       // no file_size
       R"([{"uuid":"404142434445464748494a4b4c4d4e4f","triple":"i386-pc-linux",)"
-      R"("file_path":"/foo/bar.so","file_offset":0}])",
+      R"("file_path":"/foo/bar.so","file_offset":0}]])",
   };
 
   for (const char *response : invalid_responses) {
@@ -229,7 +255,9 @@ TEST_F(GDBRemoteCommunicationClientTest, GetModulesInfoInvalidResponse) {
         R"(jModulesInfo:[{"file":"/foo/bar.so","triple":"i386-pc-linux"}])",
         response);
 
-    ASSERT_FALSE(async_result.get().hasValue()) << "response was: " << response;
+    auto result = async_result.get();
+    ASSERT_TRUE(result);
+    ASSERT_EQ(0u, result->size()) << "response was: " << response;
   }
 }