JDevlieghere created this revision. JDevlieghere added a reviewer: LLDB. Herald added a project: LLDB.
To support dumping the reproducer's GDB remote packets, we need the (de)serialization logic to live in the reproducer rather than the GDB remote plugin. This patch moves the corresponding code in the reproducer and updates its uses in the GDBRemoteCommunicationHistory and the GDBRemoteCommunicationReplayServer. Repository: rLLDB LLDB https://reviews.llvm.org/D67523 Files: lldb/include/lldb/Utility/Reproducer.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h lldb/source/Utility/Reproducer.cpp
Index: lldb/source/Utility/Reproducer.cpp =================================================================== --- lldb/source/Utility/Reproducer.cpp +++ lldb/source/Utility/Reproducer.cpp @@ -329,3 +329,45 @@ const char *ProcessGDBRemoteProvider::Info::name = "gdb-remote"; const char *VersionProvider::Info::file = "version.txt"; const char *VersionProvider::Info::name = "version"; + +void GDBRemotePacket::Serialize(raw_ostream &strm) const { + yaml::Output yout(strm); + yout << const_cast<GDBRemotePacket &>(*this); + strm.flush(); +} + +void yaml::ScalarEnumerationTraits<GDBRemotePacket::Type>::enumeration( + IO &io, GDBRemotePacket::Type &value) { + io.enumCase(value, "Invalid", GDBRemotePacket::ePacketTypeInvalid); + io.enumCase(value, "Send", GDBRemotePacket::ePacketTypeSend); + io.enumCase(value, "Recv", GDBRemotePacket::ePacketTypeRecv); +} + +void yaml::ScalarTraits<GDBRemotePacket::BinaryData>::output( + const GDBRemotePacket::BinaryData &Val, void *, raw_ostream &Out) { + Out << toHex(Val.data); +} + +StringRef yaml::ScalarTraits<GDBRemotePacket::BinaryData>::input( + StringRef Scalar, void *, GDBRemotePacket::BinaryData &Val) { + Val.data = fromHex(Scalar); + return {}; +} + +void yaml::MappingTraits<GDBRemotePacket>::mapping(IO &io, + GDBRemotePacket &Packet) { + io.mapRequired("packet", Packet.packet); + io.mapRequired("type", Packet.type); + io.mapRequired("bytes", Packet.bytes_transmitted); + io.mapRequired("index", Packet.packet_idx); + io.mapRequired("tid", Packet.tid); +} + +StringRef +yaml::MappingTraits<GDBRemotePacket>::validate(IO &io, + GDBRemotePacket &Packet) { + if (Packet.bytes_transmitted != Packet.packet.data.size()) + return "BinaryData size doesn't match bytes transmitted"; + + return {}; +} Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h @@ -62,7 +62,7 @@ static lldb::thread_result_t AsyncThread(void *arg); /// Replay history with the oldest packet at the end. - std::vector<GDBRemoteCommunicationHistory::Entry> m_packet_history; + std::vector<repro::GDBRemotePacket> m_packet_history; /// Server thread. Broadcaster m_async_broadcaster; Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp @@ -29,6 +29,7 @@ using namespace llvm; using namespace lldb; using namespace lldb_private; +using namespace lldb_private::repro; using namespace lldb_private::process_gdb_remote; /// Check if the given expected packet matches the actual packet. @@ -128,7 +129,7 @@ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); while (!m_packet_history.empty()) { // Pop last packet from the history. - GDBRemoteCommunicationHistory::Entry entry = m_packet_history.back(); + GDBRemotePacket entry = m_packet_history.back(); m_packet_history.pop_back(); // We've handled the handshake implicitly before. Skip the packet and move @@ -136,7 +137,7 @@ if (entry.packet.data == "+") continue; - if (entry.type == GDBRemoteCommunicationHistory::ePacketTypeSend) { + if (entry.type == GDBRemotePacket::ePacketTypeSend) { if (unexpected(entry.packet.data, packet.GetStringRef())) { LLDB_LOG(log, "GDBRemoteCommunicationReplayServer expected packet: '{0}'", @@ -150,14 +151,14 @@ // Ignore QEnvironment packets as they're handled earlier. if (entry.packet.data.find("QEnvironment") == 1) { assert(m_packet_history.back().type == - GDBRemoteCommunicationHistory::ePacketTypeRecv); + GDBRemotePacket::ePacketTypeRecv); m_packet_history.pop_back(); } continue; } - if (entry.type == GDBRemoteCommunicationHistory::ePacketTypeInvalid) { + if (entry.type == GDBRemotePacket::ePacketTypeInvalid) { LLDB_LOG( log, "GDBRemoteCommunicationReplayServer skipped invalid packet: '{0}'", @@ -176,10 +177,6 @@ return packet_result; } -LLVM_YAML_IS_DOCUMENT_LIST_VECTOR( - std::vector< - lldb_private::process_gdb_remote::GDBRemoteCommunicationHistory::Entry>) - llvm::Error GDBRemoteCommunicationReplayServer::LoadReplayHistory(const FileSpec &path) { auto error_or_file = MemoryBuffer::getFile(path.GetPath()); Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h @@ -12,6 +12,7 @@ #include <string> #include <vector> +#include "lldb/Utility/Reproducer.h" #include "lldb/lldb-public.h" #include "llvm/Support/YAMLTraits.h" #include "llvm/Support/raw_ostream.h" @@ -25,46 +26,17 @@ public: friend llvm::yaml::MappingTraits<GDBRemoteCommunicationHistory>; - enum PacketType { ePacketTypeInvalid = 0, ePacketTypeSend, ePacketTypeRecv }; - - /// Entry in the ring buffer containing the packet data, its type, size and - /// index. Entries can be serialized to file. - struct Entry { - Entry() - : packet(), type(ePacketTypeInvalid), bytes_transmitted(0), - packet_idx(0), tid(LLDB_INVALID_THREAD_ID) {} - - void Clear() { - packet.data.clear(); - type = ePacketTypeInvalid; - bytes_transmitted = 0; - packet_idx = 0; - tid = LLDB_INVALID_THREAD_ID; - } - - struct BinaryData { - std::string data; - }; - - void Serialize(llvm::raw_ostream &strm) const; - - BinaryData packet; - PacketType type; - uint32_t bytes_transmitted; - uint32_t packet_idx; - lldb::tid_t tid; - }; - GDBRemoteCommunicationHistory(uint32_t size = 0); ~GDBRemoteCommunicationHistory(); // For single char packets for ack, nack and /x03 - void AddPacket(char packet_char, PacketType type, uint32_t bytes_transmitted); - - void AddPacket(const std::string &src, uint32_t src_len, PacketType type, + void AddPacket(char packet_char, repro::GDBRemotePacket::Type type, uint32_t bytes_transmitted); + void AddPacket(const std::string &src, uint32_t src_len, + repro::GDBRemotePacket::Type type, uint32_t bytes_transmitted); + void Dump(Stream &strm) const; void Dump(Log *log) const; bool DidDumpToLog() const { return m_dumped_to_log; } @@ -97,7 +69,7 @@ return m_packets.empty() ? 0 : i % m_packets.size(); } - std::vector<Entry> m_packets; + std::vector<repro::GDBRemotePacket> m_packets; uint32_t m_curr_idx; uint32_t m_total_packet_count; mutable bool m_dumped_to_log; @@ -107,49 +79,4 @@ } // namespace process_gdb_remote } // namespace lldb_private -LLVM_YAML_IS_DOCUMENT_LIST_VECTOR( - lldb_private::process_gdb_remote::GDBRemoteCommunicationHistory::Entry) - -namespace llvm { -namespace yaml { - -template <> -struct ScalarEnumerationTraits<lldb_private::process_gdb_remote:: - GDBRemoteCommunicationHistory::PacketType> { - static void enumeration(IO &io, - lldb_private::process_gdb_remote:: - GDBRemoteCommunicationHistory::PacketType &value); -}; - -template <> -struct ScalarTraits<lldb_private::process_gdb_remote:: - GDBRemoteCommunicationHistory::Entry::BinaryData> { - static void output(const lldb_private::process_gdb_remote:: - GDBRemoteCommunicationHistory::Entry::BinaryData &, - void *, raw_ostream &); - - static StringRef - input(StringRef, void *, - lldb_private::process_gdb_remote::GDBRemoteCommunicationHistory::Entry:: - BinaryData &); - - static QuotingType mustQuote(StringRef S) { return QuotingType::None; } -}; - -template <> -struct MappingTraits< - lldb_private::process_gdb_remote::GDBRemoteCommunicationHistory::Entry> { - static void - mapping(IO &io, - lldb_private::process_gdb_remote::GDBRemoteCommunicationHistory::Entry - &Entry); - - static StringRef validate( - IO &io, - lldb_private::process_gdb_remote::GDBRemoteCommunicationHistory::Entry &); -}; - -} // namespace yaml -} // namespace llvm - #endif // liblldb_GDBRemoteCommunicationHistory_h_ Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp @@ -16,14 +16,9 @@ using namespace llvm; using namespace lldb; using namespace lldb_private; +using namespace lldb_private::repro; using namespace lldb_private::process_gdb_remote; -void GDBRemoteCommunicationHistory::Entry::Serialize(raw_ostream &strm) const { - yaml::Output yout(strm); - yout << const_cast<GDBRemoteCommunicationHistory::Entry &>(*this); - strm.flush(); -} - GDBRemoteCommunicationHistory::GDBRemoteCommunicationHistory(uint32_t size) : m_packets(), m_curr_idx(0), m_total_packet_count(0), m_dumped_to_log(false) { @@ -33,7 +28,8 @@ GDBRemoteCommunicationHistory::~GDBRemoteCommunicationHistory() {} -void GDBRemoteCommunicationHistory::AddPacket(char packet_char, PacketType type, +void GDBRemoteCommunicationHistory::AddPacket(char packet_char, + GDBRemotePacket::Type type, uint32_t bytes_transmitted) { const size_t size = m_packets.size(); if (size == 0) @@ -50,7 +46,8 @@ } void GDBRemoteCommunicationHistory::AddPacket(const std::string &src, - uint32_t src_len, PacketType type, + uint32_t src_len, + GDBRemotePacket::Type type, uint32_t bytes_transmitted) { const size_t size = m_packets.size(); if (size == 0) @@ -72,12 +69,14 @@ const uint32_t stop_idx = m_curr_idx + size; for (uint32_t i = first_idx; i < stop_idx; ++i) { const uint32_t idx = NormalizeIndex(i); - const Entry &entry = m_packets[idx]; - if (entry.type == ePacketTypeInvalid || entry.packet.data.empty()) + const GDBRemotePacket &entry = m_packets[idx]; + if (entry.type == GDBRemotePacket::ePacketTypeInvalid || + entry.packet.data.empty()) break; strm.Printf("history[%u] tid=0x%4.4" PRIx64 " <%4u> %s packet: %s\n", entry.packet_idx, entry.tid, entry.bytes_transmitted, - (entry.type == ePacketTypeSend) ? "send" : "read", + (entry.type == GDBRemotePacket::ePacketTypeSend) ? "send" + : "read", entry.packet.data.c_str()); } } @@ -92,51 +91,15 @@ const uint32_t stop_idx = m_curr_idx + size; for (uint32_t i = first_idx; i < stop_idx; ++i) { const uint32_t idx = NormalizeIndex(i); - const Entry &entry = m_packets[idx]; - if (entry.type == ePacketTypeInvalid || entry.packet.data.empty()) + const GDBRemotePacket &entry = m_packets[idx]; + if (entry.type == GDBRemotePacket::ePacketTypeInvalid || + entry.packet.data.empty()) break; LLDB_LOGF(log, "history[%u] tid=0x%4.4" PRIx64 " <%4u> %s packet: %s", entry.packet_idx, entry.tid, entry.bytes_transmitted, - (entry.type == ePacketTypeSend) ? "send" : "read", + (entry.type == GDBRemotePacket::ePacketTypeSend) ? "send" + : "read", entry.packet.data.c_str()); } } -void yaml::ScalarEnumerationTraits<GDBRemoteCommunicationHistory::PacketType>:: - enumeration(IO &io, GDBRemoteCommunicationHistory::PacketType &value) { - io.enumCase(value, "Invalid", - GDBRemoteCommunicationHistory::ePacketTypeInvalid); - io.enumCase(value, "Send", GDBRemoteCommunicationHistory::ePacketTypeSend); - io.enumCase(value, "Recv", GDBRemoteCommunicationHistory::ePacketTypeRecv); -} - -void yaml::ScalarTraits<GDBRemoteCommunicationHistory::Entry::BinaryData>:: - output(const GDBRemoteCommunicationHistory::Entry::BinaryData &Val, void *, - raw_ostream &Out) { - Out << toHex(Val.data); -} - -StringRef -yaml::ScalarTraits<GDBRemoteCommunicationHistory::Entry::BinaryData>::input( - StringRef Scalar, void *, - GDBRemoteCommunicationHistory::Entry::BinaryData &Val) { - Val.data = fromHex(Scalar); - return {}; -} - -void yaml::MappingTraits<GDBRemoteCommunicationHistory::Entry>::mapping( - IO &io, GDBRemoteCommunicationHistory::Entry &Entry) { - io.mapRequired("packet", Entry.packet); - io.mapRequired("type", Entry.type); - io.mapRequired("bytes", Entry.bytes_transmitted); - io.mapRequired("index", Entry.packet_idx); - io.mapRequired("tid", Entry.tid); -} - -StringRef yaml::MappingTraits<GDBRemoteCommunicationHistory::Entry>::validate( - IO &io, GDBRemoteCommunicationHistory::Entry &Entry) { - if (Entry.bytes_transmitted != Entry.packet.data.size()) - return "BinaryData size doesn't match bytes transmitted"; - - return {}; -} Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -103,7 +103,7 @@ char ch = '+'; const size_t bytes_written = Write(&ch, 1, status, nullptr); LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, ch); - m_history.AddPacket(ch, GDBRemoteCommunicationHistory::ePacketTypeSend, + m_history.AddPacket(ch, repro::GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; } @@ -114,7 +114,7 @@ char ch = '-'; const size_t bytes_written = Write(&ch, 1, status, nullptr); LLDB_LOGF(log, "<%4" PRIu64 "> send packet: %c", (uint64_t)bytes_written, ch); - m_history.AddPacket(ch, GDBRemoteCommunicationHistory::ePacketTypeSend, + m_history.AddPacket(ch, repro::GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; } @@ -178,8 +178,7 @@ } m_history.AddPacket(packet.str(), packet_length, - GDBRemoteCommunicationHistory::ePacketTypeSend, - bytes_written); + repro::GDBRemotePacket::ePacketTypeSend, bytes_written); if (bytes_written == packet_length) { if (!skip_ack && GetSendAcks()) @@ -809,7 +808,7 @@ } m_history.AddPacket(m_bytes, total_length, - GDBRemoteCommunicationHistory::ePacketTypeRecv, + repro::GDBRemotePacket::ePacketTypeRecv, total_length); // Copy the packet from m_bytes to packet_str expanding the run-length Index: lldb/include/lldb/Utility/Reproducer.h =================================================================== --- lldb/include/lldb/Utility/Reproducer.h +++ lldb/include/lldb/Utility/Reproducer.h @@ -10,6 +10,7 @@ #define LLDB_UTILITY_REPRODUCER_H #include "lldb/Utility/FileSpec.h" +#include "lldb/lldb-public.h" #include "llvm/ADT/DenseMap.h" #include "llvm/Support/Error.h" @@ -210,6 +211,39 @@ std::unique_ptr<llvm::raw_fd_ostream> m_stream_up; }; +/// GDB remote packet as used by the reproducer and the GDB remote +/// communication history. Packets can be serialized to file. +struct GDBRemotePacket { + + friend llvm::yaml::MappingTraits<GDBRemotePacket>; + + enum Type { ePacketTypeInvalid = 0, ePacketTypeSend, ePacketTypeRecv }; + + GDBRemotePacket() + : packet(), type(ePacketTypeInvalid), bytes_transmitted(0), packet_idx(0), + tid(LLDB_INVALID_THREAD_ID) {} + + void Clear() { + packet.data.clear(); + type = ePacketTypeInvalid; + bytes_transmitted = 0; + packet_idx = 0; + tid = LLDB_INVALID_THREAD_ID; + } + + struct BinaryData { + std::string data; + }; + + void Serialize(llvm::raw_ostream &strm) const; + + BinaryData packet; + Type type; + uint32_t bytes_transmitted; + uint32_t packet_idx; + lldb::tid_t tid; +}; + /// The generator is responsible for the logic needed to generate a /// reproducer. For doing so it relies on providers, who serialize data that /// is necessary for reproducing a failure. @@ -343,4 +377,37 @@ } // namespace repro } // namespace lldb_private +LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(lldb_private::repro::GDBRemotePacket) +LLVM_YAML_IS_DOCUMENT_LIST_VECTOR( + std::vector<lldb_private::repro::GDBRemotePacket>) + +namespace llvm { +namespace yaml { + +template <> +struct ScalarEnumerationTraits<lldb_private::repro::GDBRemotePacket::Type> { + static void enumeration(IO &io, + lldb_private::repro::GDBRemotePacket::Type &value); +}; + +template <> +struct ScalarTraits<lldb_private::repro::GDBRemotePacket::BinaryData> { + static void output(const lldb_private::repro::GDBRemotePacket::BinaryData &, + void *, raw_ostream &); + + static StringRef input(StringRef, void *, + lldb_private::repro::GDBRemotePacket::BinaryData &); + + static QuotingType mustQuote(StringRef S) { return QuotingType::None; } +}; + +template <> struct MappingTraits<lldb_private::repro::GDBRemotePacket> { + static void mapping(IO &io, lldb_private::repro::GDBRemotePacket &Packet); + + static StringRef validate(IO &io, lldb_private::repro::GDBRemotePacket &); +}; + +} // namespace yaml +} // namespace llvm + #endif // LLDB_UTILITY_REPRODUCER_H
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits