From 95a6db9a3a9c183d4c5ef198b28533e96084e8b0 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 11 Sep 2019 23:15:12 +0000 Subject: [PATCH 01/12] [Reproducer] Move GDB Remote Provider into Reproducer (NFC) Originally the idea was for providers to be defined close to where they are used. While this helped designing the providers in such a way that they don't depend on each other, it also means that it's not possible to access them from a central place. This proved to be a problem for some providers and resulted in them living in the reproducer class. The ProcessGDBRemote provider is the last remaining exception. This patch makes things consistent and moves it into the reproducer like the other providers. llvm-svn: 371685 (cherry picked from commit bcc24e46ba3868510ed25cea5ac7b6fc2cc47d73) --- lldb/include/lldb/Utility/Reproducer.h | 27 +++++++++++ .../Process/gdb-remote/ProcessGDBRemote.cpp | 46 ++----------------- lldb/source/Utility/Reproducer.cpp | 14 +++++- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index 670041d06bbac..b2285e96dc281 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -181,10 +181,37 @@ class CommandProvider : public Provider { std::vector> m_data_recorders; }; +class ProcessGDBRemoteProvider + : public repro::Provider { +public: + struct Info { + static const char *name; + static const char *file; + }; + + ProcessGDBRemoteProvider(const FileSpec &directory) : Provider(directory) {} + + llvm::raw_ostream *GetHistoryStream(); + + void SetCallback(std::function callback) { + m_callback = std::move(callback); + } + + void Keep() override { m_callback(); } + void Discard() override { m_callback(); } + + static char ID; + +private: + std::function m_callback; + std::unique_ptr m_stream_up; +}; + /// 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. class Generator final { + public: Generator(const FileSpec &root); ~Generator(); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 7c473634c92bc..c1f22a6f78860 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -164,45 +164,6 @@ static const ProcessKDPPropertiesSP &GetGlobalPluginProperties() { return g_settings_sp; } -class ProcessGDBRemoteProvider - : public repro::Provider { -public: - struct Info { - static const char *name; - static const char *file; - }; - - ProcessGDBRemoteProvider(const FileSpec &directory) : Provider(directory) { - } - - raw_ostream *GetHistoryStream() { - FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file); - - std::error_code EC; - m_stream_up = llvm::make_unique(history_file.GetPath(), EC, - sys::fs::OpenFlags::F_Text); - return m_stream_up.get(); - } - - void SetCallback(std::function callback) { - m_callback = std::move(callback); - } - - void Keep() override { m_callback(); } - - void Discard() override { m_callback(); } - - static char ID; - -private: - std::function m_callback; - std::unique_ptr m_stream_up; -}; - -char ProcessGDBRemoteProvider::ID = 0; -const char *ProcessGDBRemoteProvider::Info::name = "gdb-remote"; -const char *ProcessGDBRemoteProvider::Info::file = "gdb-remote.yaml"; - } // namespace // TODO Randomly assigning a port is unsafe. We should get an unused @@ -312,8 +273,8 @@ ProcessGDBRemote::ProcessGDBRemote(lldb::TargetSP target_sp, "async thread did exit"); if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) { - ProcessGDBRemoteProvider &provider = - g->GetOrCreate(); + repro::ProcessGDBRemoteProvider &provider = + g->GetOrCreate(); // 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. @@ -3424,7 +3385,8 @@ Status ProcessGDBRemote::ConnectToReplayServer(repro::Loader *loader) { return Status("No loader provided."); // Construct replay history path. - FileSpec history_file = loader->GetFile(); + FileSpec history_file = + loader->GetFile(); if (!history_file) return Status("No provider for gdb-remote."); diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp index 479ed311d1ded..ed5d8bead2b61 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -272,14 +272,26 @@ void VersionProvider::Keep() { os << m_version << "\n"; } +llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() { + FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file); + + std::error_code EC; + m_stream_up = std::make_unique(history_file.GetPath(), EC, + sys::fs::OpenFlags::OF_Text); + return m_stream_up.get(); +} + void ProviderBase::anchor() {} -char ProviderBase::ID = 0; char CommandProvider::ID = 0; char FileProvider::ID = 0; +char ProcessGDBRemoteProvider::ID = 0; +char ProviderBase::ID = 0; char VersionProvider::ID = 0; const char *CommandProvider::Info::file = "command-interpreter.yaml"; const char *CommandProvider::Info::name = "command-interpreter"; const char *FileProvider::Info::file = "files.yaml"; const char *FileProvider::Info::name = "files"; +const char *ProcessGDBRemoteProvider::Info::file = "gdb-remote.yaml"; +const char *ProcessGDBRemoteProvider::Info::name = "gdb-remote"; const char *VersionProvider::Info::file = "version.txt"; const char *VersionProvider::Info::name = "version"; From c4287573dfbaa42e8cf0450bfe221b77a1fbea3b Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 11 Sep 2019 23:27:12 +0000 Subject: [PATCH 02/12] [Reproducer] Move the command loader into the reproducer (NFC) This just moves the CommandLoader utility into the reproducer namespace and makes it accessible outside the API layer. This is setting things up for a bigger change. llvm-svn: 371689 (cherry picked from commit 4a491ec4916b960a8bbbdd857a352c38d4404b86) --- lldb/include/lldb/Utility/Reproducer.h | 13 +++++++ lldb/source/API/SBDebugger.cpp | 54 +++----------------------- lldb/source/Utility/Reproducer.cpp | 34 ++++++++++++++++ 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index b2285e96dc281..ce8e7a9de312d 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -324,6 +324,19 @@ class Reproducer { mutable std::mutex m_mutex; }; +/// Helper class for replaying commands through the reproducer. +class CommandLoader { +public: + CommandLoader(std::vector files) : m_files(files) {} + + static std::unique_ptr Create(Loader *loader); + llvm::Optional GetNextFile(); + +private: + std::vector m_files; + unsigned m_index = 0; +}; + } // namespace repro } // namespace lldb_private diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 2b962e3e70b01..4e8a3a737a4bd 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -57,51 +57,6 @@ using namespace lldb; using namespace lldb_private; -/// Helper class for replaying commands through the reproducer. -class CommandLoader { -public: - CommandLoader(std::vector files) : m_files(files) {} - - static std::unique_ptr Create() { - repro::Loader *loader = repro::Reproducer::Instance().GetLoader(); - if (!loader) - return {}; - - FileSpec file = loader->GetFile(); - if (!file) - return {}; - - auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath()); - if (auto err = error_or_file.getError()) - return {}; - - std::vector 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 llvm::make_unique(std::move(files)); - } - - FILE *GetNextFile() { - if (m_index >= m_files.size()) - return nullptr; - return FileSystem::Instance().Fopen(m_files[m_index++].c_str(), "r"); - } - -private: - std::vector m_files; - unsigned m_index = 0; -}; - static llvm::sys::DynamicLibrary LoadPlugin(const lldb::DebuggerSP &debugger_sp, const FileSpec &spec, Status &error) { @@ -349,9 +304,12 @@ void SBDebugger::SetInputFileHandle(FILE *fh, bool transfer_ownership) { if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) recorder = g->GetOrCreate().GetNewDataRecorder(); - static std::unique_ptr loader = CommandLoader::Create(); - if (loader) - fh = loader->GetNextFile(); + static std::unique_ptr loader = + repro::CommandLoader::Create(repro::Reproducer::Instance().GetLoader()); + if (loader) { + llvm::Optional file = loader->GetNextFile(); + fh = file ? FileSystem::Instance().Fopen(file->c_str(), "r") : nullptr; + } m_opaque_sp->SetInputFileHandle(fh, transfer_ownership, recorder); } diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp index ed5d8bead2b61..fd8e21c708879 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -281,6 +281,40 @@ llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() { return m_stream_up.get(); } +std::unique_ptr CommandLoader::Create(Loader *loader) { + if (!loader) + return {}; + + FileSpec file = loader->GetFile(); + if (!file) + return {}; + + auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath()); + if (auto err = error_or_file.getError()) + return {}; + + std::vector 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(std::move(files)); +} + +llvm::Optional CommandLoader::GetNextFile() { + if (m_index >= m_files.size()) + return {}; + return m_files[m_index++]; +} + void ProviderBase::anchor() {} char CommandProvider::ID = 0; char FileProvider::ID = 0; From f50e5b18be52ce8024a00bebf1a79a733a436ff4 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 27 Sep 2019 17:30:40 +0000 Subject: [PATCH 03/12] [Reproducer] Always use absolute paths for capture & replay. The VFS requires files to be have absolute paths. The file collector makes paths relative to the reproducer root. If the root is a relative path, this would trigger an assert in the VFS. This patch ensures that we always make the given path absolute. Thank you Ted Woodward for pointing this out! llvm-svn: 373102 (cherry picked from commit cdec597905cd19324f6702af03c9ec4f3bf910da) --- lldb/include/lldb/Utility/Reproducer.h | 4 ++-- lldb/lit/Reproducer/TestRelativePath.test | 8 ++++++++ lldb/source/Utility/Reproducer.cpp | 13 +++++++++++-- 3 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 lldb/lit/Reproducer/TestRelativePath.test diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index ce8e7a9de312d..19263a437fdd5 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -213,7 +213,7 @@ class ProcessGDBRemoteProvider class Generator final { public: - Generator(const FileSpec &root); + Generator(FileSpec root); ~Generator(); /// Method to indicate we want to keep the reproducer. If reproducer @@ -270,7 +270,7 @@ class Generator final { class Loader final { public: - Loader(const FileSpec &root); + Loader(FileSpec root); template FileSpec GetFile() { if (!HasFile(T::file)) diff --git a/lldb/lit/Reproducer/TestRelativePath.test b/lldb/lit/Reproducer/TestRelativePath.test new file mode 100644 index 0000000000000..1c871ee81e8be --- /dev/null +++ b/lldb/lit/Reproducer/TestRelativePath.test @@ -0,0 +1,8 @@ +# This tests relative capture paths. + +# RUN: mkdir -p %t +# RUN: cd %t +# RUN: rm -rf ./foo +# RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out +# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in -o 'reproducer dump -p files' --capture --capture-path ./foo %t/reproducer.out +# RUN: %lldb --replay ./foo diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp index fd8e21c708879..bbae907507d7c 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -136,7 +136,15 @@ FileSpec Reproducer::GetReproducerPath() const { return {}; } -Generator::Generator(const FileSpec &root) : m_root(root), m_done(false) {} +static FileSpec MakeAbsolute(FileSpec file_spec) { + SmallString<128> path; + file_spec.GetPath(path, false); + llvm::sys::fs::make_absolute(path); + return FileSpec(path, file_spec.GetPathStyle()); +} + +Generator::Generator(FileSpec root) + : m_root(MakeAbsolute(std::move(root))), m_done(false) {} Generator::~Generator() {} @@ -188,7 +196,8 @@ void Generator::AddProvidersToIndex() { yout << files; } -Loader::Loader(const FileSpec &root) : m_root(root), m_loaded(false) {} +Loader::Loader(FileSpec root) + : m_root(MakeAbsolute(std::move(root))), m_loaded(false) {} llvm::Error Loader::LoadIndex() { if (m_loaded) From 258416b4e0913cd4e0c90bfeee22d9339eb2a4ca Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:01:53 +0000 Subject: [PATCH 04/12] [Reproducer] Capture the debugger's working directory This patch extends the reproducer to capture the debugger's current working directory. This information will be used later to set the current working directory of the VFS. llvm-svn: 375059 (cherry picked from commit 27ef81cd484bae8382ed4b68b43a1d5d28d24cb0) --- lldb/include/lldb/Utility/Reproducer.h | 21 +++++++++++++++++++++ lldb/lit/Reproducer/TestWorkingDir.test | 11 +++++++++++ lldb/source/Utility/Reproducer.cpp | 16 +++++++++++++++- 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 lldb/lit/Reproducer/TestWorkingDir.test diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index 19263a437fdd5..c8fee0263d0d7 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -130,6 +130,27 @@ class VersionProvider : public Provider { static char ID; }; +/// Provider for the LLDB current working directroy. +/// +/// When the reproducer is kept, it writes lldb's current working directory to +/// a file named cwd.txt in the reproducer root. +class WorkingDirectoryProvider : public Provider { +public: + WorkingDirectoryProvider(const FileSpec &directory) : Provider(directory) { + llvm::SmallString<128> cwd; + if (std::error_code EC = llvm::sys::fs::current_path(cwd)) + return; + m_cwd = cwd.str(); + } + struct Info { + static const char *name; + static const char *file; + }; + void Keep() override; + std::string m_cwd; + static char ID; +}; + class DataRecorder { public: DataRecorder(const FileSpec &filename, std::error_code &ec) diff --git a/lldb/lit/Reproducer/TestWorkingDir.test b/lldb/lit/Reproducer/TestWorkingDir.test new file mode 100644 index 0000000000000..eb0af843dfb39 --- /dev/null +++ b/lldb/lit/Reproducer/TestWorkingDir.test @@ -0,0 +1,11 @@ +# This tests relative capture paths. + +# RUN: echo "CHECK: %t" > %t.check + +# RUN: rm -rf %t.repro +# RUN: mkdir -p %t.repro +# RUN: mkdir -p %t +# RUN: cd %t +# RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out +# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in --capture --capture-path %t.repro %t/reproducer.out +# RUN: cat %t.repro/cwd.txt | FileCheck %t.check diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp index bbae907507d7c..d5a905001d952 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -144,7 +144,9 @@ static FileSpec MakeAbsolute(FileSpec file_spec) { } Generator::Generator(FileSpec root) - : m_root(MakeAbsolute(std::move(root))), m_done(false) {} + : m_root(MakeAbsolute(std::move(root))), m_done(false) { + GetOrCreate(); +} Generator::~Generator() {} @@ -281,6 +283,15 @@ void VersionProvider::Keep() { os << m_version << "\n"; } +void WorkingDirectoryProvider::Keep() { + 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; + os << m_cwd << "\n"; +} + llvm::raw_ostream *ProcessGDBRemoteProvider::GetHistoryStream() { FileSpec history_file = GetRoot().CopyByAppendingPathComponent(Info::file); @@ -330,6 +341,7 @@ char FileProvider::ID = 0; char ProcessGDBRemoteProvider::ID = 0; char ProviderBase::ID = 0; char VersionProvider::ID = 0; +char WorkingDirectoryProvider::ID = 0; const char *CommandProvider::Info::file = "command-interpreter.yaml"; const char *CommandProvider::Info::name = "command-interpreter"; const char *FileProvider::Info::file = "files.yaml"; @@ -338,3 +350,5 @@ const char *ProcessGDBRemoteProvider::Info::file = "gdb-remote.yaml"; const char *ProcessGDBRemoteProvider::Info::name = "gdb-remote"; const char *VersionProvider::Info::file = "version.txt"; const char *VersionProvider::Info::name = "version"; +const char *WorkingDirectoryProvider::Info::file = "cwd.txt"; +const char *WorkingDirectoryProvider::Info::name = "cwd"; From a0b3c0ce97eef0fadede28d668d2c3cff86dc5ae Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 13 Sep 2019 23:27:31 +0000 Subject: [PATCH 05/12] [Reproducer] Add reproducer dump command. This adds a reproducer dump commands which makes it possible to inspect a reproducer from inside LLDB. Currently it supports the Files, Commands and Version providers. I'm planning to add support for the GDB Remote provider in a follow-up patch. Differential revision: https://reviews.llvm.org/D67474 llvm-svn: 371909 (cherry picked from commit 97fc8eb4382e5cb0af67d82bc108620f32746326) --- lldb/lit/Reproducer/Inputs/FileCapture.in | 1 + lldb/lit/Reproducer/TestDump.test | 21 ++ .../Commands/CommandObjectReproducer.cpp | 237 +++++++++++++++++- lldb/source/Commands/Options.td | 9 + llvm/include/llvm/Support/VirtualFileSystem.h | 3 +- llvm/lib/Support/VirtualFileSystem.cpp | 22 +- 6 files changed, 280 insertions(+), 13 deletions(-) create mode 100644 lldb/lit/Reproducer/TestDump.test diff --git a/lldb/lit/Reproducer/Inputs/FileCapture.in b/lldb/lit/Reproducer/Inputs/FileCapture.in index bf6f852dbedb7..2c69487575e2d 100644 --- a/lldb/lit/Reproducer/Inputs/FileCapture.in +++ b/lldb/lit/Reproducer/Inputs/FileCapture.in @@ -1,3 +1,4 @@ run reproducer status +reproducer dump -p files reproducer generate diff --git a/lldb/lit/Reproducer/TestDump.test b/lldb/lit/Reproducer/TestDump.test new file mode 100644 index 0000000000000..472c563d7f235 --- /dev/null +++ b/lldb/lit/Reproducer/TestDump.test @@ -0,0 +1,21 @@ +# This tests the reproducer dump functionality. + +# Generate a reproducer. +# RUN: mkdir -p %t +# RUN: rm -rf %t.repro +# RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out +# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in -o 'reproducer dump -p files' --capture --capture-path %t.repro %t/reproducer.out + +# RUN: %lldb -b -o 'reproducer dump -p files -f %t.repro' | FileCheck %s --check-prefix FILES +# FILES: 'reproducer.out' +# FILES: 'FileCapture.in' + +# RUN: %lldb -b -o 'reproducer dump -p version -f %t.repro' | FileCheck %s --check-prefix VERSION +# VERSION: lldb version + +# RUN: %lldb -b -o 'reproducer dump -p commands -f %t.repro' | FileCheck %s --check-prefix COMMANDS +# COMMANDS: command source +# COMMANDS: target create +# COMMANDS: command source + +# RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix FILES diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp index 895a9cc0b0d09..fcfeb58751efc 100644 --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -8,6 +8,7 @@ #include "CommandObjectReproducer.h" +#include "lldb/Host/OptionParser.h" #include "lldb/Utility/Reproducer.h" #include "lldb/Interpreter/CommandInterpreter.h" @@ -16,7 +17,52 @@ #include "lldb/Interpreter/OptionGroupBoolean.h" using namespace lldb; +using namespace llvm; using namespace lldb_private; +using namespace lldb_private::repro; + +enum ReproducerProvider { + eReproducerProviderCommands, + eReproducerProviderFiles, + eReproducerProviderGDB, + eReproducerProviderVersion, + eReproducerProviderNone +}; + +static constexpr OptionEnumValueElement g_reproducer_provider_type[] = { + { + eReproducerProviderCommands, + "commands", + "Command Interpreter Commands", + }, + { + eReproducerProviderFiles, + "files", + "Files", + }, + { + eReproducerProviderGDB, + "gdb", + "GDB Remote Packets", + }, + { + eReproducerProviderVersion, + "version", + "Version", + }, + { + eReproducerProviderNone, + "none", + "None", + }, +}; + +static constexpr OptionEnumValues ReproducerProviderType() { + return OptionEnumValues(g_reproducer_provider_type); +} + +#define LLDB_OPTIONS_reproducer +#include "CommandOptions.inc" class CommandObjectReproducerGenerate : public CommandObjectParsed { public: @@ -38,7 +84,7 @@ class CommandObjectReproducerGenerate : public CommandObjectParsed { return false; } - auto &r = repro::Reproducer::Instance(); + auto &r = Reproducer::Instance(); if (auto generator = r.GetGenerator()) { generator->Keep(); } else if (r.GetLoader()) { @@ -84,7 +130,7 @@ class CommandObjectReproducerStatus : public CommandObjectParsed { return false; } - auto &r = repro::Reproducer::Instance(); + auto &r = Reproducer::Instance(); if (r.GetGenerator()) { result.GetOutputStream() << "Reproducer is in capture mode.\n"; } else if (r.GetLoader()) { @@ -98,6 +144,191 @@ class CommandObjectReproducerStatus : public CommandObjectParsed { } }; +static void SetError(CommandReturnObject &result, Error err) { + result.GetErrorStream().Printf("error: %s\n", + toString(std::move(err)).c_str()); + result.SetStatus(eReturnStatusFailed); +} + +class CommandObjectReproducerDump : public CommandObjectParsed { +public: + CommandObjectReproducerDump(CommandInterpreter &interpreter) + : CommandObjectParsed(interpreter, "reproducer dump", + "Dump the information contained in a reproducer.", + nullptr) {} + + ~CommandObjectReproducerDump() override = default; + + Options *GetOptions() override { return &m_options; } + + class CommandOptions : public Options { + public: + CommandOptions() : Options(), file() {} + + ~CommandOptions() override = default; + + Status SetOptionValue(uint32_t option_idx, StringRef option_arg, + ExecutionContext *execution_context) override { + Status error; + const int short_option = m_getopt_table[option_idx].val; + + switch (short_option) { + case 'f': + file.SetFile(option_arg, FileSpec::Style::native); + FileSystem::Instance().Resolve(file); + break; + case 'p': + provider = (ReproducerProvider)OptionArgParser::ToOptionEnum( + option_arg, GetDefinitions()[option_idx].enum_values, 0, error); + if (!error.Success()) + error.SetErrorStringWithFormat("unrecognized value for provider '%s'", + option_arg.str().c_str()); + break; + default: + llvm_unreachable("Unimplemented option"); + } + + return error; + } + + void OptionParsingStarting(ExecutionContext *execution_context) override { + file.Clear(); + provider = eReproducerProviderNone; + } + + ArrayRef GetDefinitions() override { + return makeArrayRef(g_reproducer_options); + } + + FileSpec file; + ReproducerProvider provider = eReproducerProviderNone; + }; + +protected: + bool DoExecute(Args &command, CommandReturnObject &result) override { + if (!command.empty()) { + result.AppendErrorWithFormat("'%s' takes no arguments", + m_cmd_name.c_str()); + return false; + } + + // If no reproducer path is specified, use the loader currently used for + // replay. Otherwise create a new loader just for dumping. + llvm::Optional loader_storage; + Loader *loader = nullptr; + if (!m_options.file) { + loader = Reproducer::Instance().GetLoader(); + if (loader == nullptr) { + result.SetError( + "Not specifying a reproducer is only support during replay."); + result.SetStatus(eReturnStatusSuccessFinishNoResult); + return false; + } + } else { + loader_storage.emplace(m_options.file); + loader = &(*loader_storage); + if (Error err = loader->LoadIndex()) { + SetError(result, std::move(err)); + return false; + } + } + + // If we get here we should have a valid loader. + assert(loader); + + switch (m_options.provider) { + case eReproducerProviderFiles: { + FileSpec vfs_mapping = loader->GetFile(); + + // Read the VFS mapping. + ErrorOr> buffer = + vfs::getRealFileSystem()->getBufferForFile(vfs_mapping.GetPath()); + if (!buffer) { + SetError(result, errorCodeToError(buffer.getError())); + return false; + } + + // Initialize a VFS from the given mapping. + IntrusiveRefCntPtr vfs = vfs::getVFSFromYAML( + std::move(buffer.get()), nullptr, vfs_mapping.GetPath()); + + // Dump the VFS to a buffer. + std::string str; + raw_string_ostream os(str); + static_cast(*vfs).dump(os); + os.flush(); + + // Return the string. + result.AppendMessage(str); + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } + case eReproducerProviderVersion: { + FileSpec version_file = loader->GetFile(); + + // Load the version info into a buffer. + ErrorOr> buffer = + vfs::getRealFileSystem()->getBufferForFile(version_file.GetPath()); + if (!buffer) { + SetError(result, errorCodeToError(buffer.getError())); + return false; + } + + // Return the version string. + StringRef version = (*buffer)->getBuffer(); + result.AppendMessage(version.str()); + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } + case eReproducerProviderCommands: { + // Create a new command loader. + std::unique_ptr command_loader = + repro::CommandLoader::Create(loader); + if (!command_loader) { + SetError(result, + make_error(llvm::inconvertibleErrorCode(), + "Unable to create command loader.")); + return false; + } + + // Iterate over the command files and dump them. + while (true) { + llvm::Optional command_file = + command_loader->GetNextFile(); + if (!command_file) + break; + + auto command_buffer = llvm::MemoryBuffer::getFile(*command_file); + if (auto err = command_buffer.getError()) { + SetError(result, errorCodeToError(err)); + return false; + } + result.AppendMessage((*command_buffer)->getBuffer()); + } + + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } + case eReproducerProviderGDB: { + // FIXME: Dumping the GDB remote packets means moving the + // (de)serialization code out of the GDB-remote plugin. + result.AppendMessage("Dumping GDB remote packets isn't implemented yet."); + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } + case eReproducerProviderNone: + result.SetError("No valid provider specified."); + return false; + } + + result.SetStatus(eReturnStatusSuccessFinishNoResult); + return result.Succeeded(); + } + +private: + CommandOptions m_options; +}; + CommandObjectReproducer::CommandObjectReproducer( CommandInterpreter &interpreter) : CommandObjectMultiword( @@ -109,6 +340,8 @@ CommandObjectReproducer::CommandObjectReproducer( CommandObjectSP(new CommandObjectReproducerGenerate(interpreter))); LoadSubCommand("status", CommandObjectSP( new CommandObjectReproducerStatus(interpreter))); + LoadSubCommand("dump", + CommandObjectSP(new CommandObjectReproducerDump(interpreter))); } CommandObjectReproducer::~CommandObjectReproducer() = default; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index d71029015eade..7791302c3da37 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -442,6 +442,15 @@ let Command = "log" in { Desc<"Prepend the names of files and function that generate the logs.">; } +let Command = "reproducer" in { + def reproducer_provider : Option<"provider", "p">, Group<1>, + EnumArg<"None", "ReproducerProviderType()">, + Required, Desc<"The reproducer provider to dump.">; + def reproducer_file : Option<"file", "f">, Group<1>, Arg<"Filename">, + Desc<"The reproducer path. If a reproducer is replayed and no path is " + "provided, that reproducer is dumped.">; +} + let Command = "memory read" in { def memory_read_num_per_line : Option<"num-per-line", "l">, Group<1>, Arg<"NumberPerLine">, Desc<"The number of items per line to display.">; diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 32656881210c1..c844d9d194f0c 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -739,9 +739,10 @@ class RedirectingFileSystem : public vfs::FileSystem { StringRef getExternalContentsPrefixDir() const; + void dump(raw_ostream &OS) const; + void dumpEntry(raw_ostream &OS, Entry *E, int NumSpaces = 0) const; #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) LLVM_DUMP_METHOD void dump() const; - LLVM_DUMP_METHOD void dumpEntry(Entry *E, int NumSpaces = 0) const; #endif }; diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index bbb21f789b028..f5ba02df965c8 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1104,20 +1104,19 @@ StringRef RedirectingFileSystem::getExternalContentsPrefixDir() const { return ExternalContentsPrefixDir; } -#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) -LLVM_DUMP_METHOD void RedirectingFileSystem::dump() const { +void RedirectingFileSystem::dump(raw_ostream &OS) const { for (const auto &Root : Roots) - dumpEntry(Root.get()); + dumpEntry(OS, Root.get()); } -LLVM_DUMP_METHOD void -RedirectingFileSystem::dumpEntry(RedirectingFileSystem::Entry *E, - int NumSpaces) const { +void RedirectingFileSystem::dumpEntry(raw_ostream &OS, + RedirectingFileSystem::Entry *E, + int NumSpaces) const { StringRef Name = E->getName(); for (int i = 0, e = NumSpaces; i < e; ++i) - dbgs() << " "; - dbgs() << "'" << Name.str().c_str() << "'" - << "\n"; + OS << " "; + OS << "'" << Name.str().c_str() << "'" + << "\n"; if (E->getKind() == RedirectingFileSystem::EK_Directory) { auto *DE = dyn_cast(E); @@ -1125,9 +1124,12 @@ RedirectingFileSystem::dumpEntry(RedirectingFileSystem::Entry *E, for (std::unique_ptr &SubEntry : llvm::make_range(DE->contents_begin(), DE->contents_end())) - dumpEntry(SubEntry.get(), NumSpaces + 2); + dumpEntry(OS, SubEntry.get(), NumSpaces + 2); } } + +#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) +LLVM_DUMP_METHOD void RedirectingFileSystem::dump() const { dump(dbgs()); } #endif /// A helper class to hold the common YAML parsing state. From 074fb071fd951018dc60aef4e5b4e2b4dcba80c1 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Mon, 16 Sep 2019 23:31:06 +0000 Subject: [PATCH 06/12] [Reproducer] Implement dumping packets. This patch completes the dump functionality by adding support for dumping a reproducer's GDB remote packets. Differential revision: https://reviews.llvm.org/D67636 llvm-svn: 372046 (cherry picked from commit 8fc8d3fe0108479ce22234ec829fc7ee2b8f0bd2) --- lldb/lit/Reproducer/TestDump.test | 4 ++++ .../Commands/CommandObjectReproducer.cpp | 24 ++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lldb/lit/Reproducer/TestDump.test b/lldb/lit/Reproducer/TestDump.test index 472c563d7f235..e378043f9c157 100644 --- a/lldb/lit/Reproducer/TestDump.test +++ b/lldb/lit/Reproducer/TestDump.test @@ -18,4 +18,8 @@ # COMMANDS: target create # COMMANDS: command source +# RUN: %lldb -b -o 'reproducer dump -p gdb -f %t.repro' | FileCheck %s --check-prefix GDB +# GDB: send packet: $QStartNoAckMode#b0 +# GDB: read packet: $OK#9a + # RUN: %lldb --replay %t.repro | FileCheck %s --check-prefix FILES diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp index fcfeb58751efc..404702d3640e4 100644 --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -10,6 +10,7 @@ #include "lldb/Host/OptionParser.h" #include "lldb/Utility/Reproducer.h" +#include "lldb/Utility/GDBRemote.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -310,9 +311,26 @@ class CommandObjectReproducerDump : public CommandObjectParsed { return true; } case eReproducerProviderGDB: { - // FIXME: Dumping the GDB remote packets means moving the - // (de)serialization code out of the GDB-remote plugin. - result.AppendMessage("Dumping GDB remote packets isn't implemented yet."); + FileSpec gdb_file = loader->GetFile(); + auto error_or_file = MemoryBuffer::getFile(gdb_file.GetPath()); + if (auto err = error_or_file.getError()) { + SetError(result, errorCodeToError(err)); + return false; + } + + std::vector 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()); + } + result.SetStatus(eReturnStatusSuccessFinishResult); return true; } From 07f1a9467b4bf5d24cfd5996efd57001570a4316 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 9 Oct 2019 21:47:49 +0000 Subject: [PATCH 07/12] [Reproducer] Add convenience methods IsCapturing and IsReplaying. Add convenience methods to the Reproducer class for when you don't need access to the generator and the loader. llvm-svn: 374236 (cherry picked from commit 865cd0932f4483058ccc607b8867a4a5a597ec52) --- lldb/include/lldb/Utility/Reproducer.h | 3 +++ lldb/source/Commands/CommandObjectReproducer.cpp | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index c8fee0263d0d7..bee53e7ac6bd5 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -332,6 +332,9 @@ class Reproducer { FileSpec GetReproducerPath() const; + bool IsCapturing() { return static_cast(m_generator); }; + bool IsReplaying() { return static_cast(m_loader); }; + protected: llvm::Error SetCapture(llvm::Optional root); llvm::Error SetReplay(llvm::Optional root); diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp index 404702d3640e4..424595fc0bd75 100644 --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -88,7 +88,7 @@ class CommandObjectReproducerGenerate : public CommandObjectParsed { auto &r = Reproducer::Instance(); if (auto generator = r.GetGenerator()) { generator->Keep(); - } else if (r.GetLoader()) { + } else if (r.IsReplaying()) { // Make this operation a NOP in replay mode. result.SetStatus(eReturnStatusSuccessFinishNoResult); return result.Succeeded(); @@ -132,9 +132,9 @@ class CommandObjectReproducerStatus : public CommandObjectParsed { } auto &r = Reproducer::Instance(); - if (r.GetGenerator()) { + if (r.IsCapturing()) { result.GetOutputStream() << "Reproducer is in capture mode.\n"; - } else if (r.GetLoader()) { + } else if (r.IsReplaying()) { result.GetOutputStream() << "Reproducer is in replay mode.\n"; } else { result.GetOutputStream() << "Reproducer is off.\n"; From 499c59ad253dea8abba9838b8d5de81690c4cef3 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:01:57 +0000 Subject: [PATCH 08/12] [Reproducer] Add LoadBuffer<> helper (NFC) Introduce a helper method named LoadBuffer in the Loader to abstract reading a reproducer file from disk. llvm-svn: 375060 (cherry picked from commit b2575da9aa3f759206bf762ad2ceb7fc1946f0f6) --- lldb/include/lldb/Utility/Reproducer.h | 9 +++++++++ lldb/source/Commands/CommandObjectReproducer.cpp | 15 ++++----------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index bee53e7ac6bd5..cc10b3619f634 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -300,6 +300,15 @@ class Loader final { return GetRoot().CopyByAppendingPathComponent(T::file); } + template llvm::Expected LoadBuffer() { + FileSpec file = GetFile(); + llvm::ErrorOr> buffer = + llvm::vfs::getRealFileSystem()->getBufferForFile(file.GetPath()); + if (!buffer) + return llvm::errorCodeToError(buffer.getError()); + return (*buffer)->getBuffer().str(); + } + llvm::Error LoadIndex(); const FileSpec &GetRoot() const { return m_root; } diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp index 424595fc0bd75..0c2e95d5d21f2 100644 --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -265,19 +265,12 @@ class CommandObjectReproducerDump : public CommandObjectParsed { return true; } case eReproducerProviderVersion: { - FileSpec version_file = loader->GetFile(); - - // Load the version info into a buffer. - ErrorOr> buffer = - vfs::getRealFileSystem()->getBufferForFile(version_file.GetPath()); - if (!buffer) { - SetError(result, errorCodeToError(buffer.getError())); + Expected version = loader->LoadBuffer(); + if (!version) { + SetError(result, version.takeError()); return false; } - - // Return the version string. - StringRef version = (*buffer)->getBuffer(); - result.AppendMessage(version.str()); + result.AppendMessage(*version); result.SetStatus(eReturnStatusSuccessFinishResult); return true; } From f53c06f705c126ee8b07f2257562a3e312c26e4a Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:02:00 +0000 Subject: [PATCH 09/12] [Reproducer] Support dumping the reproducer CWD Add support for dumping the current working directory with `reproducer dump -p cwd`. llvm-svn: 375061 (cherry picked from commit f4f120125eade60089b45b4679bb3f34bbd96b86) --- lldb/lit/Reproducer/Inputs/WorkingDir.in | 4 ++++ lldb/lit/Reproducer/TestWorkingDir.test | 4 +++- .../Commands/CommandObjectReproducer.cpp | 21 +++++++++++++++++-- 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 lldb/lit/Reproducer/Inputs/WorkingDir.in diff --git a/lldb/lit/Reproducer/Inputs/WorkingDir.in b/lldb/lit/Reproducer/Inputs/WorkingDir.in new file mode 100644 index 0000000000000..4803ef3785079 --- /dev/null +++ b/lldb/lit/Reproducer/Inputs/WorkingDir.in @@ -0,0 +1,4 @@ +run +reproducer status +reproducer dump -p cwd +reproducer generate diff --git a/lldb/lit/Reproducer/TestWorkingDir.test b/lldb/lit/Reproducer/TestWorkingDir.test index eb0af843dfb39..cef57e8651108 100644 --- a/lldb/lit/Reproducer/TestWorkingDir.test +++ b/lldb/lit/Reproducer/TestWorkingDir.test @@ -7,5 +7,7 @@ # RUN: mkdir -p %t # RUN: cd %t # RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out -# RUN: %lldb -x -b -s %S/Inputs/FileCapture.in --capture --capture-path %t.repro %t/reproducer.out +# RUN: %lldb -x -b -s %S/Inputs/WorkingDir.in --capture --capture-path %t.repro %t/reproducer.out + # RUN: cat %t.repro/cwd.txt | FileCheck %t.check +# RUN: %lldb --replay %t.repro | FileCheck %t.check diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp index 0c2e95d5d21f2..72afed902649a 100644 --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -9,8 +9,8 @@ #include "CommandObjectReproducer.h" #include "lldb/Host/OptionParser.h" -#include "lldb/Utility/Reproducer.h" #include "lldb/Utility/GDBRemote.h" +#include "lldb/Utility/Reproducer.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -27,6 +27,7 @@ enum ReproducerProvider { eReproducerProviderFiles, eReproducerProviderGDB, eReproducerProviderVersion, + eReproducerProviderWorkingDirectory, eReproducerProviderNone }; @@ -51,6 +52,11 @@ static constexpr OptionEnumValueElement g_reproducer_provider_type[] = { "version", "Version", }, + { + eReproducerProviderWorkingDirectory, + "cwd", + "Working Directory", + }, { eReproducerProviderNone, "none", @@ -274,6 +280,17 @@ class CommandObjectReproducerDump : public CommandObjectParsed { result.SetStatus(eReturnStatusSuccessFinishResult); return true; } + case eReproducerProviderWorkingDirectory: { + Expected cwd = + loader->LoadBuffer(); + if (!cwd) { + SetError(result, cwd.takeError()); + return false; + } + result.AppendMessage(*cwd); + result.SetStatus(eReturnStatusSuccessFinishResult); + return true; + } case eReproducerProviderCommands: { // Create a new command loader. std::unique_ptr command_loader = @@ -320,7 +337,7 @@ class CommandObjectReproducerDump : public CommandObjectParsed { return false; } - for (GDBRemotePacket& packet : packets) { + for (GDBRemotePacket &packet : packets) { packet.Dump(result.GetOutputStream()); } From de2b7b8cbfd41acc1495e19f3416287cf8bf5b1f Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 18 Oct 2019 21:47:31 +0000 Subject: [PATCH 10/12] [Reproducer] Improve reproducer help (NFC) Provide a little more detail for the reproducer command. llvm-svn: 375292 (cherry picked from commit 64b7d95568607eac5336428a22e02f27b8663a79) --- lldb/source/Commands/CommandObjectReproducer.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp index 72afed902649a..dc4579c20fc2e 100644 --- a/lldb/source/Commands/CommandObjectReproducer.cpp +++ b/lldb/source/Commands/CommandObjectReproducer.cpp @@ -161,7 +161,9 @@ class CommandObjectReproducerDump : public CommandObjectParsed { public: CommandObjectReproducerDump(CommandInterpreter &interpreter) : CommandObjectParsed(interpreter, "reproducer dump", - "Dump the information contained in a reproducer.", + "Dump the information contained in a reproducer. " + "If no reproducer is specified during replay, it " + "dumps the content of the current reproducer.", nullptr) {} ~CommandObjectReproducerDump() override = default; @@ -361,7 +363,15 @@ CommandObjectReproducer::CommandObjectReproducer( CommandInterpreter &interpreter) : CommandObjectMultiword( interpreter, "reproducer", - "Commands for manipulate the reproducer functionality.", + "Commands for manipulating reproducers. Reproducers make it possible " + "to capture full debug sessions with all its dependencies. The " + "resulting reproducer is used to replay the debug session while " + "debugging the debugger.\n" + "Because reproducers need the whole the debug session from " + "beginning to end, you need to launch the debugger in capture or " + "replay mode, commonly though the command line driver.\n" + "Reproducers are unrelated record-replay debugging, as you cannot " + "interact with the debugger during replay.\n", "reproducer []") { LoadSubCommand( "generate", From 423f3ba0d150bc858e4f0d9e71f328c3f3988720 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 13 Sep 2019 23:14:10 +0000 Subject: [PATCH 11/12] [Reproducer] Move GDB Remote Packet into Utility. (NFC) To support dumping the reproducer's GDB remote packets, we need the (de)serialization logic to live in Utility rather than the GDB remote plugin. This patch renames StreamGDBRemote to GDBRemote and moves the relevant packet code there. Its uses in the GDBRemoteCommunicationHistory and the GDBRemoteCommunicationReplayServer are updated as well. Differential revision: https://reviews.llvm.org/D67523 llvm-svn: 371907 (cherry picked from commit ff5225bfb634369e907c889e16cbee36b260362a) --- lldb/include/lldb/Utility/GDBRemote.h | 113 ++++++++++++++++++ lldb/include/lldb/Utility/Reproducer.h | 1 - lldb/include/lldb/Utility/StreamGDBRemote.h | 45 ------- .../gdb-remote/GDBRemoteCommunication.cpp | 12 +- .../gdb-remote/GDBRemoteCommunicationClient.h | 2 +- .../GDBRemoteCommunicationHistory.cpp | 66 +++------- .../GDBRemoteCommunicationHistory.h | 85 +------------ .../GDBRemoteCommunicationReplayServer.cpp | 12 +- .../GDBRemoteCommunicationReplayServer.h | 2 +- .../GDBRemoteCommunicationServerCommon.cpp | 2 +- .../GDBRemoteCommunicationServerLLGS.cpp | 2 +- .../GDBRemoteCommunicationServerPlatform.cpp | 2 +- .../Process/gdb-remote/ProcessGDBRemote.h | 2 +- lldb/source/Utility/CMakeLists.txt | 2 +- lldb/source/Utility/GDBRemote.cpp | 88 ++++++++++++++ lldb/source/Utility/StreamGDBRemote.cpp | 45 ------- .../gdb-remote/GDBRemoteClientBaseTest.cpp | 2 +- 17 files changed, 237 insertions(+), 246 deletions(-) create mode 100644 lldb/include/lldb/Utility/GDBRemote.h delete mode 100644 lldb/include/lldb/Utility/StreamGDBRemote.h create mode 100644 lldb/source/Utility/GDBRemote.cpp delete mode 100644 lldb/source/Utility/StreamGDBRemote.cpp diff --git a/lldb/include/lldb/Utility/GDBRemote.h b/lldb/include/lldb/Utility/GDBRemote.h new file mode 100644 index 0000000000000..2ec76781e7483 --- /dev/null +++ b/lldb/include/lldb/Utility/GDBRemote.h @@ -0,0 +1,113 @@ +//===-- GDBRemote.h ----------------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef liblldb_GDBRemote_h_ +#define liblldb_GDBRemote_h_ + +#include "lldb/Utility/StreamString.h" +#include "lldb/lldb-enumerations.h" +#include "lldb/lldb-public.h" +#include "llvm/Support/YAMLTraits.h" +#include "llvm/Support/raw_ostream.h" + +#include +#include +#include +#include + +namespace lldb_private { + +class StreamGDBRemote : public StreamString { +public: + StreamGDBRemote(); + + StreamGDBRemote(uint32_t flags, uint32_t addr_size, + lldb::ByteOrder byte_order); + + ~StreamGDBRemote() override; + + /// Output a block of data to the stream performing GDB-remote escaping. + /// + /// \param[in] s + /// A block of data. + /// + /// \param[in] src_len + /// The amount of data to write. + /// + /// \return + /// Number of bytes written. + // TODO: Convert this function to take ArrayRef + int PutEscapedBytes(const void *s, size_t src_len); +}; + +/// 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; + + 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; +}; + +} // namespace lldb_private + +LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(lldb_private::GDBRemotePacket) +LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(std::vector) + +namespace llvm { +namespace yaml { + +template <> +struct ScalarEnumerationTraits { + static void enumeration(IO &io, lldb_private::GDBRemotePacket::Type &value); +}; + +template <> struct ScalarTraits { + static void output(const lldb_private::GDBRemotePacket::BinaryData &, void *, + raw_ostream &); + + static StringRef input(StringRef, void *, + lldb_private::GDBRemotePacket::BinaryData &); + + static QuotingType mustQuote(StringRef S) { return QuotingType::None; } +}; + +template <> struct MappingTraits { + static void mapping(IO &io, lldb_private::GDBRemotePacket &Packet); + + static StringRef validate(IO &io, lldb_private::GDBRemotePacket &); +}; + +} // namespace yaml +} // namespace llvm + +#endif // liblldb_GDBRemote_h_ diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index cc10b3619f634..7cfb724e8ce5e 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -11,7 +11,6 @@ #include "lldb/Utility/FileCollector.h" #include "lldb/Utility/FileSpec.h" - #include "llvm/ADT/DenseMap.h" #include "llvm/Support/Error.h" #include "llvm/Support/YAMLTraits.h" diff --git a/lldb/include/lldb/Utility/StreamGDBRemote.h b/lldb/include/lldb/Utility/StreamGDBRemote.h deleted file mode 100644 index dd0ea31126d93..0000000000000 --- a/lldb/include/lldb/Utility/StreamGDBRemote.h +++ /dev/null @@ -1,45 +0,0 @@ -//===-- StreamGDBRemote.h ----------------------------------------*- C++-*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef liblldb_StreamGDBRemote_h_ -#define liblldb_StreamGDBRemote_h_ - -#include "lldb/Utility/StreamString.h" -#include "lldb/lldb-enumerations.h" - -#include -#include - -namespace lldb_private { - -class StreamGDBRemote : public StreamString { -public: - StreamGDBRemote(); - - StreamGDBRemote(uint32_t flags, uint32_t addr_size, - lldb::ByteOrder byte_order); - - ~StreamGDBRemote() override; - - /// Output a block of data to the stream performing GDB-remote escaping. - /// - /// \param[in] s - /// A block of data. - /// - /// \param[in] src_len - /// The amount of data to write. - /// - /// \return - /// Number of bytes written. - // TODO: Convert this function to take ArrayRef - int PutEscapedBytes(const void *s, size_t src_len); -}; - -} // namespace lldb_private - -#endif // liblldb_StreamGDBRemote_h_ diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 3a26d20c362e3..c52ba7131ea21 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -101,8 +101,7 @@ size_t GDBRemoteCommunication::SendAck() { 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, - bytes_written); + m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; } @@ -112,8 +111,7 @@ size_t GDBRemoteCommunication::SendNack() { 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, - bytes_written); + m_history.AddPacket(ch, GDBRemotePacket::ePacketTypeSend, bytes_written); return bytes_written; } @@ -176,8 +174,7 @@ GDBRemoteCommunication::SendRawPacketNoLock(llvm::StringRef packet, } m_history.AddPacket(packet.str(), packet_length, - GDBRemoteCommunicationHistory::ePacketTypeSend, - bytes_written); + GDBRemotePacket::ePacketTypeSend, bytes_written); if (bytes_written == packet_length) { if (!skip_ack && GetSendAcks()) @@ -808,8 +805,7 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len, } m_history.AddPacket(m_bytes, total_length, - GDBRemoteCommunicationHistory::ePacketTypeRecv, - total_length); + GDBRemotePacket::ePacketTypeRecv, total_length); // Clear packet_str in case there is some existing data in it. packet_str.clear(); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index daee68059520b..898942bb54594 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -18,7 +18,7 @@ #include #include "lldb/Utility/ArchSpec.h" -#include "lldb/Utility/StreamGDBRemote.h" +#include "lldb/Utility/GDBRemote.h" #include "lldb/Utility/StructuredData.h" #if defined(_WIN32) #include "lldb/Host/windows/PosixApi.h" diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp index f9f67fcbcaa92..898a6ec6851c7 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp @@ -18,12 +18,6 @@ using namespace lldb; using namespace lldb_private; using namespace lldb_private::process_gdb_remote; -void GDBRemoteCommunicationHistory::Entry::Serialize(raw_ostream &strm) const { - yaml::Output yout(strm); - yout << const_cast(*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 +27,8 @@ GDBRemoteCommunicationHistory::GDBRemoteCommunicationHistory(uint32_t size) 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 +45,8 @@ void GDBRemoteCommunicationHistory::AddPacket(char packet_char, PacketType type, } 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 +68,14 @@ void GDBRemoteCommunicationHistory::Dump(Stream &strm) const { 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 +90,15 @@ void GDBRemoteCommunicationHistory::Dump(Log *log) const { 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:: - 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:: - output(const GDBRemoteCommunicationHistory::Entry::BinaryData &Val, void *, - raw_ostream &Out) { - Out << toHex(Val.data); -} - -StringRef -yaml::ScalarTraits::input( - StringRef Scalar, void *, - GDBRemoteCommunicationHistory::Entry::BinaryData &Val) { - Val.data = fromHex(Scalar); - return {}; -} - -void yaml::MappingTraits::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::validate( - IO &io, GDBRemoteCommunicationHistory::Entry &Entry) { - if (Entry.bytes_transmitted != Entry.packet.data.size()) - return "BinaryData size doesn't match bytes transmitted"; - - return {}; -} diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h index 85f112b506236..c006fbd34a4b9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.h @@ -12,6 +12,7 @@ #include #include +#include "lldb/Utility/GDBRemote.h" #include "lldb/lldb-public.h" #include "llvm/Support/YAMLTraits.h" #include "llvm/Support/raw_ostream.h" @@ -25,46 +26,17 @@ class GDBRemoteCommunicationHistory { public: friend llvm::yaml::MappingTraits; - 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, GDBRemotePacket::Type type, uint32_t bytes_transmitted); + void AddPacket(const std::string &src, uint32_t src_len, + 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 @@ class GDBRemoteCommunicationHistory { return m_packets.empty() ? 0 : i % m_packets.size(); } - std::vector m_packets; + std::vector m_packets; uint32_t m_curr_idx; uint32_t m_total_packet_count; mutable bool m_dumped_to_log; @@ -107,49 +79,4 @@ class GDBRemoteCommunicationHistory { } // 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 { - static void enumeration(IO &io, - lldb_private::process_gdb_remote:: - GDBRemoteCommunicationHistory::PacketType &value); -}; - -template <> -struct ScalarTraits { - 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_ diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp index 417f5737a30ff..359fab903111b 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp @@ -127,7 +127,7 @@ GDBRemoteCommunicationReplayServer::GetPacketAndSendResponse( 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 @@ -135,7 +135,7 @@ GDBRemoteCommunicationReplayServer::GetPacketAndSendResponse( 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}'", @@ -149,14 +149,14 @@ GDBRemoteCommunicationReplayServer::GetPacketAndSendResponse( // 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}'", @@ -175,10 +175,6 @@ GDBRemoteCommunicationReplayServer::GetPacketAndSendResponse( 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()); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h index 26d65e265463b..0b5e910f7c6a6 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.h @@ -62,7 +62,7 @@ class GDBRemoteCommunicationReplayServer : public GDBRemoteCommunication { static lldb::thread_result_t AsyncThread(void *arg); /// Replay history with the oldest packet at the end. - std::vector m_packet_history; + std::vector m_packet_history; /// Server thread. Broadcaster m_async_broadcaster; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index d0ae5191efd6f..4284bc616cf53 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -29,9 +29,9 @@ #include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/Platform.h" #include "lldb/Utility/Endian.h" +#include "lldb/Utility/GDBRemote.h" #include "lldb/Utility/JSON.h" #include "lldb/Utility/Log.h" -#include "lldb/Utility/StreamGDBRemote.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StructuredData.h" #include "llvm/ADT/Triple.h" diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index 6bdbbd24fdbb7..df2a3da8b2147 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -11,7 +11,7 @@ #include "lldb/Host/Config.h" #include "GDBRemoteCommunicationServerLLGS.h" -#include "lldb/Utility/StreamGDBRemote.h" +#include "lldb/Utility/GDBRemote.h" #include #include diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index 677f348dec209..83370bd2e98f0 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -27,9 +27,9 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Target/Platform.h" #include "lldb/Target/UnixSignals.h" +#include "lldb/Utility/GDBRemote.h" #include "lldb/Utility/JSON.h" #include "lldb/Utility/Log.h" -#include "lldb/Utility/StreamGDBRemote.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StructuredData.h" #include "lldb/Utility/UriParser.h" diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 84004c195660f..0e3e3b39d9c8e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -24,8 +24,8 @@ #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/Broadcaster.h" #include "lldb/Utility/ConstString.h" +#include "lldb/Utility/GDBRemote.h" #include "lldb/Utility/Status.h" -#include "lldb/Utility/StreamGDBRemote.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StringExtractor.h" #include "lldb/Utility/StringList.h" diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt index 03254c334dc76..24b178e251f32 100644 --- a/lldb/source/Utility/CMakeLists.txt +++ b/lldb/source/Utility/CMakeLists.txt @@ -26,6 +26,7 @@ add_lldb_library(lldbUtility FileCollector.cpp Event.cpp FileSpec.cpp + GDBRemote.cpp IOObject.cpp JSON.cpp LLDBAssert.cpp @@ -45,7 +46,6 @@ add_lldb_library(lldbUtility Status.cpp Stream.cpp StreamCallback.cpp - StreamGDBRemote.cpp StreamString.cpp StringExtractor.cpp StringExtractorGDBRemote.cpp diff --git a/lldb/source/Utility/GDBRemote.cpp b/lldb/source/Utility/GDBRemote.cpp new file mode 100644 index 0000000000000..f0982922a8927 --- /dev/null +++ b/lldb/source/Utility/GDBRemote.cpp @@ -0,0 +1,88 @@ +//===-- GDBRemote.cpp -----------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/GDBRemote.h" + +#include "lldb/Utility/Flags.h" +#include "lldb/Utility/Stream.h" + +#include + +using namespace lldb; +using namespace lldb_private; +using namespace llvm; + +StreamGDBRemote::StreamGDBRemote() : StreamString() {} + +StreamGDBRemote::StreamGDBRemote(uint32_t flags, uint32_t addr_size, + ByteOrder byte_order) + : StreamString(flags, addr_size, byte_order) {} + +StreamGDBRemote::~StreamGDBRemote() {} + +int StreamGDBRemote::PutEscapedBytes(const void *s, size_t src_len) { + int bytes_written = 0; + const uint8_t *src = static_cast(s); + bool binary_is_set = m_flags.Test(eBinary); + m_flags.Clear(eBinary); + while (src_len) { + uint8_t byte = *src; + src++; + src_len--; + if (byte == 0x23 || byte == 0x24 || byte == 0x7d || byte == 0x2a) { + bytes_written += PutChar(0x7d); + byte ^= 0x20; + } + bytes_written += PutChar(byte); + }; + if (binary_is_set) + m_flags.Set(eBinary); + return bytes_written; +} + +void GDBRemotePacket::Serialize(raw_ostream &strm) const { + yaml::Output yout(strm); + yout << const_cast(*this); + strm.flush(); +} + +void yaml::ScalarEnumerationTraits::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::output( + const GDBRemotePacket::BinaryData &Val, void *, raw_ostream &Out) { + Out << toHex(Val.data); +} + +StringRef yaml::ScalarTraits::input( + StringRef Scalar, void *, GDBRemotePacket::BinaryData &Val) { + Val.data = fromHex(Scalar); + return {}; +} + +void yaml::MappingTraits::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::validate(IO &io, + GDBRemotePacket &Packet) { + if (Packet.bytes_transmitted != Packet.packet.data.size()) + return "BinaryData size doesn't match bytes transmitted"; + + return {}; +} diff --git a/lldb/source/Utility/StreamGDBRemote.cpp b/lldb/source/Utility/StreamGDBRemote.cpp deleted file mode 100644 index c710bbe3eecb3..0000000000000 --- a/lldb/source/Utility/StreamGDBRemote.cpp +++ /dev/null @@ -1,45 +0,0 @@ -//===-- StreamGDBRemote.cpp -------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "lldb/Utility/StreamGDBRemote.h" - -#include "lldb/Utility/Flags.h" -#include "lldb/Utility/Stream.h" - -#include - -using namespace lldb; -using namespace lldb_private; - -StreamGDBRemote::StreamGDBRemote() : StreamString() {} - -StreamGDBRemote::StreamGDBRemote(uint32_t flags, uint32_t addr_size, - ByteOrder byte_order) - : StreamString(flags, addr_size, byte_order) {} - -StreamGDBRemote::~StreamGDBRemote() {} - -int StreamGDBRemote::PutEscapedBytes(const void *s, size_t src_len) { - int bytes_written = 0; - const uint8_t *src = static_cast(s); - bool binary_is_set = m_flags.Test(eBinary); - m_flags.Clear(eBinary); - while (src_len) { - uint8_t byte = *src; - src++; - src_len--; - if (byte == 0x23 || byte == 0x24 || byte == 0x7d || byte == 0x2a) { - bytes_written += PutChar(0x7d); - byte ^= 0x20; - } - bytes_written += PutChar(byte); - }; - if (binary_is_set) - m_flags.Set(eBinary); - return bytes_written; -} diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp index afba5c64266d5..d56e9cf81bc75 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp @@ -12,7 +12,7 @@ #include "Plugins/Process/Utility/LinuxSignals.h" #include "Plugins/Process/gdb-remote/GDBRemoteClientBase.h" #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h" -#include "lldb/Utility/StreamGDBRemote.h" +#include "lldb/Utility/GDBRemote.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Testing/Support/Error.h" From 0cfa969420d7540f9a4d2d2dba2e538721a62ccb Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Mon, 16 Sep 2019 20:02:57 +0000 Subject: [PATCH 12/12] [NFC] Move dumping into GDBRemotePacket This moves the dumping logic from the GDBRemoteCommunicationHistory class into the GDBRemotePacket so that it can be reused from the reproducer command object. llvm-svn: 372028 (cherry picked from commit 4e053ff1d188bae61f6f7d20c591a462b32a9992) --- lldb/include/lldb/Utility/GDBRemote.h | 4 ++++ .../GDBRemoteCommunicationHistory.cpp | 7 ++----- lldb/source/Utility/GDBRemote.cpp | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Utility/GDBRemote.h b/lldb/include/lldb/Utility/GDBRemote.h index 2ec76781e7483..b4adeb3685248 100644 --- a/lldb/include/lldb/Utility/GDBRemote.h +++ b/lldb/include/lldb/Utility/GDBRemote.h @@ -70,12 +70,16 @@ struct GDBRemotePacket { }; void Serialize(llvm::raw_ostream &strm) const; + void Dump(Stream &strm) const; BinaryData packet; Type type; uint32_t bytes_transmitted; uint32_t packet_idx; lldb::tid_t tid; + +private: + llvm::StringRef GetTypeStr() const; }; } // namespace lldb_private diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp index 898a6ec6851c7..d2cc32f63f20a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationHistory.cpp @@ -72,11 +72,8 @@ void GDBRemoteCommunicationHistory::Dump(Stream &strm) const { 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 == GDBRemotePacket::ePacketTypeSend) ? "send" - : "read", - entry.packet.data.c_str()); + strm.Printf("history[%u] ", entry.packet_idx); + entry.Dump(strm); } } diff --git a/lldb/source/Utility/GDBRemote.cpp b/lldb/source/Utility/GDBRemote.cpp index f0982922a8927..85c4bc69a8d10 100644 --- a/lldb/source/Utility/GDBRemote.cpp +++ b/lldb/source/Utility/GDBRemote.cpp @@ -51,6 +51,23 @@ void GDBRemotePacket::Serialize(raw_ostream &strm) const { strm.flush(); } +llvm::StringRef GDBRemotePacket::GetTypeStr() const { + switch (type) { + case GDBRemotePacket::ePacketTypeSend: + return "send"; + case GDBRemotePacket::ePacketTypeRecv: + return "read"; + case GDBRemotePacket::ePacketTypeInvalid: + return "invalid"; + } + llvm_unreachable("All enum cases should be handled"); +} + +void GDBRemotePacket::Dump(Stream &strm) const { + strm.Printf("tid=0x%4.4" PRIx64 " <%4u> %s packet: %s\n", tid, + bytes_transmitted, GetTypeStr().data(), packet.data.c_str()); +} + void yaml::ScalarEnumerationTraits::enumeration( IO &io, GDBRemotePacket::Type &value) { io.enumCase(value, "Invalid", GDBRemotePacket::ePacketTypeInvalid);