Add a new affordance that the Python module in a dSYM #133290
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
told what target it has been loaded into. When lldb is loading modules, while creating a target, it will run "command script import" on any Python modules in Resources/Python in the dSYM. However, this happens WHILE the target is being created, so it is not yet in the target list. That means that these scripts can't act on the target that they a part of when they get loaded. This patch adds a new python API that lldb will call: __lldb_module_added_to_target if it is defined in the module, passing in the Target the module was being added to, so that code in these dSYM's don't have to guess.
|
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesSo the dSYM can be told what target it has been loaded into. When lldb is loading modules, while creating a target, it will run "command script import" on any Python modules in Resources/Python in the dSYM. However, this happens WHILE the target is being created, so it is not yet in the target list. That means that these scripts can't act on the target that they a part of when they get loaded. This patch adds a new python API that lldb will call: __lldb_module_added_to_target if it is defined in the module, passing in the Target the module was being added to, so that code in these dSYM's don't have to guess. Full diff: https://github.com/llvm/llvm-project/pull/133290.diff 12 Files Affected:
diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 57c7ac387145e..3d1d04e47e70b 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/lldb/bindings/python/python-wrapper.swig
@@ -966,6 +966,28 @@ bool lldb_private::python::SWIGBridge::LLDBSWIGPythonRunScriptKeywordValue(
return true;
}
+bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallModuleNewTarget(
+ const char *python_module_name, const char *session_dictionary_name,
+ lldb::TargetSP target_sp) {
+ std::string python_function_name_string = python_module_name;
+ python_function_name_string += ".__lldb_module_added_to_target";
+ const char *python_function_name = python_function_name_string.c_str();
+
+ PyErr_Cleaner py_err_cleaner(true);
+
+ auto dict = PythonModule::MainModule().ResolveName<PythonDictionary>(
+ session_dictionary_name);
+ auto pfunc = PythonObject::ResolveNameWithDictionary<PythonCallable>(
+ python_function_name, dict);
+
+ if (!pfunc.IsAllocated())
+ return true;
+
+ pfunc(SWIGBridge::ToSWIGWrapper(std::move(target_sp)), dict);
+
+ return true;
+}
+
bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallModuleInit(
const char *python_module_name, const char *session_dictionary_name,
lldb::DebuggerSP debugger) {
diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index c5aa19959aa61..25e82779f05c6 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -522,7 +522,8 @@ class ScriptInterpreter : public PluginInterface {
LoadScriptingModule(const char *filename, const LoadScriptOptions &options,
lldb_private::Status &error,
StructuredData::ObjectSP *module_sp = nullptr,
- FileSpec extra_search_dir = {});
+ FileSpec extra_search_dir = {},
+ lldb::TargetSP loaded_into_target_sp = {});
virtual bool IsReservedWord(const char *word) { return false; }
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index d70f292abaea4..7b52ba08cd379 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1485,7 +1485,9 @@ bool Module::LoadScriptingResourceInTarget(Target *target, Status &error,
scripting_fspec.Dump(scripting_stream.AsRawOstream());
LoadScriptOptions options;
bool did_load = script_interpreter->LoadScriptingModule(
- scripting_stream.GetData(), options, error);
+ scripting_stream.GetData(), options, error,
+ /*module_sp*/ nullptr, /*extra_path*/ {},
+ target->shared_from_this());
if (!did_load)
return false;
}
diff --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index 4424b6c894356..8d9655a07e7e9 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -52,7 +52,8 @@ bool ScriptInterpreter::LoadScriptingModule(const char *filename,
const LoadScriptOptions &options,
lldb_private::Status &error,
StructuredData::ObjectSP *module_sp,
- FileSpec extra_search_dir) {
+ FileSpec extra_search_dir,
+ lldb::TargetSP loaded_into_target_sp) {
error = Status::FromErrorString(
"This script interpreter does not support importing modules.");
return false;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h b/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
index a2252d164ab83..137d7c53636a0 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -215,6 +215,10 @@ class SWIGBridge {
const char *session_dictionary_name,
lldb::DebuggerSP debugger);
+ static bool LLDBSwigPythonCallModuleNewTarget(const char *python_module_name,
+ const char *session_dictionary_name,
+ lldb::TargetSP target);
+
static python::PythonObject
LLDBSWIGPythonCreateOSPlugin(const char *python_class_name,
const char *session_dictionary_name,
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 4b7694de697c1..b4bde233d3e5f 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2316,7 +2316,8 @@ uint64_t replace_all(std::string &str, const std::string &oldStr,
bool ScriptInterpreterPythonImpl::LoadScriptingModule(
const char *pathname, const LoadScriptOptions &options,
lldb_private::Status &error, StructuredData::ObjectSP *module_sp,
- FileSpec extra_search_dir) {
+ FileSpec extra_search_dir,
+ lldb::TargetSP target_sp) {
namespace fs = llvm::sys::fs;
namespace path = llvm::sys::path;
@@ -2495,6 +2496,12 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
PyRefType::Owned, static_cast<PyObject *>(module_pyobj)));
}
+ // Finally, if we got a target passed in, then we should tell the new module
+ // about this target:
+ if (target_sp) {
+ return SWIGBridge::LLDBSwigPythonCallModuleNewTarget(module_name.c_str(),
+ m_dictionary_name.c_str(), target_sp);
+ }
return true;
}
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index 2dc784777151b..0f2902813397a 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -245,7 +245,8 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
const LoadScriptOptions &options,
lldb_private::Status &error,
StructuredData::ObjectSP *module_sp = nullptr,
- FileSpec extra_search_dir = {}) override;
+ FileSpec extra_search_dir = {},
+ lldb::TargetSP loaded_into_target_sp = {}) override;
bool IsReservedWord(const char *word) override;
diff --git a/lldb/test/API/macosx/dsym_modules/Makefile b/lldb/test/API/macosx/dsym_modules/Makefile
new file mode 100644
index 0000000000000..695335e068c0c
--- /dev/null
+++ b/lldb/test/API/macosx/dsym_modules/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
diff --git a/lldb/test/API/macosx/dsym_modules/TestdSYMModuleInit.py b/lldb/test/API/macosx/dsym_modules/TestdSYMModuleInit.py
new file mode 100644
index 0000000000000..5458d195bc713
--- /dev/null
+++ b/lldb/test/API/macosx/dsym_modules/TestdSYMModuleInit.py
@@ -0,0 +1,68 @@
+"""
+Test that we read in the Python module from a dSYM, and run the
+init in debugger and the init in target routines.
+"""
+
+import os, shutil
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+@skipUnlessDarwin
+class TestdSYMModuleInit(TestBase):
+
+ @no_debug_info_test
+ def test_add_module(self):
+ """This loads a file into a target and ensures that the python module was
+ correctly added and the two intialization functions are called."""
+ self.exe_name = "has_dsym"
+ self.py_name = self.exe_name+".py"
+
+ # Now load the target the first time into the debugger:
+ self.runCmd("settings set target.load-script-from-symbol-file true")
+ self.interp = self.dbg.GetCommandInterpreter()
+
+ executable = self.build_dsym(self.exe_name+"_1")
+ target = self.createTestTarget(file_path=executable)
+ self.check_answers(executable, ['1', '1', "has_dsym_1"])
+
+ # Now make a second target and make sure both get called:
+ executable_2 = self.build_dsym(self.exe_name+"_2")
+ target_2 = self.createTestTarget(file_path=executable_2)
+ self.check_answers(executable_2, ['2', '2', "has_dsym_2"])
+
+ def check_answers(self, name, answers):
+ result = lldb.SBCommandReturnObject()
+ self.interp.HandleCommand("report_command", result)
+ self.assertTrue(result.Succeeded(), f"report_command succeeded {result.GetError()}")
+
+ cmd_results = result.GetOutput().split()
+ self.assertEqual(answers[0], cmd_results[0], "Right number of module imports")
+ self.assertEqual(answers[1], cmd_results[1], "Right number of target notices")
+ self.assertIn(answers[2], name, "Right target name")
+
+
+ def build_dsym(self, name):
+ self.build(debug_info="dsym", dictionary={"EXE" : name})
+ executable = self.getBuildArtifact(name)
+ dsym_path = self.getBuildArtifact(name+".dSYM")
+ python_dir_path = dsym_path
+ python_dir_path = os.path.join(
+ dsym_path, "Contents", "Resources", "Python"
+ )
+ if not os.path.exists(python_dir_path):
+ os.mkdir(python_dir_path)
+
+ python_file_name = name+".py"
+
+ module_dest_path = os.path.join(
+ python_dir_path, python_file_name
+ )
+ module_origin_path = os.path.join(self.getSourceDir(), self.py_name)
+ shutil.copy(module_origin_path, module_dest_path)
+
+ return executable
+
+
diff --git a/lldb/test/API/macosx/dsym_modules/has_dsym.py b/lldb/test/API/macosx/dsym_modules/has_dsym.py
new file mode 100644
index 0000000000000..ed5f4dac08f2a
--- /dev/null
+++ b/lldb/test/API/macosx/dsym_modules/has_dsym.py
@@ -0,0 +1,21 @@
+import lldb
+
+def report_command(debugger, command, exe_ctx, result, internal_dict):
+ result.AppendMessage(f"{lldb.num_module_inits} {lldb.num_target_inits} \"{lldb.target_name}\"")
+ result.SetStatus(lldb.eReturnStatusSuccessFinishResult)
+
+def __lldb_init_module(debugger, internal_dict):
+ # We only want to make one copy of the report command so it will be shared
+ if "has_dsym_1" in __name__:
+ # lldb is a convenient place to store our counters.
+ lldb.num_module_inits = 0
+ lldb.num_target_inits = 0
+ lldb.target_name = "<unknown>"
+
+ debugger.HandleCommand(f"command script add -o -f '{__name__}.report_command' report_command")
+
+ lldb.num_module_inits += 1
+
+def __lldb_module_added_to_target(target, internal_dict):
+ lldb.num_target_inits += 1
+ target_name = target.executable.fullpath
diff --git a/lldb/test/API/macosx/dsym_modules/main.c b/lldb/test/API/macosx/dsym_modules/main.c
new file mode 100644
index 0000000000000..df22f570bf33a
--- /dev/null
+++ b/lldb/test/API/macosx/dsym_modules/main.c
@@ -0,0 +1,15 @@
+#include <stdio.h>
+
+int global_test_var = 10;
+
+int
+main()
+{
+ int test_var = 10;
+ printf ("Set a breakpoint here: %d.\n", test_var);
+ //% test_var = self.frame().FindVariable("test_var")
+ //% test_value = test_var.GetValueAsUnsigned()
+ //% self.assertSuccess(test_var.GetError(), "Failed to fetch test_var")
+ //% self.assertEqual(test_value, 10, "Failed to get the right value for test_var")
+ return global_test_var;
+}
diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
index 3faeb587c3a91..ca42e9e00faf6 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -230,6 +230,13 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallModuleInit(
return false;
}
+bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallModuleNewTarget(
+ const char *python_module_name, const char *session_dictionary_name,
+ lldb::TargetSP target) {
+ return false;
+}
+
+
python::PythonObject
lldb_private::python::SWIGBridge::LLDBSWIGPythonCreateOSPlugin(
const char *python_class_name, const char *session_dictionary_name,
|
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Do we want to just do the "command script import" after the target is created to avoid this? Or it would be great to have a way for a python script to add a callback function to be run after a target is created. So this diff could just do something like: Many scripts would love to be able to add functionality during the lifetime of a debug session like:
|
Adding callbacks for "life-cycle events" of a target to lldb is a long-standing but considerably larger piece of work. We would want there to be a command line way to do this, and register & deregister actions, etc. Basically a general extension to what we do with stop-hooks. I don't have time to add that feature as the solution to this fairly narrow problem right now. Just deferring the command script import doesn't seem like a very robust solution, because you still aren't telling the code in the dSYM module what Target it's getting loaded into. The only way that the |
JDevlieghere
approved these changes
Mar 28, 2025
Comment on lines
2500
to
2503
| if (target_sp) { | ||
| return SWIGBridge::LLDBSwigPythonCallModuleNewTarget( | ||
| module_name.c_str(), m_dictionary_name.c_str(), target_sp); | ||
| } |
There was a problem hiding this comment.
Nit: no braces around single-line (statement?) ifs.
JDevlieghere
added a commit
that referenced
this pull request
Apr 2, 2025
Update the ScriptInterpreterLua::LoadScriptingModule signature after the TargetSP argument was added in #133290.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
Apr 2, 2025
So the dSYM can be told what target it has been loaded into. When lldb is loading modules, while creating a target, it will run "command script import" on any Python modules in Resources/Python in the dSYM. However, this happens WHILE the target is being created, so it is not yet in the target list. That means that these scripts can't act on the target that they a part of when they get loaded. This patch adds a new python API that lldb will call: __lldb_module_added_to_target if it is defined in the module, passing in the Target the module was being added to, so that code in these dSYM's don't have to guess.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
Apr 2, 2025
Update the ScriptInterpreterLua::LoadScriptingModule signature after the TargetSP argument was added in llvm#133290.
jimingham
added a commit
to jimingham/from-apple-llvm-project
that referenced
this pull request
Apr 9, 2025
So the dSYM can be told what target it has been loaded into. When lldb is loading modules, while creating a target, it will run "command script import" on any Python modules in Resources/Python in the dSYM. However, this happens WHILE the target is being created, so it is not yet in the target list. That means that these scripts can't act on the target that they a part of when they get loaded. This patch adds a new python API that lldb will call: __lldb_module_added_to_target if it is defined in the module, passing in the Target the module was being added to, so that code in these dSYM's don't have to guess. (cherry picked from commit 347c5a7)
jimingham
pushed a commit
to jimingham/from-apple-llvm-project
that referenced
this pull request
Apr 9, 2025
Update the ScriptInterpreterLua::LoadScriptingModule signature after the TargetSP argument was added in llvm#133290. (cherry picked from commit 28b300d)
jimingham
added a commit
to jimingham/from-apple-llvm-project
that referenced
this pull request
Apr 9, 2025
So the dSYM can be told what target it has been loaded into. When lldb is loading modules, while creating a target, it will run "command script import" on any Python modules in Resources/Python in the dSYM. However, this happens WHILE the target is being created, so it is not yet in the target list. That means that these scripts can't act on the target that they a part of when they get loaded. This patch adds a new python API that lldb will call: __lldb_module_added_to_target if it is defined in the module, passing in the Target the module was being added to, so that code in these dSYM's don't have to guess. (cherry picked from commit 347c5a7)
jimingham
pushed a commit
to jimingham/from-apple-llvm-project
that referenced
this pull request
Apr 9, 2025
Update the ScriptInterpreterLua::LoadScriptingModule signature after the TargetSP argument was added in llvm#133290. (cherry picked from commit 28b300d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So the dSYM can be told what target it has been loaded into.
When lldb is loading modules, while creating a target, it will run "command script import" on any Python modules in Resources/Python in the dSYM. However, this happens WHILE the target is being created, so it is not yet in the target list. That means that these scripts can't act on the target that they a part of when they get loaded.
This patch adds a new python API that lldb will call:
__lldb_module_added_to_target
if it is defined in the module, passing in the Target the module was being added to, so that code in these dSYM's don't have to guess.