[lldb/bindings] Add Python ScriptedProcess base class to lldb module

[mib]

Summary

In order to facilitate the writting of Scripted Processes, this patch
introduces a ScriptedProcess python base class in the lldb module.

The base class holds the python interface with all the - abstract -
methods that need to be implemented by the inherited class but also some
methods that can be overwritten.

This patch also provides an example of a Scripted Process with the
MyScriptedProcess class.

rdar://65508855

Signed-off-by: Med Ismail Bennani <[e-mail]>

[mib]

Fix typo in swig wrapper.

[JDevlieghere]

Can you add a test for this too? I assume I would look very similar to the example. Maybe have one that checks the base class and one that overrides it with some known values.

lldb/bindings/python/python-scripted-process.swig

1 ↗

(On Diff #320250)

Does this have to be a SWIG file? I guess there's nothing really special about this file except that it should end up in the lldb module?

22 ↗

(On Diff #320250)

I don't think this adds much. I think it would be more valuable to have pydoc style comments on the methods.

28 ↗

(On Diff #320250)

Type annotations are Python 3 only. We've agreed to keep the bindings Python 2 compatible until at least after the 13 release is cut. (https://lists.llvm.org/pipermail/lldb-dev/2020-August/016388.html)

[mib]

Address @JDevlieghere comments:

• Instead of having the ScriptedProcess python base class in a SWIG file, I moved it to a python file in the example folder. There is a post-build event that copies it to the python lldb module under to lldb.plugins package.
• Added pydoc documentation to the ScriptedProcess base class.
• Removed all the typing hints that prevented the feature to be compatible with Python 2. I, however, kept them in the MyScriptProcess example.

I also added a first test that checks that the base class exists in the lldb python module.

[labath]

Unfortunately I don't have the bandwidth to review this, though I feel that this should have a wider discussion, given that its destined to become a pretty big chunk of our public immutable api surface.
Some questions to seed that discussion:

get_num_memory_regions() -> int:
get_memory_region_at_index(idx: int) -> lldb.SBMemoryRegionInfo:

This means that the implementation needs to know the exact count of memory regions at any given moment. Elsewhere, we've handled this by having a single api like get_memory_region_containing_address. This permits (but doesn't force) the implementation to be lazy in calculating the memory regions, and the caller can still enumerate the memory regions by starting with get_memory_region_containing_address(0) and continuing by get_memory_region_containing_address(prev_region.base + prev_region+size). I think we should do the same here.

get_num_threads() -> int:
get_thread_at_index(idx: int) -> Dict:

This suffers from the same problem, though the solution is not that simple. But given that, not too long ago, we've (me&Jim) had a long discussion about how materializing all the threads all the time is prohibitively expensive, it might be good to elaborate on how exactly is this supposed to work (must the implementation always return all threads, or can it have threads disappear temporarily, etc.)

get_register_for_thread(tid:int) -> Dict:

I guess this should be at least get_registers_for_thread

read_memory_at_address(addr:int) -> lldb.SBData:

How much memory is this supposed to read?

can_debug() -> bool:

What's the use case for this?

[mib]

In D95712#2563091, @labath wrote:

Unfortunately I don't have the bandwidth to review this, though I feel that this should have a wider discussion, given that its destined to become a pretty big chunk of our public immutable api surface.

Thanks for taking the time to peek at this :) !

Some questions to seed that discussion:

get_num_memory_regions() -> int:
get_memory_region_at_index(idx: int) -> lldb.SBMemoryRegionInfo:

This means that the implementation needs to know the exact count of memory regions at any given moment. Elsewhere, we've handled this by having a single api like get_memory_region_containing_address. This permits (but doesn't force) the implementation to be lazy in calculating the memory regions, and the caller can still enumerate the memory regions by starting with get_memory_region_containing_address(0) and continuing by get_memory_region_containing_address(prev_region.base + prev_region+size). I think we should do the same here.

get_num_threads() -> int:
get_thread_at_index(idx: int) -> Dict:

This suffers from the same problem, though the solution is not that simple. But given that, not too long ago, we've (me&Jim) had a long discussion about how materializing all the threads all the time is prohibitively expensive, it might be good to elaborate on how exactly is this supposed to work (must the implementation always return all threads, or can it have threads disappear temporarily, etc.)

Sounds good! Regarding the threads, after talking to @jingham and @jasonmolenda, it sounds like XNU doesn't have a way to provide all its kernel threads at a single time, only the one loaded in each CPU core.
I changed the python interface to allow lazy loading, using the address to fetch a memory region or a thread ID to fetch a certain thread.

get_register_for_thread(tid:int) -> Dict:

I guess this should be at least get_registers_for_thread

Typo :p !

read_memory_at_address(addr:int) -> lldb.SBData:

How much memory is this supposed to read?

Right ... Originally, I didn't want to limit the users on how much data they should store in the SBData, that's why I didn't provided the size and checked the SBData size in the C++ interface.
In hindsight, this was a wrong design.

can_debug() -> bool:

What's the use case for this?

Indeed, that didn't serve much purpose. Got rid of it :)

[mib]

Changed python API to allow lazy fetching for threads and memory regions.

[JDevlieghere]

In D95712#2563091, @labath wrote:

Unfortunately I don't have the bandwidth to review this, though I feel that this should have a wider discussion, given that its destined to become a pretty big chunk of our public immutable api surface.

When @mib and I discussed this offline, the idea was to start with an interface that matches the C++ class and iterate on it as we gain more experience through different use cases. Initially those uses cases consist of core file debugging with a scripted process and using it to write tests for things that have historically hard to test (@jasonmolenda has a better idea of what these are). Long term the scriptable process plugin would be like the OS plugins on steroids and would probably superseed them. I expect the XNU people (who are currently the main users of the OS plugins) to have valuable input on the interface as well. Until then the interface shouldn't be considered stable.

@mib Can you please add a disclaimed to the base class saying that the interface is subject to change?

[mib]

Add disclaimer in the base class docstring saying that the interface is not stable.

[JDevlieghere]

lldb/examples/python/scripted_process/scripted_process.py

19

Doesn't the python documentation generator infer the attributes and methods from this file? If not, is it worth having to keep it in sync?

68–72

Personally I don't think these belongs in the base class:

1. The corresponding methods don't return these lists.
2. More importantly, if they're attributes then that becomes part of the interface. Now every implementation needs to populate e.g. the threads list while they might want to compute that lazily, like some of the OS plugin implementations do.

lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

28

None of the expect calls seems to be checking that the package exists and can be imported?

34

It's not clear to me what this is actually testing. The same is true for the other expect calls below. Maybe a comment or add a failure string to the expect calls.

[mib]

Address @JDevlieghere comments.

[JDevlieghere]

This LGTM. @labath are you fine with landing this and incrementally improving the interface?

[mib]

Remove repetitive documentation.

[mib]

In D95712#2572343, @JDevlieghere wrote:

This LGTM. @labath are you fine with landing this and incrementally improving the interface?

@labath Any objection on the patches ? We would like to land these as soon as possible.

[mib]

Hey @labath! I'm going to land this patch but I'm happy to iterate over it if you have more feedbacks.

[labath]

In D95712#2566203, @JDevlieghere wrote:

In D95712#2563091, @labath wrote:

Unfortunately I don't have the bandwidth to review this, though I feel that this should have a wider discussion, given that its destined to become a pretty big chunk of our public immutable api surface.

When @mib and I discussed this offline, the idea was to start with an interface that matches the C++ class and iterate on it as we gain more experience through different use cases. Initially those uses cases consist of core file debugging with a scripted process and using it to write tests for things that have historically hard to test (@jasonmolenda has a better idea of what these are). Long term the scriptable process plugin would be like the OS plugins on steroids and would probably superseed them. I expect the XNU people (who are currently the main users of the OS plugins) to have valuable input on the interface as well. Until then the interface shouldn't be considered stable.

@mib Can you please add a disclaimed to the base class saying that the interface is subject to change?

Making the api flexible alleviates a lot of my concerns. Thanks.

lldb/bindings/python/CMakeLists.txt

function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_target_dir)
 
   create_python_package(
     ${swig_target}
     ${lldb_python_target_dir}
     "utils"
     FILES "${LLDB_SOURCE_DIR}/examples/python/in_call_stack.py"
           "${LLDB_SOURCE_DIR}/examples/python/symbolication.py")
 
+  create_python_package(
+    ${swig_target}
+    ${lldb_python_target_dir}
+    "plugins"
+    FILES
+    "${LLDB_SOURCE_DIR}/examples/python/scripted_process/scripted_process.py")
+
   if(APPLE)
     create_python_package(
       ${swig_target}
       ${lldb_python_target_dir} "macosx"
       FILES "${LLDB_SOURCE_DIR}/examples/python/crashlog.py"
             "${LLDB_SOURCE_DIR}/examples/darwin/heap_find/heap.py")
 
     create_python_package(

lldb/examples/python/scripted_process/my_scripted_process.py

+import os
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+
+class MyScriptedProcess(ScriptedProcess):
+    def __init__(self, target: lldb.SBTarget, args : lldb.SBStructuredData):
+        super().__init__(target, args)
+
+    def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+        return self.memory_regions[0]
+
+    def get_thread_with_id(self, tid: int):
+        return {}
+
+    def get_registers_for_thread(self, tid: int):
+        return {}
+
+    def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+        data = lldb.SBData().CreateDataFromCString(
+                                    self.target.GetByteOrder(),
+                                    self.target.GetCodeByteSize(),
+                                    "Hello, world!")
+        return data
+
+    def get_loaded_images(self):
+        return self.loaded_images
+
+    def get_process_id(self) -> int:
+        return 42
+
+    def is_alive(self) -> bool:
+        return True
+
+def __lldb_init_module(debugger, dict):
+    if not 'SKIP_SCRIPTED_PROCESS_LAUNCH' in os.environ:
+        debugger.HandleCommand(
+            "process launch -C %s.%s" % (__name__,
+                                     MyScriptedProcess.__name__))
+    else:
+        print("Name of the class that will manage the scripted process: '%s.%s'"
+                % (__name__, MyScriptedProcess.__name__))
 No newline at end of file

lldb/examples/python/scripted_process/scripted_process.py

+from abc import ABCMeta, abstractmethod
+import six
+
+import lldb
+
[e-mail]_metaclass(ABCMeta)
+class ScriptedProcess:
+
+    """
+    The base class for a scripted process.
+
+    Most of the base class methods are `@abstractmethod` that need to be
+    overwritten by the inheriting class.
+
+    DISCLAIMER: THIS INTERFACE IS STILL UNDER DEVELOPMENT AND NOT STABLE.
+                THE METHODS EXPOSED MIGHT CHANGE IN THE FUTURE.
+    """
+
+    @abstractmethod
+    def __init__(self, target, args):
+        """ Construct a scripted process.
+
+        Args:
+            target (lldb.SBTarget): The target launching the scripted process.
+            args (lldb.SBStructuredData): A Dictionary holding arbitrary
+                key/value pairs used by the scripted process.
+        """
+        self.target = None
+        self.args = None
+        if isinstance(target, lldb.SBTarget) and target.IsValid():
+            self.target = target
+        if isinstance(args, lldb.SBStructuredData) and args.IsValid():
+            self.args = args
+
+    @abstractmethod
+    def get_memory_region_containing_address(addr):
+        """ Get the memory region for the scripted process, containing a
+            specific address.
+
+        Args:
+            addr (int): Address to look for in the scripted process memory
+                regions.
+
+        Returns:
+            lldb.SBMemoryRegionInfo: The memory region containing the address.
+                None if out of bounds.
+        """
+        pass
+
+    @abstractmethod
+    def get_thread_with_id(tid):
+        """ Get the scripted process thread with a specific ID.
+
+        Args:
+            tid (int): Thread ID to look for in the scripted process.
+
+        Returns:
+            Dict: The thread represented as a dictionary, withr the
+                tid thread ID. None if tid doesn't match any of the scripted
+                process threads.
+        """
+        pass
+
+    @abstractmethod
+    def get_registers_for_thread(tid):
+        """ Get the register context dictionary for a certain thread of
+            the scripted process.
+
+        Args:
+            tid (int): Thread ID for the thread's register context.
+
+        Returns:
+            Dict: The register context represented as a dictionary, for the
+                tid thread. None if tid doesn't match any of the scripted
+                process threads.
+        """
+        pass
+
+    @abstractmethod
+    def read_memory_at_address(addr, size):
+        """ Get a memory buffer from the scripted process at a certain address,
+            of a certain size.
+
+        Args:
+            addr (int): Address from which we should start reading.
+            size (int): Size of the memory to read.
+
+        Returns:
+            lldb.SBData: An `lldb.SBData` buffer with the target byte size and
+                byte order storing the memory read.
+        """
+        pass
+
+    @abstractmethod
+    def get_loaded_images(self):
+        """ Get the list of loaded images for the scripted process.
+
+        ```
+        class ScriptedProcessImage:
+            def __init__(name, file_spec, uuid, load_address):
+              self.name = name
+              self.file_spec = file_spec
+              self.uuid = uuid
+              self.load_address = load_address
+        ```
+
+        Returns:
+            List[ScriptedProcessImage]: A list of `ScriptedProcessImage`
+                containing for each entry, the name of the library, a UUID,
+                an `lldb.SBFileSpec` and a load address.
+                None if the list is empty.
+        """
+        pass
+
+    def get_process_id(self):
+        """ Get the scripted process identifier.
+
+        Returns:
+            int: The scripted process identifier.
+        """
+        return 0
+
+
+    def launch(self):
+        """ Simulate the scripted process launch.
+
+        Returns:
+            lldb.SBError: An `lldb.SBError` with error code 0.
+        """
+        return lldb.SBError()
+
+    def resume(self):
+        """ Simulate the scripted process resume.
+
+        Returns:
+            lldb.SBError: An `lldb.SBError` with error code 0.
+        """
+        return lldb.SBError()
+
+    @abstractmethod
+    def is_alive(self):
+        """ Check if the scripted process is alive.
+
+        Returns:
+            bool: True if scripted process is alive. False otherwise.
+        """
+        pass

lldb/test/API/functionalities/scripted_process/Makefile

+C_SOURCES := main.c
+
+include Makefile.rules
+

lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

+"""
+Test python scripted process in lldb
+"""
+
+import os
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbtest
+
+
+class PlatformProcessCrashInfoTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+        self.source = "main.c"
+
+    def tearDown(self):
+        TestBase.tearDown(self)
+
+    def test_python_plugin_package(self):
+        """Test that the lldb python module has a `plugins.scripted_process`
+        package."""
+        self.expect('script import lldb.plugins',
+                    substrs=["ModuleNotFoundError"], matching=False)
+
+        self.expect('script dir(lldb.plugins)',
+                    substrs=["scripted_process"])
+
+        self.expect('script import lldb.plugins.scripted_process',
+                    substrs=["ModuleNotFoundError"], matching=False)
+
+        self.expect('script dir(lldb.plugins.scripted_process)',
+                    substrs=["ScriptedProcess"])
+
+        self.expect('script from lldb.plugins.scripted_process import ScriptedProcess',
+                    substrs=["ImportError"], matching=False)
+
+        self.expect('script dir(ScriptedProcess)',
+                    substrs=["launch"])
+

lldb/test/API/functionalities/scripted_process/main.c

+#include <stdlib.h>
+
+int main() {
+  return 0; // break here
+}