JDevlieghere created this revision.
JDevlieghere added a reviewer: labath.
Herald added a subscriber: abidh.
Herald added a project: LLDB.
When running the test suite with always capture on, a handful of tests are
failing because they have multiple targets and therefore multiple GDB remote
connections. The current reproducer infrastructure is capable of dealing with
that.
This patch reworks the GDB remote provider to support multiple GDB remote
connections, similar to how the reproducers support shadowing multiple command
interpreter inputs. The provider now keeps a list of packet recorders which
deal with a single GDB remote connection. During replay we rely on the order of
creation to match the number of packets to the GDB remote connection.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D71105
Files:
lldb/include/lldb/Utility/GDBRemote.h
lldb/include/lldb/Utility/Reproducer.h
lldb/source/API/SBDebugger.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Utility/GDBRemote.cpp
lldb/source/Utility/Reproducer.cpp
Index: lldb/source/Utility/Reproducer.cpp
===================================================================
--- lldb/source/Utility/Reproducer.cpp
+++ lldb/source/Utility/Reproducer.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "lldb/Utility/Reproducer.h"
+#include "lldb/Utility/GDBRemote.h"
#include "lldb/Utility/LLDBAssert.h"
#include "llvm/Support/FileSystem.h"
@@ -255,7 +256,7 @@
DataRecorder *CommandProvider::GetNewDataRecorder() {
std::size_t i = m_data_recorders.size() + 1;
std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") +
- llvm::Twine(i) + llvm::Twine(".txt"))
+ llvm::Twine(i) + llvm::Twine(".yaml"))
.str();
auto recorder_or_error =
DataRecorder::Create(GetRoot().CopyByAppendingPathComponent(filename));
@@ -304,47 +305,63 @@
os << m_cwd << "\n";
}
-llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() {
- FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file);
+void ProcessGDBRemoteProvider::Keep() {
+ std::vector<std::string> files;
+ for (auto &recorder : m_packet_recorders) {
+ files.push_back(recorder->GetFilename().GetPath());
+ }
- std::error_code EC;
- m_stream_up = std::make_unique<raw_fd_ostream>(history_file.GetPath(), EC,
- sys::fs::OpenFlags::OF_Text);
- return m_stream_up.get();
+ FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file);
+ std::error_code ec;
+ llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text);
+ if (ec)
+ return;
+ yaml::Output yout(os);
+ yout << files;
}
-std::unique_ptr<CommandLoader> CommandLoader::Create(Loader *loader) {
- if (!loader)
- return {};
-
- FileSpec file = loader->GetFile<repro::CommandProvider::Info>();
- if (!file)
- return {};
+void ProcessGDBRemoteProvider::Discard() { m_packet_recorders.clear(); }
- auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
- if (auto err = error_or_file.getError())
- return {};
-
- std::vector<std::string> files;
- llvm::yaml::Input yin((*error_or_file)->getBuffer());
- yin >> files;
-
- if (auto err = yin.error())
- return {};
+llvm::Expected<std::unique_ptr<PacketRecorder>>
+PacketRecorder::Create(const FileSpec &filename) {
+ std::error_code ec;
+ auto recorder = std::make_unique<PacketRecorder>(std::move(filename), ec);
+ if (ec)
+ return llvm::errorCodeToError(ec);
+ return std::move(recorder);
+}
- for (auto &file : files) {
- FileSpec absolute_path =
- loader->GetRoot().CopyByAppendingPathComponent(file);
- file = absolute_path.GetPath();
+PacketRecorder *ProcessGDBRemoteProvider::GetNewPacketRecorder() {
+ std::size_t i = m_packet_recorders.size() + 1;
+ std::string filename = (llvm::Twine(Info::name) + llvm::Twine("-") +
+ llvm::Twine(i) + llvm::Twine(".yaml"))
+ .str();
+ auto recorder_or_error =
+ PacketRecorder::Create(GetRoot().CopyByAppendingPathComponent(filename));
+ if (!recorder_or_error) {
+ llvm::consumeError(recorder_or_error.takeError());
+ return nullptr;
}
- return std::make_unique<CommandLoader>(std::move(files));
+ m_packet_recorders.push_back(std::move(*recorder_or_error));
+ return m_packet_recorders.back().get();
}
-llvm::Optional<std::string> CommandLoader::GetNextFile() {
- if (m_index >= m_files.size())
- return {};
- return m_files[m_index++];
+void PacketRecorder::Record(const GDBRemotePacket &packet) {
+ if (!m_record)
+ return;
+ yaml::Output yout(m_os);
+ yout << const_cast<GDBRemotePacket &>(packet);
+ m_os.flush();
+}
+
+llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() {
+ FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file);
+
+ std::error_code EC;
+ m_stream_up = std::make_unique<raw_fd_ostream>(history_file.GetPath(), EC,
+ sys::fs::OpenFlags::OF_Text);
+ return m_stream_up.get();
}
void ProviderBase::anchor() {}
Index: lldb/source/Utility/GDBRemote.cpp
===================================================================
--- lldb/source/Utility/GDBRemote.cpp
+++ lldb/source/Utility/GDBRemote.cpp
@@ -45,12 +45,6 @@
return bytes_written;
}
-void GDBRemotePacket::Serialize(raw_ostream &strm) const {
- yaml::Output yout(strm);
- yout << const_cast<GDBRemotePacket &>(*this);
- strm.flush();
-}
-
llvm::StringRef GDBRemotePacket::GetTypeStr() const {
switch (type) {
case GDBRemotePacket::ePacketTypeSend:
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -281,10 +281,7 @@
if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) {
repro::ProcessGDBRemoteProvider &provider =
g->GetOrCreate<repro::ProcessGDBRemoteProvider>();
- // Set the history stream to the stream owned by the provider.
- m_gdb_comm.SetHistoryStream(provider.GetHistoryStream());
- // Make sure to clear the stream again when we're finished.
- provider.SetCallback([&]() { m_gdb_comm.SetHistoryStream(nullptr); });
+ m_gdb_comm.SetPacketRecorder(provider.GetNewPacketRecorder());
}
Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_ASYNC));
@@ -3362,17 +3359,21 @@
if (!loader)
return Status("No loader provided.");
- // Construct replay history path.
- FileSpec history_file =
- loader->GetFile<repro::ProcessGDBRemoteProvider::Info>();
- if (!history_file)
- return Status("No provider for gdb-remote.");
+ static std::unique_ptr<repro::MultiLoader<repro::ProcessGDBRemoteProvider>>
+ multi_loader =
+ repro::MultiLoader<repro::ProcessGDBRemoteProvider>::Create(
+ repro::Reproducer::Instance().GetLoader());
- // Enable replay mode.
- m_replay_mode = true;
+ if (!multi_loader)
+ return Status("No gdb remote provider found.");
+
+ llvm::Optional<std::string> history_file = multi_loader->GetNextFile();
+ if (!history_file)
+ return Status("No gdb remote packet log found.");
// Load replay history.
- if (auto error = m_gdb_replay_server.LoadReplayHistory(history_file))
+ if (auto error =
+ m_gdb_replay_server.LoadReplayHistory(FileSpec(*history_file)))
return Status("Unable to load replay history");
// Make a local connection.
@@ -3380,6 +3381,9 @@
m_gdb_replay_server))
return Status("Unable to connect to replay server");
+ // Enable replay mode.
+ m_replay_mode = true;
+
// Start server thread.
m_gdb_replay_server.StartAsyncThread();
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
@@ -13,11 +13,15 @@
#include <vector>
#include "lldb/Utility/GDBRemote.h"
+#include "lldb/Utility/Reproducer.h"
#include "lldb/lldb-public.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
namespace lldb_private {
+namespace repro {
+class PacketRecorder;
+}
namespace process_gdb_remote {
/// The history keeps a circular buffer of GDB remote packets. The history is
@@ -41,7 +45,7 @@
void Dump(Log *log) const;
bool DidDumpToLog() const { return m_dumped_to_log; }
- void SetStream(llvm::raw_ostream *strm) { m_stream = strm; }
+ void SetRecorder(repro::PacketRecorder *recorder) { m_recorder = recorder; }
private:
uint32_t GetFirstSavedPacketIndex() const {
@@ -73,7 +77,7 @@
uint32_t m_curr_idx;
uint32_t m_total_packet_count;
mutable bool m_dumped_to_log;
- llvm::raw_ostream *m_stream = nullptr;
+ repro::PacketRecorder *m_recorder = nullptr;
};
} // namespace process_gdb_remote
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
@@ -40,8 +40,8 @@
m_packets[idx].bytes_transmitted = bytes_transmitted;
m_packets[idx].packet_idx = m_total_packet_count;
m_packets[idx].tid = llvm::get_threadid();
- if (m_stream)
- m_packets[idx].Serialize(*m_stream);
+ if (m_recorder)
+ m_recorder->Record(m_packets[idx]);
}
void GDBRemoteCommunicationHistory::AddPacket(const std::string &src,
@@ -58,8 +58,8 @@
m_packets[idx].bytes_transmitted = bytes_transmitted;
m_packets[idx].packet_idx = m_total_packet_count;
m_packets[idx].tid = llvm::get_threadid();
- if (m_stream)
- m_packets[idx].Serialize(*m_stream);
+ if (m_recorder)
+ m_recorder->Record(m_packets[idx]);
}
void GDBRemoteCommunicationHistory::Dump(Stream &strm) const {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -27,6 +27,9 @@
#include "lldb/lldb-public.h"
namespace lldb_private {
+namespace repro {
+class PacketRecorder;
+}
namespace process_gdb_remote {
enum GDBStoppointType {
@@ -133,7 +136,8 @@
// fork/exec to avoid having to connect/accept
void DumpHistory(Stream &strm);
- void SetHistoryStream(llvm::raw_ostream *strm);
+
+ void SetPacketRecorder(repro::PacketRecorder *recorder);
static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
GDBRemoteCommunication &server);
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
@@ -31,6 +31,7 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/RegularExpression.h"
+#include "lldb/Utility/Reproducer.h"
#include "lldb/Utility/StreamString.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -1243,8 +1244,9 @@
void GDBRemoteCommunication::DumpHistory(Stream &strm) { m_history.Dump(strm); }
-void GDBRemoteCommunication::SetHistoryStream(llvm::raw_ostream *strm) {
- m_history.SetStream(strm);
+void GDBRemoteCommunication::SetPacketRecorder(
+ repro::PacketRecorder *recorder) {
+ m_history.SetRecorder(recorder);
}
llvm::Error
Index: lldb/source/Commands/CommandObjectReproducer.cpp
===================================================================
--- lldb/source/Commands/CommandObjectReproducer.cpp
+++ lldb/source/Commands/CommandObjectReproducer.cpp
@@ -408,8 +408,9 @@
}
case eReproducerProviderCommands: {
// Create a new command loader.
- std::unique_ptr<repro::CommandLoader> command_loader =
- repro::CommandLoader::Create(loader);
+ std::unique_ptr<repro::MultiLoader<repro::CommandProvider>>
+ command_loader =
+ repro::MultiLoader<repro::CommandProvider>::Create(loader);
if (!command_loader) {
SetError(result,
make_error<StringError>(llvm::inconvertibleErrorCode(),
@@ -436,24 +437,34 @@
return true;
}
case eReproducerProviderGDB: {
- FileSpec gdb_file = loader->GetFile<ProcessGDBRemoteProvider::Info>();
- auto error_or_file = MemoryBuffer::getFile(gdb_file.GetPath());
- if (auto err = error_or_file.getError()) {
- SetError(result, errorCodeToError(err));
- return false;
- }
- std::vector<GDBRemotePacket> packets;
- yaml::Input yin((*error_or_file)->getBuffer());
- yin >> packets;
+ static std::unique_ptr<
+ repro::MultiLoader<repro::ProcessGDBRemoteProvider>>
+ multi_loader =
+ repro::MultiLoader<repro::ProcessGDBRemoteProvider>::Create(
+ loader);
+ llvm::Optional<std::string> gdb_file = multi_loader->GetNextFile();
+ while (gdb_file) {
+ auto error_or_file = MemoryBuffer::getFile(*gdb_file);
+ if (auto err = error_or_file.getError()) {
+ SetError(result, errorCodeToError(err));
+ return false;
+ }
- if (auto err = yin.error()) {
- SetError(result, errorCodeToError(err));
- return false;
- }
+ std::vector<GDBRemotePacket> packets;
+ yaml::Input yin((*error_or_file)->getBuffer());
+ yin >> packets;
+
+ if (auto err = yin.error()) {
+ SetError(result, errorCodeToError(err));
+ return false;
+ }
+
+ for (GDBRemotePacket &packet : packets) {
+ packet.Dump(result.GetOutputStream());
+ }
- for (GDBRemotePacket &packet : packets) {
- packet.Dump(result.GetOutputStream());
+ gdb_file = multi_loader->GetNextFile();
}
result.SetStatus(eReturnStatusSuccessFinishResult);
Index: lldb/source/API/SBDebugger.cpp
===================================================================
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -315,8 +315,9 @@
FileSP file_sp = file.m_opaque_sp;
- static std::unique_ptr<repro::CommandLoader> loader =
- repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader());
+ static std::unique_ptr<repro::MultiLoader<repro::CommandProvider>> loader =
+ repro::MultiLoader<repro::CommandProvider>::Create(
+ repro::Reproducer::Instance().GetLoader());
if (loader) {
llvm::Optional<std::string> nextfile = loader->GetNextFile();
FILE *fh = nextfile ? FileSystem::Instance().Fopen(nextfile->c_str(), "r")
Index: lldb/include/lldb/Utility/Reproducer.h
===================================================================
--- lldb/include/lldb/Utility/Reproducer.h
+++ lldb/include/lldb/Utility/Reproducer.h
@@ -20,6 +20,7 @@
#include <vector>
namespace lldb_private {
+struct GDBRemotePacket;
namespace repro {
class Reproducer;
@@ -153,6 +154,30 @@
static char ID;
};
+class PacketRecorder {
+public:
+ PacketRecorder(const FileSpec &filename, std::error_code &ec)
+ : m_filename(filename.GetFilename().GetStringRef()),
+ m_os(filename.GetPath(), ec, llvm::sys::fs::OF_Text), m_record(true) {}
+
+ static llvm::Expected<std::unique_ptr<PacketRecorder>>
+ Create(const FileSpec &filename);
+
+ const FileSpec &GetFilename() { return m_filename; }
+
+ void Record(const GDBRemotePacket &packet);
+
+ void Stop() {
+ assert(m_record);
+ m_record = false;
+ }
+
+private:
+ FileSpec m_filename;
+ llvm::raw_fd_ostream m_os;
+ bool m_record;
+};
+
class DataRecorder {
public:
DataRecorder(const FileSpec &filename, std::error_code &ec)
@@ -215,19 +240,22 @@
ProcessGDBRemoteProvider(const FileSpec &directory) : Provider(directory) {}
llvm::raw_ostream *GetHistoryStream();
+ PacketRecorder *GetNewPacketRecorder();
void SetCallback(std::function<void()> callback) {
m_callback = std::move(callback);
}
- void Keep() override { m_callback(); }
- void Discard() override { m_callback(); }
+ void Keep() override;
+ void Discard() override;
static char ID;
private:
std::function<void()> m_callback;
std::unique_ptr<llvm::raw_fd_ostream> m_stream_up;
+
+ std::vector<std::unique_ptr<PacketRecorder>> m_packet_recorders;
};
/// The generator is responsible for the logic needed to generate a
@@ -359,15 +387,48 @@
mutable std::mutex m_mutex;
};
-/// Helper class for replaying commands through the reproducer.
-class CommandLoader {
+template <typename T> class MultiLoader {
public:
- CommandLoader(std::vector<std::string> files) : m_files(files) {}
+ MultiLoader(std::vector<std::string> files) : m_files(files) {}
+
+ static std::unique_ptr<MultiLoader> Create(Loader *loader) {
+ if (!loader)
+ return {};
+
+ FileSpec file = loader->GetFile<typename T::Info>();
+ if (!file)
+ return {};
+
+ auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath());
+ if (auto err = error_or_file.getError())
+ return {};
- static std::unique_ptr<CommandLoader> Create(Loader *loader);
- llvm::Optional<std::string> GetNextFile();
+ std::vector<std::string> files;
+ llvm::yaml::Input yin((*error_or_file)->getBuffer());
+ yin >> files;
+
+ if (auto err = yin.error())
+ return {};
+
+ for (auto &file : files) {
+ FileSpec absolute_path =
+ loader->GetRoot().CopyByAppendingPathComponent(file);
+ file = absolute_path.GetPath();
+ }
+
+ return std::make_unique<MultiLoader<T>>(std::move(files));
+ }
+
+ llvm::Optional<std::string> GetNextFile() {
+ if (m_index >= m_files.size())
+ return {};
+ return m_files[m_index++];
+ }
private:
+ FileSpec GetMainFile(Loader *loader) {
+ return loader->template GetFile<typename T::Info>();
+ }
std::vector<std::string> m_files;
unsigned m_index = 0;
};
Index: lldb/include/lldb/Utility/GDBRemote.h
===================================================================
--- lldb/include/lldb/Utility/GDBRemote.h
+++ lldb/include/lldb/Utility/GDBRemote.h
@@ -69,7 +69,6 @@
std::string data;
};
- void Serialize(llvm::raw_ostream &strm) const;
void Dump(Stream &strm) const;
BinaryData packet;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
