JDevlieghere updated this revision to Diff 357037.
JDevlieghere marked 3 inline comments as done.
JDevlieghere added a comment.
- Address code review feedback
- Use `LoadScriptOptions`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105327/new/
https://reviews.llvm.org/D105327
Files:
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/Options.td
lldb/source/Core/Module.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test
Index: lldb/test/Shell/ScriptInterpreter/Python/silent_command_script_import.test
===================================================================
--- /dev/null
+++ 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
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -234,7 +234,8 @@
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;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1077,11 +1077,24 @@
llvm::StringRef in_string, ScriptInterpreter::ScriptReturnType return_type,
void *ret_value, const ExecuteScriptOptions &options) {
+ llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+ 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 @@
if (in_string == nullptr)
return Status();
+ llvm::Expected<std::unique_ptr<ScriptInterpreterIORedirect>>
+ 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 @@
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 @@
}
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<std::unique_ptr<ScriptInterpreterIORedirect>>
+ 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<llvm::StringError>(
"invalid directory name", llvm::inconvertibleErrorCode());
@@ -2766,11 +2811,7 @@
"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<llvm::StringError>(
"Python sys.path handling failed", llvm::inconvertibleErrorCode());
@@ -2844,21 +2885,18 @@
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 @@
} 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 @@
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<StructuredPythonObject>(module_pyobj);
}
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
@@ -42,7 +42,8 @@
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;
Index: lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
@@ -195,8 +195,9 @@
}
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)) {
Index: lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
===================================================================
--- lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -91,13 +91,13 @@
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)
Index: lldb/source/Interpreter/ScriptInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/ScriptInterpreter.cpp
+++ lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -47,7 +47,7 @@
}
bool ScriptInterpreter::LoadScriptingModule(const char *filename,
- bool init_session,
+ const LoadScriptOptions &options,
lldb_private::Status &error,
StructuredData::ObjectSP *module_sp,
FileSpec extra_search_dir) {
Index: lldb/source/Core/Module.cpp
===================================================================
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -1528,9 +1528,9 @@
}
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;
}
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -745,6 +745,8 @@
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 {
Index: lldb/source/Commands/CommandObjectCommands.cpp
===================================================================
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1242,6 +1242,9 @@
case 'c':
relative_to_command_file = true;
break;
+ case 's':
+ silent = true;
+ break;
default:
llvm_unreachable("Unimplemented option");
}
@@ -1257,6 +1260,7 @@
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 @@
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 @@
// 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",
Index: lldb/include/lldb/Interpreter/ScriptInterpreter.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -71,6 +71,28 @@
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 @@
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 = {});
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
