From f9517353959b4ef5c706e4356b56aeb9e0087584 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 9 Jul 2021 09:23:54 -0700 Subject: [PATCH] [lldb] Add the ability to silently import scripted commands Add the ability to silence command script import. The motivation for this change is being able to add command script import -s lldb.macosx.crashlog to your ~/.lldbinit without it printing the following message at the beginning of every debug session. "malloc_info", "ptr_refs", "cstr_refs", "find_variable", and "objc_refs" commands have been installed, use the "--help" options on these commands for detailed help. In addition to forwarding the silent option to LoadScriptingModule, this also changes ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn and ScriptInterpreterPythonImpl::ExecuteMultipleLines to honor the enable IO option in ExecuteScriptOptions, which until now was ignored. Note that IO is only enabled (or disabled) at the start of a session, and for this particular use case, that's done when taking the Python lock in LoadScriptingModule, which means that the changes to these two functions are not strictly necessary, but (IMO) desirable nonetheless. Differential revision: https://reviews.llvm.org/D105327 --- lldb/include/lldb/Interpreter/ScriptInterpreter.h | 24 +++++- lldb/source/Commands/CommandObjectCommands.cpp | 12 ++- lldb/source/Commands/Options.td | 2 + lldb/source/Core/Module.cpp | 4 +- lldb/source/Interpreter/ScriptInterpreter.cpp | 2 +- .../Python/OperatingSystemPython.cpp | 6 +- .../ScriptInterpreter/Lua/ScriptInterpreterLua.cpp | 5 +- .../ScriptInterpreter/Lua/ScriptInterpreterLua.h | 3 +- .../Python/ScriptInterpreterPython.cpp | 94 +++++++++++++++------- .../Python/ScriptInterpreterPythonImpl.h | 3 +- .../Python/silent_command_script_import.test | 8 ++ 11 files changed, 121 insertions(+), 42 deletions(-) create mode 100644 lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h index 8d1c38a..e98e1be 100644 --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -71,6 +71,28 @@ private: bool m_maskout_errors = true; }; +class LoadScriptOptions { +public: + LoadScriptOptions() = default; + + bool GetInitSession() const { return m_init_session; } + bool GetSilent() const { return m_silent; } + + LoadScriptOptions &SetInitSession(bool b) { + m_init_session = b; + return *this; + } + + LoadScriptOptions &SetSilent(bool b) { + m_silent = b; + return *this; + } + +private: + bool m_init_session = false; + bool m_silent = false; +}; + class ScriptInterpreterIORedirect { public: /// Create an IO redirect. If IO is enabled, this will redirects the output @@ -509,7 +531,7 @@ public: virtual bool CheckObjectExists(const char *name) { return false; } virtual bool - LoadScriptingModule(const char *filename, bool init_session, + LoadScriptingModule(const char *filename, const LoadScriptOptions &options, lldb_private::Status &error, StructuredData::ObjectSP *module_sp = nullptr, FileSpec extra_search_dir = {}); diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp index c6b0f7f..9a8b81c 100644 --- a/lldb/source/Commands/CommandObjectCommands.cpp +++ b/lldb/source/Commands/CommandObjectCommands.cpp @@ -1242,6 +1242,9 @@ protected: case 'c': relative_to_command_file = true; break; + case 's': + silent = true; + break; default: llvm_unreachable("Unimplemented option"); } @@ -1257,6 +1260,7 @@ protected: return llvm::makeArrayRef(g_script_import_options); } bool relative_to_command_file = false; + bool silent = false; }; bool DoExecute(Args &command, CommandReturnObject &result) override { @@ -1278,7 +1282,10 @@ protected: for (auto &entry : command.entries()) { Status error; - const bool init_session = true; + LoadScriptOptions options; + options.SetInitSession(true); + options.SetSilent(m_options.silent); + // FIXME: this is necessary because CommandObject::CheckRequirements() // assumes that commands won't ever be recursively invoked, but it's // actually possible to craft a Python script that does other "command @@ -1289,7 +1296,8 @@ protected: // more) m_exe_ctx.Clear(); if (GetDebugger().GetScriptInterpreter()->LoadScriptingModule( - entry.c_str(), init_session, error, nullptr, source_dir)) { + entry.c_str(), options, error, /*module_sp=*/nullptr, + source_dir)) { result.SetStatus(eReturnStatusSuccessFinishNoResult); } else { result.AppendErrorWithFormat("module importing failed: %s", diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 6b5a33f..57208c5 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -745,6 +745,8 @@ let Command = "script import" in { Group<1>, Desc<"Resolve non-absolute paths relative to the location of the " "current command file. This argument can only be used when the command is " "being sourced from a file.">; + def silent : Option<"silent", "s">, Group<1>, + Desc<"If true don't print any script output while importing.">; } let Command = "script add" in { diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index fb80535..19c97be 100644 --- a/lldb/source/Core/Module.cpp +++ b/lldb/source/Core/Module.cpp @@ -1528,9 +1528,9 @@ bool Module::LoadScriptingResourceInTarget(Target *target, Status &error, } StreamString scripting_stream; scripting_fspec.Dump(scripting_stream.AsRawOstream()); - const bool init_lldb_globals = false; + LoadScriptOptions options; bool did_load = script_interpreter->LoadScriptingModule( - scripting_stream.GetData(), init_lldb_globals, error); + scripting_stream.GetData(), options, error); if (!did_load) return false; } diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp index 7351143..f264748 100644 --- a/lldb/source/Interpreter/ScriptInterpreter.cpp +++ b/lldb/source/Interpreter/ScriptInterpreter.cpp @@ -47,7 +47,7 @@ void ScriptInterpreter::CollectDataForWatchpointCommandCallback( } bool ScriptInterpreter::LoadScriptingModule(const char *filename, - bool init_session, + const LoadScriptOptions &options, lldb_private::Status &error, StructuredData::ObjectSP *module_sp, FileSpec extra_search_dir) { diff --git a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp index a117ce0..730c88f 100644 --- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp +++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp @@ -91,13 +91,13 @@ OperatingSystemPython::OperatingSystemPython(lldb_private::Process *process, std::string os_plugin_class_name( python_module_path.GetFilename().AsCString("")); if (!os_plugin_class_name.empty()) { - const bool init_session = false; + LoadScriptOptions options; char python_module_path_cstr[PATH_MAX]; python_module_path.GetPath(python_module_path_cstr, sizeof(python_module_path_cstr)); Status error; - if (m_interpreter->LoadScriptingModule(python_module_path_cstr, - init_session, error)) { + if (m_interpreter->LoadScriptingModule(python_module_path_cstr, options, + error)) { // Strip the ".py" extension if there is one size_t py_extension_pos = os_plugin_class_name.rfind(".py"); if (py_extension_pos != std::string::npos) diff --git a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp index 2105f4a..ef46401 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp @@ -207,8 +207,9 @@ void ScriptInterpreterLua::ExecuteInterpreterLoop() { } bool ScriptInterpreterLua::LoadScriptingModule( - const char *filename, bool init_session, lldb_private::Status &error, - StructuredData::ObjectSP *module_sp, FileSpec extra_search_dir) { + const char *filename, const LoadScriptOptions &options, + lldb_private::Status &error, StructuredData::ObjectSP *module_sp, + FileSpec extra_search_dir) { FileSystem::Instance().Collect(filename); if (llvm::Error e = m_lua->LoadModule(filename)) { diff --git a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h index 5eeac56..808000b 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h +++ b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h @@ -43,7 +43,8 @@ public: void ExecuteInterpreterLoop() override; - bool LoadScriptingModule(const char *filename, bool init_session, + bool LoadScriptingModule(const char *filename, + const LoadScriptOptions &options, lldb_private::Status &error, StructuredData::ObjectSP *module_sp = nullptr, FileSpec extra_search_dir = {}) override; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 86549b0..7ad6372 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1077,11 +1077,24 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn( llvm::StringRef in_string, ScriptInterpreter::ScriptReturnType return_type, void *ret_value, const ExecuteScriptOptions &options) { + llvm::Expected> + io_redirect_or_error = ScriptInterpreterIORedirect::Create( + options.GetEnableIO(), m_debugger, /*result=*/nullptr); + + if (!io_redirect_or_error) { + llvm::consumeError(io_redirect_or_error.takeError()); + return false; + } + + ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error; + Locker locker(this, Locker::AcquireLock | Locker::InitSession | (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) | Locker::NoSTDIN, - Locker::FreeAcquiredLock | Locker::TearDownSession); + Locker::FreeAcquiredLock | Locker::TearDownSession, + io_redirect.GetInputFile(), io_redirect.GetOutputFile(), + io_redirect.GetErrorFile()); PythonModule &main_module = GetMainModule(); PythonDictionary globals = main_module.GetDictionary(); @@ -1190,11 +1203,22 @@ Status ScriptInterpreterPythonImpl::ExecuteMultipleLines( if (in_string == nullptr) return Status(); + llvm::Expected> + io_redirect_or_error = ScriptInterpreterIORedirect::Create( + options.GetEnableIO(), m_debugger, /*result=*/nullptr); + + if (!io_redirect_or_error) + return Status(io_redirect_or_error.takeError()); + + ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error; + Locker locker(this, Locker::AcquireLock | Locker::InitSession | (options.GetSetLLDBGlobals() ? Locker::InitGlobals : 0) | Locker::NoSTDIN, - Locker::FreeAcquiredLock | Locker::TearDownSession); + Locker::FreeAcquiredLock | Locker::TearDownSession, + io_redirect.GetInputFile(), io_redirect.GetOutputFile(), + io_redirect.GetErrorFile()); PythonModule &main_module = GetMainModule(); PythonDictionary globals = main_module.GetDictionary(); @@ -2057,7 +2081,10 @@ ScriptInterpreterPythonImpl::LoadPluginModule(const FileSpec &file_spec, StructuredData::ObjectSP module_sp; - if (LoadScriptingModule(file_spec.GetPath().c_str(), true, error, &module_sp)) + LoadScriptOptions load_script_options = + LoadScriptOptions().SetInitSession(true).SetSilent(false); + if (LoadScriptingModule(file_spec.GetPath().c_str(), load_script_options, + error, &module_sp)) return module_sp; return StructuredData::ObjectSP(); @@ -2732,26 +2759,44 @@ uint64_t replace_all(std::string &str, const std::string &oldStr, } bool ScriptInterpreterPythonImpl::LoadScriptingModule( - const char *pathname, bool init_session, lldb_private::Status &error, - StructuredData::ObjectSP *module_sp, FileSpec extra_search_dir) { + const char *pathname, const LoadScriptOptions &options, + lldb_private::Status &error, StructuredData::ObjectSP *module_sp, + FileSpec extra_search_dir) { namespace fs = llvm::sys::fs; namespace path = llvm::sys::path; + ExecuteScriptOptions exc_options = ExecuteScriptOptions() + .SetEnableIO(!options.GetSilent()) + .SetSetLLDBGlobals(false); + if (!pathname || !pathname[0]) { error.SetErrorString("invalid pathname"); return false; } + llvm::Expected> + io_redirect_or_error = ScriptInterpreterIORedirect::Create( + exc_options.GetEnableIO(), m_debugger, /*result=*/nullptr); + + if (!io_redirect_or_error) { + error = io_redirect_or_error.takeError(); + return false; + } + + ScriptInterpreterIORedirect &io_redirect = **io_redirect_or_error; lldb::DebuggerSP debugger_sp = m_debugger.shared_from_this(); // Before executing Python code, lock the GIL. Locker py_lock(this, Locker::AcquireLock | - (init_session ? Locker::InitSession : 0) | Locker::NoSTDIN, + (options.GetInitSession() ? Locker::InitSession : 0) | + Locker::NoSTDIN, Locker::FreeAcquiredLock | - (init_session ? Locker::TearDownSession : 0)); + (options.GetInitSession() ? Locker::TearDownSession : 0), + io_redirect.GetInputFile(), io_redirect.GetOutputFile(), + io_redirect.GetErrorFile()); - auto ExtendSysPath = [this](std::string directory) -> llvm::Error { + auto ExtendSysPath = [&](std::string directory) -> llvm::Error { if (directory.empty()) { return llvm::make_error( "invalid directory name", llvm::inconvertibleErrorCode()); @@ -2766,11 +2811,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( "sys.path.insert(1,'%s');\n\n", directory.c_str(), directory.c_str()); bool syspath_retval = - ExecuteMultipleLines(command_stream.GetData(), - ExecuteScriptOptions() - .SetEnableIO(false) - .SetSetLLDBGlobals(false)) - .Success(); + ExecuteMultipleLines(command_stream.GetData(), exc_options).Success(); if (!syspath_retval) { return llvm::make_error( "Python sys.path handling failed", llvm::inconvertibleErrorCode()); @@ -2844,21 +2885,18 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( return false; } - // check if the module is already import-ed + // Check if the module is already imported. StreamString command_stream; command_stream.Clear(); command_stream.Printf("sys.modules.__contains__('%s')", module_name.c_str()); bool does_contain = false; - // this call will succeed if the module was ever imported in any Debugger - // in the lifetime of the process in which this LLDB framework is living - const bool was_imported_globally = - (ExecuteOneLineWithReturn( - command_stream.GetData(), - ScriptInterpreterPythonImpl::eScriptReturnTypeBool, &does_contain, - ExecuteScriptOptions() - .SetEnableIO(false) - .SetSetLLDBGlobals(false)) && - does_contain); + // This call will succeed if the module was ever imported in any Debugger in + // the lifetime of the process in which this LLDB framework is living. + const bool does_contain_executed = ExecuteOneLineWithReturn( + command_stream.GetData(), + ScriptInterpreterPythonImpl::eScriptReturnTypeBool, &does_contain, exc_options); + + const bool was_imported_globally = does_contain_executed && does_contain; const bool was_imported_locally = GetSessionDictionary() .GetItemForKey(PythonString(module_name)) @@ -2876,10 +2914,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( } else command_stream.Printf("import %s", module_name.c_str()); - error = ExecuteMultipleLines(command_stream.GetData(), - ExecuteScriptOptions() - .SetEnableIO(false) - .SetSetLLDBGlobals(false)); + error = ExecuteMultipleLines(command_stream.GetData(), exc_options); if (error.Fail()) return false; @@ -2898,7 +2933,8 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule( void *module_pyobj = nullptr; if (ExecuteOneLineWithReturn( command_stream.GetData(), - ScriptInterpreter::eScriptReturnTypeOpaqueObject, &module_pyobj) && + ScriptInterpreter::eScriptReturnTypeOpaqueObject, &module_pyobj, + exc_options) && module_pyobj) *module_sp = std::make_shared(module_pyobj); } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h index e0056f1..d1b0b3f 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h @@ -234,7 +234,8 @@ public: bool RunScriptFormatKeyword(const char *impl_function, ValueObject *value, std::string &output, Status &error) override; - bool LoadScriptingModule(const char *filename, bool init_session, + bool LoadScriptingModule(const char *filename, + const LoadScriptOptions &options, lldb_private::Status &error, StructuredData::ObjectSP *module_sp = nullptr, FileSpec extra_search_dir = {}) override; diff --git a/lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test b/lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test new file mode 100644 index 0000000..58e1543 --- /dev/null +++ b/lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test @@ -0,0 +1,8 @@ +# REQUIRES: python + +# RUN: %lldb --script-language python -o 'command script import lldb.macosx.crashlog' 2>&1 | FileCheck %s +# RUN: %lldb --script-language python -o 'command script import -s lldb.macosx.crashlog' 2>&1 | FileCheck %s --check-prefix SILENT +# RUN: %lldb --script-language python -o 'command script import --silent lldb.macosx.crashlog' 2>&1 | FileCheck %s --check-prefix SILENT + +CHECK: commands have been installed +SILENT-NOT: commands have been installed -- 2.7.4