From eb4ceb65bbc23aa4e47c2b38d756f7d5e2e09589 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 8 Oct 2019 19:17:42 +0000 Subject: [PATCH 01/14] [Reproducer] Don't isntrument methods that get called from the signal handler. LLDB's signal handlers call SBDebugger methods, which themselves try to be really careful about not doing anything non-signal safe. The Reproducer record macro is not careful though, and does unsafe things which potentially caused LLDB to crash. Given that these methods are not particularly interesting I've swapped the RECORD macros with DUMMY ones, so that we still register the API boundary but don't do anything non-signal safe. Thanks Jim for figuring this one out! llvm-svn: 374104 (cherry picked from commit b328dcbf850ed6b7a4fab603f58dc6a51e14f984) --- lldb/include/lldb/Utility/ReproducerInstrumentation.h | 2 ++ lldb/source/API/SBDebugger.cpp | 9 ++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Utility/ReproducerInstrumentation.h b/lldb/include/lldb/Utility/ReproducerInstrumentation.h index 9be632ec6a950..06909b0bf1df3 100644 --- a/lldb/include/lldb/Utility/ReproducerInstrumentation.h +++ b/lldb/include/lldb/Utility/ReproducerInstrumentation.h @@ -175,6 +175,8 @@ template inline std::string stringify_args(const Ts &... ts) { #define LLDB_RECORD_DUMMY(Result, Class, Method, Signature, ...) \ lldb_private::repro::Recorder sb_recorder(LLVM_PRETTY_FUNCTION, \ stringify_args(__VA_ARGS__)); +#define LLDB_RECORD_DUMMY_NO_ARGS(Result, Class, Method) \ + lldb_private::repro::Recorder sb_recorder(LLVM_PRETTY_FUNCTION); namespace lldb_private { namespace repro { diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 7e6f8d2c6e979..e297c3807c447 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -407,14 +407,14 @@ FILE *SBDebugger::GetErrorFileHandle() { } void SBDebugger::SaveInputTerminalState() { - LLDB_RECORD_METHOD_NO_ARGS(void, SBDebugger, SaveInputTerminalState); + LLDB_RECORD_DUMMY_NO_ARGS(void, SBDebugger, SaveInputTerminalState); if (m_opaque_sp) m_opaque_sp->SaveInputTerminalState(); } void SBDebugger::RestoreInputTerminalState() { - LLDB_RECORD_METHOD_NO_ARGS(void, SBDebugger, RestoreInputTerminalState); + LLDB_RECORD_DUMMY_NO_ARGS(void, SBDebugger, RestoreInputTerminalState); if (m_opaque_sp) m_opaque_sp->RestoreInputTerminalState(); @@ -1075,7 +1075,7 @@ void SBDebugger::DispatchInput(const void *data, size_t data_len) { } void SBDebugger::DispatchInputInterrupt() { - LLDB_RECORD_METHOD_NO_ARGS(void, SBDebugger, DispatchInputInterrupt); + LLDB_RECORD_DUMMY_NO_ARGS(void, SBDebugger, DispatchInputInterrupt); if (m_opaque_sp) m_opaque_sp->DispatchInputInterrupt(); @@ -1235,8 +1235,7 @@ uint32_t SBDebugger::GetTerminalWidth() const { } void SBDebugger::SetTerminalWidth(uint32_t term_width) { - LLDB_RECORD_METHOD(void, SBDebugger, SetTerminalWidth, (uint32_t), - term_width); + LLDB_RECORD_DUMMY(void, SBDebugger, SetTerminalWidth, (uint32_t), term_width); if (m_opaque_sp) m_opaque_sp->SetTerminalWidth(term_width); From 63a48149538b0f916dad3eb8ab179a1bbb26ccfb Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 13 Sep 2019 23:27:31 +0000 Subject: [PATCH 02/14] [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 dba784b7da11a..9e4e186815fe6 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -452,6 +452,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 0bb574e1b1211..45325e2ad3f40 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -721,9 +721,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 48ce31491a80c..4431d8a534138 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -1079,20 +1079,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); @@ -1100,9 +1099,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 6ed1e70fb8002efdbde0bcb9df08d380fb3af516 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Mon, 16 Sep 2019 23:31:06 +0000 Subject: [PATCH 03/14] [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 91d3273abd5ac126316150cade384bdf475460ac Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 9 Oct 2019 21:47:49 +0000 Subject: [PATCH 04/14] [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 8d88654cbe018..3c16f10198c48 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -287,6 +287,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 449a4e3bce5e03b534f56d046645021fe55bcca1 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 11 Sep 2019 23:15:12 +0000 Subject: [PATCH 05/14] [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 | 16 ++++++- 3 files changed, 45 insertions(+), 44 deletions(-) diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h index 3c16f10198c48..b9df50295271c 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -184,10 +184,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(FileSpec root); ~Generator(); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index e33da77207636..9e66070b4cdf0 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 = std::make_unique( - history_file.GetPath(), EC, sys::fs::OpenFlags::OF_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. @@ -3423,7 +3384,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 b73b66031785c..03887120ed48d 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -184,7 +184,7 @@ void Generator::AddProvidersToIndex() { std::error_code EC; auto strm = std::make_unique(index.GetPath(), EC, - sys::fs::OpenFlags::OF_None); + sys::fs::OpenFlags::OF_None); yaml::Output yout(*strm); std::vector files; @@ -281,14 +281,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 c515885c50fe70144a7bd621a28f2b0ebbf2a892 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 11 Sep 2019 23:27:12 +0000 Subject: [PATCH 06/14] [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 b9df50295271c..f3d47c858ebaa 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -330,6 +330,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 e297c3807c447..a37bf76c62280 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 std::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 03887120ed48d..a8a581f0761a0 100644 --- a/lldb/source/Utility/Reproducer.cpp +++ b/lldb/source/Utility/Reproducer.cpp @@ -290,6 +290,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 ebc1975390f1e84083ba5ddb339ef2c7ef4df877 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:01:53 +0000 Subject: [PATCH 07/14] [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 f3d47c858ebaa..e83bdc967692d 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -133,6 +133,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 a8a581f0761a0..4777d7576a321 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 fc9b1cb3522f08e7937ff59f982d6b1b4e239faa Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:01:57 +0000 Subject: [PATCH 08/14] [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 e83bdc967692d..86291daa55aad 100644 --- a/lldb/include/lldb/Utility/Reproducer.h +++ b/lldb/include/lldb/Utility/Reproducer.h @@ -303,6 +303,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 dbb1d6ad72fc3798363c1603bed8abbbb3fa9f7a Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:02:00 +0000 Subject: [PATCH 09/14] [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 8854daca1294f3242eaf1d1e4b4f79ababe98953 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 00:24:37 +0000 Subject: [PATCH 10/14] [Reproducer] Set the working directory in the VFS Now that the VFS knows how to deal with virtual working directories, we can set the current working directory to the one we recorded during reproducer capture. This ensures that relative paths are resolved correctly during replay. llvm-svn: 375064 (cherry picked from commit f80f15e38a21fb6a85d24853bdf2b8a6f1068571) --- lldb/lit/Reproducer/TestWorkingDir.test | 10 +++++++--- lldb/source/Initialization/SystemInitializerCommon.cpp | 7 +++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lldb/lit/Reproducer/TestWorkingDir.test b/lldb/lit/Reproducer/TestWorkingDir.test index cef57e8651108..c2f0124477770 100644 --- a/lldb/lit/Reproducer/TestWorkingDir.test +++ b/lldb/lit/Reproducer/TestWorkingDir.test @@ -1,13 +1,17 @@ -# This tests relative capture paths. +# This tests that the reproducer can deal with relative files. We create a +# binary in a subdirectory and pass its relative path to LLDB. The subdirectory +# is removed before replay so that it only exists in the reproducer's VFS. # RUN: echo "CHECK: %t" > %t.check # RUN: rm -rf %t.repro # RUN: mkdir -p %t.repro # RUN: mkdir -p %t +# RUN: mkdir -p %t/binary # RUN: cd %t -# RUN: %clang %S/Inputs/simple.c -g -o %t/reproducer.out -# RUN: %lldb -x -b -s %S/Inputs/WorkingDir.in --capture --capture-path %t.repro %t/reproducer.out +# RUN: %clang %S/Inputs/simple.c -g -o binary/reproducer.out +# RUN: %lldb -x -b -s %S/Inputs/WorkingDir.in --capture --capture-path %t.repro binary/reproducer.out +# RUN: rm -rf %t/binary # RUN: cat %t.repro/cwd.txt | FileCheck %t.check # RUN: %lldb --replay %t.repro | FileCheck %t.check diff --git a/lldb/source/Initialization/SystemInitializerCommon.cpp b/lldb/source/Initialization/SystemInitializerCommon.cpp index 190216dc3973c..11299c794868d 100644 --- a/lldb/source/Initialization/SystemInitializerCommon.cpp +++ b/lldb/source/Initialization/SystemInitializerCommon.cpp @@ -78,6 +78,13 @@ llvm::Error SystemInitializerCommon::Initialize() { } else { FileSystem::Initialize(); } + if (llvm::Expected cwd = + loader->LoadBuffer()) { + FileSystem::Instance().GetVirtualFileSystem()->setCurrentWorkingDirectory( + *cwd); + } else { + return cwd.takeError(); + } } else if (repro::Generator *g = r.GetGenerator()) { repro::VersionProvider &vp = g->GetOrCreate(); vp.SetVersion(lldb_private::GetVersion()); From 4922c86b902c1688fd846a5e114ab73cd551029c Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Thu, 17 Oct 2019 17:58:44 +0000 Subject: [PATCH 11/14] [Reproducer] Surface error if setting the cwd fails Make sure that we surface an error if setting the current working directory fails during replay. llvm-svn: 375146 (cherry picked from commit 2b7899b730b76a1d6162ef2cfbfe8cac179d08d2) --- lldb/source/Initialization/SystemInitializerCommon.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lldb/source/Initialization/SystemInitializerCommon.cpp b/lldb/source/Initialization/SystemInitializerCommon.cpp index 11299c794868d..e905cc9dd2d46 100644 --- a/lldb/source/Initialization/SystemInitializerCommon.cpp +++ b/lldb/source/Initialization/SystemInitializerCommon.cpp @@ -80,8 +80,13 @@ llvm::Error SystemInitializerCommon::Initialize() { } if (llvm::Expected cwd = loader->LoadBuffer()) { - FileSystem::Instance().GetVirtualFileSystem()->setCurrentWorkingDirectory( - *cwd); + cwd->erase(std::remove_if(cwd->begin(), cwd->end(), std::iscntrl), + cwd->end()); + if (std::error_code ec = FileSystem::Instance() + .GetVirtualFileSystem() + ->setCurrentWorkingDirectory(*cwd)) { + return llvm::errorCodeToError(ec); + } } else { return cwd.takeError(); } From 4503eea91f5208e098d1e2ae52058754eb6dd641 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 18 Oct 2019 17:11:48 +0000 Subject: [PATCH 12/14] [Reproducer] Use ::rtrim() to remove trailing control characters. Pavel correctly pointed out that removing all control characters from the working directory is overkill. It should be sufficient to just strip the last ones. llvm-svn: 375259 (cherry picked from commit ded44e220f6ce15258663a0353cccc188211f1d7) --- lldb/source/Initialization/SystemInitializerCommon.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/source/Initialization/SystemInitializerCommon.cpp b/lldb/source/Initialization/SystemInitializerCommon.cpp index e905cc9dd2d46..cd6485cd9bf2c 100644 --- a/lldb/source/Initialization/SystemInitializerCommon.cpp +++ b/lldb/source/Initialization/SystemInitializerCommon.cpp @@ -80,11 +80,10 @@ llvm::Error SystemInitializerCommon::Initialize() { } if (llvm::Expected cwd = loader->LoadBuffer()) { - cwd->erase(std::remove_if(cwd->begin(), cwd->end(), std::iscntrl), - cwd->end()); + llvm::StringRef working_dir = llvm::StringRef(*cwd).rtrim(); if (std::error_code ec = FileSystem::Instance() .GetVirtualFileSystem() - ->setCurrentWorkingDirectory(*cwd)) { + ->setCurrentWorkingDirectory(working_dir)) { return llvm::errorCodeToError(ec); } } else { From 7fecc1f53c7643626c4f74ab10ca3db9b4b0b218 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 18 Oct 2019 21:47:31 +0000 Subject: [PATCH 13/14] [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 7698e715060d818308331fd9993dade9079883ae Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 18 Oct 2019 22:16:15 +0000 Subject: [PATCH 14/14] [Reproducer] XFAIL TestWorkingDir on Windows I'm having a hard time reproducing this and it's failing on the Windows bot. Temporarily X-failing this test while I continue to try building LLDB on Windows. llvm-svn: 375294 (cherry picked from commit 06a2beae92f5d2adcd0143a6798918418c2ea325) --- lldb/lit/Reproducer/TestWorkingDir.test | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/lit/Reproducer/TestWorkingDir.test b/lldb/lit/Reproducer/TestWorkingDir.test index c2f0124477770..fd41e1d43ad68 100644 --- a/lldb/lit/Reproducer/TestWorkingDir.test +++ b/lldb/lit/Reproducer/TestWorkingDir.test @@ -1,3 +1,5 @@ +# XFAIL: system-windows + # This tests that the reproducer can deal with relative files. We create a # binary in a subdirectory and pass its relative path to LLDB. The subdirectory # is removed before replay so that it only exists in the reproducer's VFS.