JDevlieghere updated this revision to Diff 188617. JDevlieghere added a comment.
Pavel made a good point that with the previous implementation, the first call to RunCommandInterpreter would replay every recorded commands. This is indeed incorrect, because it's possible and likely that the state of the debugger has changed between different runs of the commands interpreter. Now every call to RunCommandInterpreter gets its own buffer. During replay, we will change the input file handler before invocation of "RunCommandInterpreter". This works because this function is only called through the SB layer. I'm not convinced doing the recorded at a lower level has any benefits. I investigated this route before and the IOHandler seems basically the same things as the command interpreter. We would still need to make the distinction between things that should and shouldn't be recorded (e.g. sourcing a file vs every command in the file). This would be a lot harder to do there, because we have less information. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58564/new/ https://reviews.llvm.org/D58564 Files: lldb/include/lldb/Interpreter/CommandInterpreter.h lldb/lit/Reproducer/Inputs/CommandRepro.in lldb/lit/Reproducer/TestCommandRepro.test lldb/source/Interpreter/CommandInterpreter.cpp
Index: lldb/source/Interpreter/CommandInterpreter.cpp =================================================================== --- lldb/source/Interpreter/CommandInterpreter.cpp +++ lldb/source/Interpreter/CommandInterpreter.cpp @@ -45,6 +45,7 @@ #include "lldb/Core/PluginManager.h" #include "lldb/Core/StreamFile.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/Reproducer.h" #include "lldb/Utility/State.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/Timer.h" @@ -74,6 +75,7 @@ using namespace lldb; using namespace lldb_private; +using namespace llvm; static const char *k_white_space = " \t\v"; @@ -116,6 +118,70 @@ eEchoCommentCommands = 5 }; +/// Provider for the command interpreter. Every command is logged to file which +/// is used as input during replay. The latter takes place at the SB API layer +/// by changing the input file handle. +class lldb_private::CommandProvider + : public repro::Provider<lldb_private::CommandProvider> { +public: + typedef CommandProviderInfo info; + + CommandProvider(const FileSpec &directory) : Provider(directory) {} + + /// Capture a single command. + void CaptureCommand(std::string command) { + m_commands.back().push_back(std::move(command)); + } + + void StartNewBuffer() { m_commands.emplace_back(); } + + /// Commands are kept in memory and written to a file when the reproducer + /// needs to be kept. + void Keep() override { + // Construct filename for every buffer. + std::vector<std::string> buffers; + for (std::size_t i = 0; i < m_commands.size(); ++i) { + llvm::Twine filename = llvm::Twine(info::name) + llvm::Twine("-") + + llvm::Twine(i) + llvm::Twine(".txt"); + buffers.push_back(filename.str()); + } + + // Populate the top level file with all the filenames. + FileSpec file = + GetRoot().CopyByAppendingPathComponent(CommandProviderInfo::file); + std::error_code ec; + llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::F_Text); + if (ec) + return; + for (auto &buffer : buffers) + os << buffer << '\n'; + + // Populate every buffer with its commands. + std::size_t i = 0; + for (auto &buffer : buffers) { + FileSpec buffer_file = GetRoot().CopyByAppendingPathComponent(buffer); + llvm::raw_fd_ostream os(buffer_file.GetPath(), ec, llvm::sys::fs::F_Text); + if (ec) + continue; + for (auto &command : m_commands[i++]) + os << command << '\n'; + } + } + + /// Commands are kept in memory and are cleared when the reproducer is + /// discarded. + void Discard() override { m_commands.clear(); } + + static char ID; + +private: + std::vector<std::vector<std::string>> m_commands; +}; + +char CommandProvider::ID = 0; +const char *CommandProviderInfo::name = "command-interpreter"; +const char *CommandProviderInfo::file = "command-interpreter.txt"; + ConstString &CommandInterpreter::GetStaticBroadcasterClass() { static ConstString class_name("lldb.commandInterpreter"); return class_name; @@ -141,6 +207,9 @@ SetEventName(eBroadcastBitQuitCommandReceived, "quit"); CheckInWithManager(); m_collection_sp->Initialize(g_properties); + + if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) + m_provider = &g->GetOrCreate<CommandProvider>(); } bool CommandInterpreter::GetExpandRegexAliases() const { @@ -1604,7 +1673,8 @@ CommandReturnObject &result, ExecutionContext *override_context, bool repeat_on_empty_command, - bool no_context_switching) + bool no_context_switching, + bool add_to_reproducer) { @@ -1694,6 +1764,9 @@ Status error(PreprocessCommand(command_string)); + if (m_provider && add_to_reproducer) + m_provider->CaptureCommand(original_command_string); + if (error.Fail()) { result.AppendError(error.AsCString()); result.SetStatus(eReturnStatusFailed); @@ -2175,6 +2248,7 @@ options.SetSilent(true); options.SetStopOnError(false); options.SetStopOnContinue(true); + options.SetAddToReproducer(false); HandleCommandsFromFile(init_file, nullptr, // Execution context @@ -2252,7 +2326,8 @@ HandleCommand(cmd, options.m_add_to_history, tmp_result, nullptr, /* override_context */ true, /* repeat_on_empty_command */ - override_context != nullptr /* no_context_switching */); + override_context != nullptr /* no_context_switching */, + options.GetAddToReproducer()); if (!options.GetAddToHistory()) m_command_source_depth--; @@ -2364,7 +2439,8 @@ eHandleCommandFlagEchoCommand = (1u << 2), eHandleCommandFlagEchoCommentCommand = (1u << 3), eHandleCommandFlagPrintResult = (1u << 4), - eHandleCommandFlagStopOnCrash = (1u << 5) + eHandleCommandFlagStopOnCrash = (1u << 5), + eHandleCommandFlagAddToReproducer = (1u << 6) }; void CommandInterpreter::HandleCommandsFromFile( @@ -2463,6 +2539,10 @@ flags |= eHandleCommandFlagPrintResult; } + if (options.GetAddToReproducer()) { + flags |= eHandleCommandFlagAddToReproducer; + } + if (flags & eHandleCommandFlagPrintResult) { debugger.GetOutputFile()->Printf("Executing commands in '%s'.\n", cmd_file_path.c_str()); @@ -2799,7 +2879,8 @@ StartHandlingCommand(); lldb_private::CommandReturnObject result; - HandleCommand(line.c_str(), eLazyBoolCalculate, result); + HandleCommand(line.c_str(), eLazyBoolCalculate, result, nullptr, true, false, + io_handler.GetFlags().Test(eHandleCommandFlagAddToReproducer)); // Now emit the command output text from the command we just executed if (io_handler.GetFlags().Test(eHandleCommandFlagPrintResult)) { @@ -2969,6 +3050,8 @@ flags |= eHandleCommandFlagEchoCommentCommand; if (options->m_print_results != eLazyBoolNo) flags |= eHandleCommandFlagPrintResult; + if (options->m_add_to_reproducer != eLazyBoolNo) + flags |= eHandleCommandFlagAddToReproducer; } else { flags = eHandleCommandFlagEchoCommand | eHandleCommandFlagPrintResult; } @@ -2989,6 +3072,10 @@ void CommandInterpreter::RunCommandInterpreter( bool auto_handle_events, bool spawn_thread, CommandInterpreterRunOptions &options) { + // We keep different buffers for different runs. + if (m_provider) + m_provider->StartNewBuffer(); + // Always re-create the command interpreter when we run it in case any file // handles have changed. bool force_create = true; Index: lldb/lit/Reproducer/TestCommandRepro.test =================================================================== --- /dev/null +++ lldb/lit/Reproducer/TestCommandRepro.test @@ -0,0 +1,11 @@ +# This tests the recording of commands + +# RUN: %lldb -x -b -o 'expr 1' -s %S/Inputs/CommandRepro.in -o 'expr 4' -o 'reproducer generate' --capture %t.repro +# RUN: cat %t.repro/command-interpreter.txt | FileCheck %s + +# CHECK: expr 1 +# CHECK: command source +# CHECK-NOT: expr 2 +# CHECK-NOT: expr 3 +# CHECK: expr 4 +# CHECK: reproducer generate Index: lldb/lit/Reproducer/Inputs/CommandRepro.in =================================================================== --- /dev/null +++ lldb/lit/Reproducer/Inputs/CommandRepro.in @@ -0,0 +1,2 @@ +expr 2 +expr 3 Index: lldb/include/lldb/Interpreter/CommandInterpreter.h =================================================================== --- lldb/include/lldb/Interpreter/CommandInterpreter.h +++ lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -27,6 +27,14 @@ namespace lldb_private { +/// Reproducer provider for the command interpreter. The info struct needs to +/// be public because replay takes place at the SB API layer. +class CommandProvider; +struct CommandProviderInfo { + static const char *name; + static const char *file; +}; + class CommandInterpreterRunOptions { public: //------------------------------------------------------------------ @@ -135,6 +143,12 @@ m_add_to_history = add_to_history ? eLazyBoolYes : eLazyBoolNo; } + bool GetAddToReproducer() const { return DefaultToYes(m_add_to_reproducer); } + + void SetAddToReproducer(bool add_to_reproducer) { + m_add_to_reproducer = add_to_reproducer ? eLazyBoolYes : eLazyBoolNo; + } + LazyBool m_stop_on_continue; LazyBool m_stop_on_error; LazyBool m_stop_on_crash; @@ -142,6 +156,7 @@ LazyBool m_echo_comment_commands; LazyBool m_print_results; LazyBool m_add_to_history; + LazyBool m_add_to_reproducer; private: static bool DefaultToYes(LazyBool flag) { @@ -250,7 +265,8 @@ CommandReturnObject &result, ExecutionContext *override_context = nullptr, bool repeat_on_empty_command = true, - bool no_context_switching = false); + bool no_context_switching = false, + bool add_to_reproducer = false); bool WasInterrupted() const; @@ -606,6 +622,9 @@ bool m_quit_requested; bool m_stopped_for_crash; + /// Reproducer provider. + CommandProvider *m_provider = nullptr; + // The exit code the user has requested when calling the 'quit' command. // No value means the user hasn't set a custom exit code so far. llvm::Optional<int> m_quit_exit_code;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits