Skip to content

Add the ability to define a Python based command that uses CommandObjectParsed #70734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 13, 2024
Merged

Add the ability to define a Python based command that uses CommandObjectParsed #70734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a0c72c0
Add the ability to define a Python based command that uses the
jimingham Oct 30, 2023
580f68d
First pass at addressing review comments
jimingham Jan 21, 2024
48eaf17
Address more review comments
jimingham Jan 27, 2024
6c24cc6
Finishing up the current review comments in CommandObjectCommands.cpp.
jimingham Jan 30, 2024
e1b7cee
Convert the cmdtemplate.py to use the lldb parsed commands.
jimingham Feb 3, 2024
ebc5cee
Change usage->help and varname->dest
jimingham Feb 5, 2024
d396962
Address David's latest review comments.
jimingham Feb 6, 2024
2137361
Address Ismail's review comments.
jimingham Feb 12, 2024
ea2b242
One more review nit.
jimingham Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lldb/bindings/python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,15 @@ function(finish_swig_python swig_target lldb_python_bindings_dir lldb_python_tar
${lldb_python_target_dir}
"utils"
FILES "${LLDB_SOURCE_DIR}/examples/python/in_call_stack.py"
"${LLDB_SOURCE_DIR}/examples/python/symbolication.py")
"${LLDB_SOURCE_DIR}/examples/python/symbolication.py"
)

create_python_package(
${swig_target}
${lldb_python_target_dir}
"plugins"
FILES
"${LLDB_SOURCE_DIR}/examples/python/templates/parsed_cmd.py"
"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_process.py"
"${LLDB_SOURCE_DIR}/examples/python/templates/scripted_platform.py"
"${LLDB_SOURCE_DIR}/examples/python/templates/operating_system.py")
Expand Down
31 changes: 27 additions & 4 deletions lldb/bindings/python/python-wrapper.swig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,12 @@ PythonObject lldb_private::python::SWIGBridge::LLDBSwigPythonCreateScriptedThrea
}

bool lldb_private::python::SWIGBridge::LLDBSWIGPythonCallThreadPlan(
void *implementor, const char *method_name, lldb_private::Event *event,
void *implementer, const char *method_name, lldb_private::Event *event,
bool &got_error) {
got_error = false;

PyErr_Cleaner py_err_cleaner(false);
PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementor));
PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementer));
auto pfunc = self.ResolveName<PythonCallable>(method_name);

if (!pfunc.IsAllocated())
Expand Down Expand Up @@ -325,12 +325,12 @@ bool lldb_private::python::SWIGBridge::LLDBSWIGPythonCallThreadPlan(
}

bool lldb_private::python::SWIGBridge::LLDBSWIGPythonCallThreadPlan(
void *implementor, const char *method_name, lldb_private::Stream *stream,
void *implementer, const char *method_name, lldb_private::Stream *stream,
bool &got_error) {
got_error = false;

PyErr_Cleaner py_err_cleaner(false);
PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementor));
PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementer));
auto pfunc = self.ResolveName<PythonCallable>(method_name);

if (!pfunc.IsAllocated())
Expand Down Expand Up @@ -831,6 +831,29 @@ bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallCommandObject(
return true;
}

#include "lldb/Interpreter/CommandReturnObject.h"

bool lldb_private::python::SWIGBridge::LLDBSwigPythonCallParsedCommandObject(
PyObject *implementor, lldb::DebuggerSP debugger, lldb_private::StructuredDataImpl &args_impl,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling mistake: implementor -> implementer

Copy link
Member

@medismailben medismailben Nov 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll get rid of function altogether later this week, replacing by a call to the ScriptedPythonInterface methods

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's spelled incorrectly in the whole rest of this file except for the thread plan instances. We should either get rid of all of these by using ScriptedPythonInterface, or fix all the spellings to be consistent, but that seems better as a separate commit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix the rest of the spellings in a follow up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, whoever spelt this originally was sure implementor was what they wanted to use, the C++ files use the same spelling.

lldb_private::CommandReturnObject &cmd_retobj,
lldb::ExecutionContextRefSP exe_ctx_ref_sp) {

PyErr_Cleaner py_err_cleaner(true);

PythonObject self(PyRefType::Borrowed, implementor);
auto pfunc = self.ResolveName<PythonCallable>("__call__");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some other safety checks to pfunc other than if it's allocated? Something like "It takes the number of arguments expected"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do any checking for arguments in any of the pfunc lookups in the python wrapper, except in one or two cases where we support overloaded implementation functions. I don't think doing this piecemeal is a great idea. It would be better to have some kind of check_pfunc feature that does the lookup and takes some specification about what to expect of the found function. Then we can insert this in all the current lookups.
But this patch is long enough already, it would be better to do that as a separate patch.


if (!pfunc.IsAllocated()) {
cmd_retobj.AppendError("Could not find '__call__' method in implementation class");
return false;
}

pfunc(SWIGBridge::ToSWIGWrapper(std::move(debugger)), SWIGBridge::ToSWIGWrapper(args_impl),
SWIGBridge::ToSWIGWrapper(exe_ctx_ref_sp), SWIGBridge::ToSWIGWrapper(cmd_retobj).obj());

return true;
}

PythonObject lldb_private::python::SWIGBridge::LLDBSWIGPythonCreateOSPlugin(
const char *python_class_name, const char *session_dictionary_name,
const lldb::ProcessSP &process_sp) {
Expand Down
129 changes: 49 additions & 80 deletions lldb/examples/python/cmdtemplate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,115 +11,84 @@

import inspect
import lldb
import optparse
import shlex
import sys
from lldb.plugins.parsed_cmd import ParsedCommand


class FrameStatCommand:
class FrameStatCommand(ParsedCommand):
program = "framestats"

@classmethod
def register_lldb_command(cls, debugger, module_name):
parser = cls.create_options()
cls.__doc__ = parser.format_help()
# Add any commands contained in this module to LLDB
command = "command script add -o -c %s.%s %s" % (
module_name,
cls.__name__,
cls.program,
)
debugger.HandleCommand(command)
ParsedCommandBase.do_register_cmd(cls, debugger, module_name)
print(
'The "{0}" command has been installed, type "help {0}" or "{0} '
'--help" for detailed help.'.format(cls.program)
)

@classmethod
def create_options(cls):
usage = "usage: %prog [options]"
description = (
"This command is meant to be an example of how to make "
"an LLDB command that does something useful, follows "
"best practices, and exploits the SB API. "
"Specifically, this command computes the aggregate "
"and average size of the variables in the current "
"frame and allows you to tweak exactly which variables "
"are to be accounted in the computation."
)
def setup_command_definition(self):

# Pass add_help_option = False, since this keeps the command in line
# with lldb commands, and we wire up "help command" to work by
# providing the long & short help methods below.
parser = optparse.OptionParser(
description=description,
prog=cls.program,
usage=usage,
add_help_option=False,
self.ov_parser.add_option(
"i",
"in-scope",
help = "in_scope_only = True",
value_type = lldb.eArgTypeBoolean,
dest = "bool_arg",
default = True,
)

parser.add_option(
"-i",
"--in-scope",
action="store_true",
dest="inscope",
help="in_scope_only = True",
self.ov_parser.add_option(
"i",
"in-scope",
help = "in_scope_only = True",
value_type = lldb.eArgTypeBoolean,
dest = "inscope",
default=True,
)

parser.add_option(
"-a",
"--arguments",
action="store_true",
dest="arguments",
help="arguments = True",
default=True,
self.ov_parser.add_option(
"a",
"arguments",
help = "arguments = True",
value_type = lldb.eArgTypeBoolean,
dest = "arguments",
default = True,
)

parser.add_option(
"-l",
"--locals",
action="store_true",
dest="locals",
help="locals = True",
default=True,
self.ov_parser.add_option(
"l",
"locals",
help = "locals = True",
value_type = lldb.eArgTypeBoolean,
dest = "locals",
default = True,
)

parser.add_option(
"-s",
"--statics",
action="store_true",
dest="statics",
help="statics = True",
default=True,
self.ov_parser.add_option(
"s",
"statics",
help = "statics = True",
value_type = lldb.eArgTypeBoolean,
dest = "statics",
default = True,
)

return parser

def get_short_help(self):
return "Example command for use in debugging"

def get_long_help(self):
return self.help_string
return ("This command is meant to be an example of how to make "
"an LLDB command that does something useful, follows "
"best practices, and exploits the SB API. "
"Specifically, this command computes the aggregate "
"and average size of the variables in the current "
"frame and allows you to tweak exactly which variables "
"are to be accounted in the computation.")


def __init__(self, debugger, unused):
self.parser = self.create_options()
self.help_string = self.parser.format_help()
super().__init__(debugger, unused)

def __call__(self, debugger, command, exe_ctx, result):
# Use the Shell Lexer to properly parse up command options just like a
# shell would
command_args = shlex.split(command)

try:
(options, args) = self.parser.parse_args(command_args)
except:
# if you don't handle exceptions, passing an incorrect argument to
# the OptionParser will cause LLDB to exit (courtesy of OptParse
# dealing with argument errors by throwing SystemExit)
result.SetError("option parsing failed")
return

# Always get program state from the lldb.SBExecutionContext passed
# in as exe_ctx
frame = exe_ctx.GetFrame()
Expand All @@ -128,7 +97,7 @@ def __call__(self, debugger, command, exe_ctx, result):
return

variables_list = frame.GetVariables(
options.arguments, options.locals, options.statics, options.inscope
self.ov_parser.arguments, self.ov_parser.locals, self.ov_parser.statics, self.ov_parser.inscope
)
variables_count = variables_list.GetSize()
if variables_count == 0:
Expand Down
Loading