Author: labath Date: Mon Feb 19 07:42:48 2018 New Revision: 325511 URL: http://llvm.org/viewvc/llvm-project?rev=325511&view=rev Log: Fix TestStopReplyContainsThreadPcs on 32-bit x86 (pr36013)
Summary: The issue was that we were parsing the registers into 64-bit integers and the calling swapByteOrder without regard for the actual size of the register. This switches the test to use the RegisterValue class which tracks the register size, and knows how to initialize itself from a piece of memory (so we don't need to swap byte order ourselves). Reviewers: eugene, davide Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D43376 Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp?rev=325511&r1=325510&r2=325511&view=diff ============================================================================== --- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp (original) +++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp Mon Feb 19 07:42:48 2018 @@ -9,14 +9,13 @@ #include "MessageObjects.h" #include "lldb/Interpreter/Args.h" -#include "lldb/Utility/StructuredData.h" +#include "lldb/Utility/StringExtractor.h" #include "llvm/ADT/StringExtras.h" #include "gtest/gtest.h" using namespace lldb_private; using namespace lldb; using namespace llvm; -using namespace llvm::support; namespace llgs_tests { Expected<ProcessInfo> ProcessInfo::create(StringRef response) { @@ -55,36 +54,50 @@ Expected<ProcessInfo> ProcessInfo::creat lldb::pid_t ProcessInfo::GetPid() const { return m_pid; } -endianness ProcessInfo::GetEndian() const { return m_endian; } +support::endianness ProcessInfo::GetEndian() const { return m_endian; } //====== ThreadInfo ============================================================ -ThreadInfo::ThreadInfo(StringRef name, StringRef reason, - const RegisterMap ®isters, unsigned int signal) - : m_name(name.str()), m_reason(reason.str()), m_registers(registers), - m_signal(signal) {} - -StringRef ThreadInfo::ReadRegister(unsigned int register_id) const { - return m_registers.lookup(register_id); -} - -Expected<uint64_t> -ThreadInfo::ReadRegisterAsUint64(unsigned int register_id) const { - uint64_t value; - std::string value_str(m_registers.lookup(register_id)); - if (!llvm::to_integer(value_str, value, 16)) - return make_parsing_error("ThreadInfo value for register {0}: {1}", - register_id, value_str); - - sys::swapByteOrder(value); - return value; +ThreadInfo::ThreadInfo(StringRef name, StringRef reason, RegisterMap registers, + unsigned int signal) + : m_name(name.str()), m_reason(reason.str()), + m_registers(std::move(registers)), m_signal(signal) {} + +const RegisterValue *ThreadInfo::ReadRegister(unsigned int Id) const { + auto Iter = m_registers.find(Id); + return Iter == m_registers.end() ? nullptr : &Iter->getSecond(); } //====== JThreadsInfo ========================================================== -Expected<JThreadsInfo> JThreadsInfo::Create(StringRef response, - endianness endian) { + +Expected<RegisterMap> +JThreadsInfo::parseRegisters(const StructuredData::Dictionary &Dict, + ArrayRef<RegisterInfo> RegInfos) { + RegisterMap Result; + + auto KeysObj = Dict.GetKeys(); + auto Keys = KeysObj->GetAsArray(); + for (size_t i = 0; i < Keys->GetSize(); i++) { + StringRef KeyStr, ValueStr; + Keys->GetItemAtIndexAsString(i, KeyStr); + Dict.GetValueForKeyAsString(KeyStr, ValueStr); + unsigned int Register; + if (!llvm::to_integer(KeyStr, Register, 10)) + return make_parsing_error("JThreadsInfo: register key[{0}]", i); + + auto RegValOr = + parseRegisterValue(RegInfos[Register], ValueStr, support::big); + if (!RegValOr) + return RegValOr.takeError(); + Result[Register] = std::move(*RegValOr); + } + return std::move(Result); +} + +Expected<JThreadsInfo> JThreadsInfo::create(StringRef Response, + ArrayRef<RegisterInfo> RegInfos) { JThreadsInfo jthreads_info; - StructuredData::ObjectSP json = StructuredData::ParseJSON(response); + StructuredData::ObjectSP json = StructuredData::ParseJSON(Response); StructuredData::Array *array = json->GetAsArray(); if (!array) return make_parsing_error("JThreadsInfo: JSON array"); @@ -108,23 +121,11 @@ Expected<JThreadsInfo> JThreadsInfo::Cre if (!register_dict) return make_parsing_error("JThreadsInfo: registers JSON obj"); - RegisterMap registers; - - auto keys_obj = register_dict->GetKeys(); - auto keys = keys_obj->GetAsArray(); - for (size_t i = 0; i < keys->GetSize(); i++) { - StringRef key_str, value_str; - keys->GetItemAtIndexAsString(i, key_str); - register_dict->GetValueForKeyAsString(key_str, value_str); - unsigned int register_id; - if (key_str.getAsInteger(10, register_id)) - return make_parsing_error("JThreadsInfo: register key[{0}]", i); - - registers[register_id] = value_str.str(); - } - + auto RegsOr = parseRegisters(*register_dict, RegInfos); + if (!RegsOr) + return RegsOr.takeError(); jthreads_info.m_thread_infos[tid] = - ThreadInfo(name, reason, registers, signal); + ThreadInfo(name, reason, std::move(*RegsOr), signal); } return jthreads_info; @@ -200,20 +201,64 @@ Expected<RegisterInfo> RegisterInfoParse return std::move(Info); } +Expected<RegisterValue> parseRegisterValue(const RegisterInfo &Info, + StringRef HexValue, + llvm::support::endianness Endian) { + SmallVector<uint8_t, 64> Bytes(HexValue.size() / 2); + StringExtractor(HexValue).GetHexBytes(Bytes, '\xcc'); + RegisterValue Value; + Status ST; + Value.SetFromMemoryData( + &Info, Bytes.data(), Bytes.size(), + Endian == support::little ? eByteOrderLittle : eByteOrderBig, ST); + if (ST.Fail()) + return ST.ToError(); + return Value; +} + //====== StopReply ============================================================= Expected<std::unique_ptr<StopReply>> -StopReply::create(StringRef Response, llvm::support::endianness Endian) { +StopReply::create(StringRef Response, llvm::support::endianness Endian, + ArrayRef<RegisterInfo> RegInfos) { if (Response.size() < 3) return make_parsing_error("StopReply: Invalid packet"); if (Response.consume_front("T")) - return StopReplyStop::create(Response, Endian); + return StopReplyStop::create(Response, Endian, RegInfos); if (Response.consume_front("W")) return StopReplyExit::create(Response); return make_parsing_error("StopReply: Invalid packet"); } +Expected<RegisterMap> StopReplyStop::parseRegisters( + const StringMap<SmallVector<StringRef, 2>> &Elements, + support::endianness Endian, ArrayRef<lldb_private::RegisterInfo> RegInfos) { + + RegisterMap Result; + for (const auto &E : Elements) { + StringRef Key = E.getKey(); + const auto &Val = E.getValue(); + if (Key.size() != 2) + continue; + + unsigned int Reg; + if (!to_integer(Key, Reg, 16)) + continue; + + if (Val.size() != 1) + return make_parsing_error( + "StopReplyStop: multiple entries for register field [{0:x}]", Reg); + + auto RegValOr = parseRegisterValue(RegInfos[Reg], Val[0], Endian); + if (!RegValOr) + return RegValOr.takeError(); + Result[Reg] = std::move(*RegValOr); + } + return std::move(Result); +} + Expected<std::unique_ptr<StopReplyStop>> -StopReplyStop::create(StringRef Response, llvm::support::endianness Endian) { +StopReplyStop::create(StringRef Response, support::endianness Endian, + ArrayRef<RegisterInfo> RegInfos) { unsigned int Signal; StringRef SignalStr = Response.take_front(2); Response = Response.drop_front(2); @@ -244,40 +289,31 @@ StopReplyStop::create(StringRef Response if (Threads.size() != Pcs.size()) return make_parsing_error("StopReply: thread/PC count mismatch"); - U64Map ThreadPcs; + RegisterMap ThreadPcs; + const RegisterInfo *PcInfo = find_if(RegInfos, [](const RegisterInfo &Info) { + return Info.kinds[eRegisterKindGeneric] == LLDB_REGNUM_GENERIC_PC; + }); + assert(PcInfo); + for (auto ThreadPc : zip(Threads, Pcs)) { lldb::tid_t Id; - uint64_t Pc; if (!to_integer(std::get<0>(ThreadPc), Id, 16)) return make_parsing_error("StopReply: Thread id '{0}'", std::get<0>(ThreadPc)); - if (!to_integer(std::get<1>(ThreadPc), Pc, 16)) - return make_parsing_error("StopReply Thread Pc '{0}'", - std::get<1>(ThreadPc)); - - ThreadPcs[Id] = Pc; - } - - RegisterMap Registers; - for (const auto &E : Elements) { - StringRef Key = E.getKey(); - const auto &Val = E.getValue(); - if (Key.size() != 2) - continue; - - unsigned int Reg; - if (!to_integer(Key, Reg, 16)) - continue; - if (Val.size() != 1) - return make_parsing_error( - "StopReply: multiple entries for register field [{0:x}]", Reg); - - Registers[Reg] = Val[0].str(); + auto PcOr = parseRegisterValue(*PcInfo, std::get<1>(ThreadPc), Endian); + if (!PcOr) + return PcOr.takeError(); + ThreadPcs[Id] = std::move(*PcOr); } - return llvm::make_unique<StopReplyStop>(Signal, Thread, Name, ThreadPcs, - Registers, Reason); + auto RegistersOr = parseRegisters(Elements, Endian, RegInfos); + if (!RegistersOr) + return RegistersOr.takeError(); + + return llvm::make_unique<StopReplyStop>(Signal, Thread, Name, + std::move(ThreadPcs), + std::move(*RegistersOr), Reason); } Expected<std::unique_ptr<StopReplyExit>> @@ -319,3 +355,12 @@ StringMap<SmallVector<StringRef, 2>> Spl return pairs; } } // namespace llgs_tests + +std::ostream &lldb_private::operator<<(std::ostream &OS, + const RegisterValue &RegVal) { + ArrayRef<uint8_t> Bytes(static_cast<const uint8_t *>(RegVal.GetBytes()), + RegVal.GetByteSize()); + return OS << formatv("RegisterValue[{0}]: {1:@[x-2]}", RegVal.GetByteSize(), + make_range(Bytes.begin(), Bytes.end())) + .str(); +} Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h?rev=325511&r1=325510&r2=325511&view=diff ============================================================================== --- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h (original) +++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h Mon Feb 19 07:42:48 2018 @@ -10,7 +10,9 @@ #ifndef LLDB_SERVER_TESTS_MESSAGEOBJECTS_H #define LLDB_SERVER_TESTS_MESSAGEOBJECTS_H +#include "lldb/Core/RegisterValue.h" #include "lldb/Host/Host.h" +#include "lldb/Utility/StructuredData.h" #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallString.h" @@ -22,8 +24,7 @@ namespace llgs_tests { class ThreadInfo; typedef llvm::DenseMap<uint64_t, ThreadInfo> ThreadInfoMap; -typedef llvm::DenseMap<uint64_t, uint64_t> U64Map; -typedef llvm::DenseMap<unsigned int, std::string> RegisterMap; +typedef llvm::DenseMap<unsigned int, lldb_private::RegisterValue> RegisterMap; template <typename T> struct Parser { using result_type = T; }; @@ -51,10 +52,9 @@ class ThreadInfo { public: ThreadInfo() = default; ThreadInfo(llvm::StringRef name, llvm::StringRef reason, - const RegisterMap ®isters, unsigned int signal); + RegisterMap registers, unsigned int signal); - llvm::StringRef ReadRegister(unsigned int register_id) const; - llvm::Expected<uint64_t> ReadRegisterAsUint64(unsigned int register_id) const; + const lldb_private::RegisterValue *ReadRegister(unsigned int Id) const; private: std::string m_name; @@ -63,14 +63,19 @@ private: unsigned int m_signal; }; -class JThreadsInfo { +class JThreadsInfo : public Parser<JThreadsInfo> { public: - static llvm::Expected<JThreadsInfo> Create(llvm::StringRef response, - llvm::support::endianness endian); + static llvm::Expected<JThreadsInfo> + create(llvm::StringRef Response, + llvm::ArrayRef<lldb_private::RegisterInfo> RegInfos); const ThreadInfoMap &GetThreadInfos() const; private: + static llvm::Expected<RegisterMap> + parseRegisters(const lldb_private::StructuredData::Dictionary &Dict, + llvm::ArrayRef<lldb_private::RegisterInfo> RegInfos); + JThreadsInfo() = default; ThreadInfoMap m_thread_infos; }; @@ -80,13 +85,18 @@ struct RegisterInfoParser : public Parse create(llvm::StringRef Response); }; +llvm::Expected<lldb_private::RegisterValue> +parseRegisterValue(const lldb_private::RegisterInfo &Info, + llvm::StringRef HexValue, llvm::support::endianness Endian); + class StopReply { public: StopReply() = default; virtual ~StopReply() = default; static llvm::Expected<std::unique_ptr<StopReply>> - create(llvm::StringRef response, llvm::support::endianness endian); + create(llvm::StringRef Response, llvm::support::endianness Endian, + llvm::ArrayRef<lldb_private::RegisterInfo> RegInfos); // for llvm::cast<> virtual lldb_private::WaitStatus getKind() const = 0; @@ -98,15 +108,17 @@ public: class StopReplyStop : public StopReply { public: StopReplyStop(uint8_t Signal, lldb::tid_t ThreadId, llvm::StringRef Name, - U64Map ThreadPcs, RegisterMap Registers, llvm::StringRef Reason) + RegisterMap ThreadPcs, RegisterMap Registers, + llvm::StringRef Reason) : Signal(Signal), ThreadId(ThreadId), Name(Name), ThreadPcs(std::move(ThreadPcs)), Registers(std::move(Registers)), Reason(Reason) {} static llvm::Expected<std::unique_ptr<StopReplyStop>> - create(llvm::StringRef response, llvm::support::endianness endian); + create(llvm::StringRef Response, llvm::support::endianness Endian, + llvm::ArrayRef<lldb_private::RegisterInfo> RegInfos); - const U64Map &getThreadPcs() const { return ThreadPcs; } + const RegisterMap &getThreadPcs() const { return ThreadPcs; } lldb::tid_t getThreadId() const { return ThreadId; } // for llvm::cast<> @@ -118,10 +130,15 @@ public: } private: + static llvm::Expected<RegisterMap> parseRegisters( + const llvm::StringMap<llvm::SmallVector<llvm::StringRef, 2>> &Elements, + llvm::support::endianness Endian, + llvm::ArrayRef<lldb_private::RegisterInfo> RegInfos); + uint8_t Signal; lldb::tid_t ThreadId; std::string Name; - U64Map ThreadPcs; + RegisterMap ThreadPcs; RegisterMap Registers; std::string Reason; }; @@ -163,4 +180,8 @@ llvm::Error make_parsing_error(llvm::Str } // namespace llgs_tests +namespace lldb_private { +std::ostream &operator<<(std::ostream &OS, const RegisterValue &RegVal); +} + #endif // LLDB_SERVER_TESTS_MESSAGEOBJECTS_H Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp?rev=325511&r1=325510&r2=325511&view=diff ============================================================================== --- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp (original) +++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp Mon Feb 19 07:42:48 2018 @@ -150,17 +150,8 @@ const llgs_tests::ProcessInfo &TestClien return *m_process_info; } -Optional<JThreadsInfo> TestClient::GetJThreadsInfo() { - std::string response; - if (SendMessage("jThreadsInfo", response)) - return llvm::None; - auto creation = JThreadsInfo::Create(response, m_process_info->GetEndian()); - if (auto create_error = creation.takeError()) { - GTEST_LOG_(ERROR) << toString(std::move(create_error)); - return llvm::None; - } - - return std::move(*creation); +Expected<JThreadsInfo> TestClient::GetJThreadsInfo() { + return SendMessage<JThreadsInfo>("jThreadsInfo", m_register_infos); } const StopReply &TestClient::GetLatestStopReply() { @@ -247,7 +238,8 @@ Error TestClient::Continue(StringRef mes std::string response; if (Error E = SendMessage(message, response)) return E; - auto creation = StopReply::create(response, m_process_info->GetEndian()); + auto creation = StopReply::create(response, m_process_info->GetEndian(), + m_register_infos); if (Error E = creation.takeError()) return E; Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h?rev=325511&r1=325510&r2=325511&view=diff ============================================================================== --- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h (original) +++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h Mon Feb 19 07:42:48 2018 @@ -59,7 +59,7 @@ public: llvm::Error ContinueAll(); llvm::Error ContinueThread(unsigned long thread_id); const ProcessInfo &GetProcessInfo(); - llvm::Optional<JThreadsInfo> GetJThreadsInfo(); + llvm::Expected<JThreadsInfo> GetJThreadsInfo(); const StopReply &GetLatestStopReply(); template <typename T> llvm::Expected<const T &> GetLatestStopReplyAs() { assert(m_stop_reply); @@ -75,8 +75,9 @@ public: llvm::Error SendMessage(llvm::StringRef message, std::string &response_string, PacketResult expected_result); - template <typename P> - llvm::Expected<typename P::result_type> SendMessage(llvm::StringRef Message); + template <typename P, typename... CreateArgs> + llvm::Expected<typename P::result_type> SendMessage(llvm::StringRef Message, + CreateArgs &&... Args); unsigned int GetPcRegisterId(); private: @@ -97,13 +98,13 @@ private: unsigned int m_pc_register = LLDB_INVALID_REGNUM; }; -template <typename P> +template <typename P, typename... CreateArgs> llvm::Expected<typename P::result_type> -TestClient::SendMessage(llvm::StringRef Message) { +TestClient::SendMessage(llvm::StringRef Message, CreateArgs &&... Args) { std::string ResponseText; if (llvm::Error E = SendMessage(Message, ResponseText)) return std::move(E); - return P::create(ResponseText); + return P::create(ResponseText, std::forward<CreateArgs>(Args)...); } } // namespace llgs_tests Modified: lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp?rev=325511&r1=325510&r2=325511&view=diff ============================================================================== --- lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp (original) +++ lldb/trunk/unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp Mon Feb 19 07:42:48 2018 @@ -9,13 +9,19 @@ #include "TestBase.h" #include "TestClient.h" +#include "lldb/Utility/DataExtractor.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" #include "llvm/Testing/Support/Error.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include <string> using namespace llgs_tests; +using namespace lldb_private; using namespace llvm; +using namespace lldb; +using namespace testing; TEST_F(StandardStartupTest, TestStopReplyContainsThreadPcs) { // This inferior spawns 4 threads, then forces a break. @@ -29,7 +35,7 @@ TEST_F(StandardStartupTest, TestStopRepl ASSERT_NE(pc_reg, UINT_MAX); auto jthreads_info = Client->GetJThreadsInfo(); - ASSERT_TRUE(jthreads_info); + ASSERT_THAT_EXPECTED(jthreads_info, Succeeded()); auto stop_reply = Client->GetLatestStopReplyAs<StopReplyStop>(); ASSERT_THAT_EXPECTED(stop_reply, Succeeded()); @@ -42,9 +48,7 @@ TEST_F(StandardStartupTest, TestStopRepl unsigned long tid = stop_reply_pc.first; ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end()) << "Thread ID: " << tid << " not in JThreadsInfo."; - auto pc_value = thread_infos[tid].ReadRegisterAsUint64(pc_reg); - ASSERT_THAT_EXPECTED(pc_value, Succeeded()); - ASSERT_EQ(stop_reply_pc.second, *pc_value) - << "Mismatched PC for thread: " << tid; + EXPECT_THAT(thread_infos[tid].ReadRegister(pc_reg), + Pointee(Eq(stop_reply_pc.second))); } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
