jingham created this revision. jingham added reviewers: clayborg, JDevlieghere, jasonmolenda, bulbazord. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
We load python modules in two main ways: through `command script import` and automatically when we read in a dSYM and find a scripting resource in the dSYM. The latter case has a slight flaw currently which is that when the scripting resource from the dSYM is loaded, the target it's being loaded for is in the process of being created, so the module being imported has no way of knowing what that target is. Even if the debugger had selected this new target before loading the scripting resource, it would still be relying on that being the "currently selected target" which is racy. It's better to do this explicitly. This patch adds a detector for an optional: __lldb_init_module_with_target and if that is present it will run that routine - passing in the target - before running __lldb_init_module. I considered making an overload of __lldb_init_module that also passed in the target, but that trick seems to end up being confusing. And this will allow more convenient "dual use" modules, where if you are being sourced for a target you can prime the state for the regular init, and if not, then run the regular init. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146152 Files: lldb/bindings/python/python-wrapper.swig lldb/include/lldb/Interpreter/ScriptInterpreter.h lldb/source/Core/Module.cpp lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/test/API/commands/add-dsym/uuid/TestAddDsymCommand.py lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp =================================================================== --- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp +++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp @@ -197,7 +197,7 @@ bool lldb_private::LLDBSwigPythonCallModuleInit( const char *python_module_name, const char *session_dictionary_name, - lldb::DebuggerSP debugger) { + lldb::DebuggerSP debugger, lldb::TargetSP target) { return false; } Index: lldb/test/API/commands/add-dsym/uuid/TestAddDsymCommand.py =================================================================== --- lldb/test/API/commands/add-dsym/uuid/TestAddDsymCommand.py +++ lldb/test/API/commands/add-dsym/uuid/TestAddDsymCommand.py @@ -59,6 +59,17 @@ self.exe_name = 'a.out' self.do_add_dsym_with_dSYM_bundle(self.exe_name) + @no_debug_info_test + def test_add_dsym_checking_initializer(self): + """Test that the 'add-dsym' correctly imports python code.""" + + # Call the program generator to produce main.cpp, version 1. + self.generate_main_cpp(version=1) + self.build(debug_info="dsym") + + self.exe_name = 'a.out' + self.do_add_dsym_and_check_initializers(self.exe_name) + def generate_main_cpp(self, version=0): """Generate main.cpp from main.cpp.template.""" temp = os.path.join(self.getSourceDir(), self.template) @@ -122,7 +133,45 @@ exe_path = self.getBuildArtifact(exe_name) self.runCmd("file " + exe_path, CURRENT_EXECUTABLE_SET) + # This time, the UUID should be found inside the bundle right_path = "%s.dSYM" % exe_path self.expect("add-dsym " + right_path, substrs=['symbol file', 'has been added to']) + + # Also test that when we added the dSYM, we also ran any python that was available, + # and ran both module initializers in the right order: + def do_add_dsym_and_check_initializers(self, exe_name): + import os, shutil + exe_path = self.getBuildArtifact(exe_name) + target = self.dbg.CreateTarget(exe_path) + self.dbg.SetSelectedTarget(target) + + self.assertTrue(target.IsValid(), "Made a valid target") + + # This time, the UUID should be found inside the bundle + right_path = "%s.dSYM" % exe_path + # Add our scripting resource: + self.assertTrue(os.path.exists(right_path), "dSYM exists") + python_path = os.path.join(right_path, "Contents") + self.assertTrue(os.path.exists(python_path), "Contents exists") + python_path = os.path.join(python_path, "Resources") + self.assertTrue(os.path.exists(python_path), "Resources exists") + python_path = os.path.join(python_path, "Python") + os.mkdir(python_path) + source_name = "a_out.py" + shutil.copy(os.path.join(self.getSourceDir(), source_name), + os.path.join(python_path, source_name)) + + # Turn on reading in python in dsym files: + self.runCmd("settings set target.load-script-from-symbol-file true") + self.expect("add-dsym " + right_path, + substrs=['symbol file', 'has been added to']) + + + self.expect("script a_out.called_module_init", substrs=["True"], + msg = "Called module initialization") + self.expect("script a_out.called_target_init", substrs=["True"], + msg = "Called target initializer") + self.expect(f"script a_out.init_target", substrs = [target.executable.fullpath], + msg = "Got the expected target") Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -2723,7 +2723,8 @@ // call __lldb_init_module(debugger,dict) if (!LLDBSwigPythonCallModuleInit(module_name.c_str(), m_dictionary_name.c_str(), - m_debugger.shared_from_this())) { + m_debugger.shared_from_this(), + options.GetTarget())) { error.SetErrorString("calling __lldb_init_module failed"); return false; } Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h +++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h @@ -194,7 +194,8 @@ bool LLDBSwigPythonCallModuleInit(const char *python_module_name, const char *session_dictionary_name, - lldb::DebuggerSP debugger); + lldb::DebuggerSP debugger, + lldb::TargetSP target); python::PythonObject LLDBSWIGPythonCreateOSPlugin(const char *python_class_name, Index: lldb/source/Core/Module.cpp =================================================================== --- lldb/source/Core/Module.cpp +++ lldb/source/Core/Module.cpp @@ -1524,6 +1524,7 @@ StreamString scripting_stream; scripting_fspec.Dump(scripting_stream.AsRawOstream()); LoadScriptOptions options; + options.SetTarget(target->shared_from_this()); bool did_load = script_interpreter->LoadScriptingModule( scripting_stream.GetData(), options, error); if (!did_load) Index: lldb/include/lldb/Interpreter/ScriptInterpreter.h =================================================================== --- lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -92,10 +92,19 @@ m_silent = b; return *this; } + + void SetTarget(lldb::TargetSP target_sp) { + m_target_sp = target_sp; + } + + lldb::TargetSP GetTarget() const { + return m_target_sp; + } private: bool m_init_session = false; bool m_silent = false; + lldb::TargetSP m_target_sp; }; class ScriptInterpreterIORedirect { Index: lldb/bindings/python/python-wrapper.swig =================================================================== --- lldb/bindings/python/python-wrapper.swig +++ lldb/bindings/python/python-wrapper.swig @@ -1019,20 +1019,37 @@ bool lldb_private::LLDBSwigPythonCallModuleInit( const char *python_module_name, const char *session_dictionary_name, - lldb::DebuggerSP debugger) { + lldb::DebuggerSP debugger, lldb::TargetSP target) { + std::string python_function_name_string = python_module_name; - python_function_name_string += ".__lldb_init_module"; - const char *python_function_name = python_function_name_string.c_str(); + const char *python_function_name = nullptr; PyErr_Cleaner py_err_cleaner(true); - auto dict = PythonModule::MainModule().ResolveName<PythonDictionary>( session_dictionary_name); + + // First call the target initializer: + if (target) { + python_function_name_string += ".__lldb_init_module_with_target"; + python_function_name = python_function_name_string.c_str(); + + auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>( + python_function_name, dict); + + if (pfunc.IsAllocated()) + pfunc(ToSWIGWrapper(std::move(target)), dict); + } + + // Now call the general method. also This method is optional and need + // not exist. So if we don't find it, it's actually a success, not a + // failure. + python_function_name_string = python_module_name; + python_function_name_string += ".__lldb_init_module"; + python_function_name = python_function_name_string.c_str(); + auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>( python_function_name, dict); - // This method is optional and need not exist. So if we don't find it, - // it's actually a success, not a failure. if (!pfunc.IsAllocated()) return true;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits