From 8c92c899c6f4a895d3ebe990b684e1c30df12bc7 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 18 Dec 2017 14:31:44 +0000 Subject: [PATCH] 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 --- .../GDBRemoteCommunicationClient.cpp | 4 +- .../GDBRemoteCommunicationClientTest.cpp | 42 +++++++++++++++---- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index e79f5d0c467e..867f57c475ce 100644 --- 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 90a9c7ea7a0f..1d7632a67a30 100644 --- 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>> 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; } }