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

Reply via email to