[LLDB] bugfix: command script add -f doesn't work for some callables

Summary:
When users define a debugger command from python, they provide a callable
object.   Because the signature of the function has been extended, LLDB
needs to inspect the number of parameters the callable can take.

The rule it was using to decide was weird, apparently not tested, and
giving wrong results for some kinds of python callables.

This patch replaces the weird rule with a simple one: if the callable can
take 5 arguments, it gets the 5 argument version of the signature.
Otherwise it gets the old 4 argument version.

It also adds tests with a bunch of different kinds of python callables
with both 4 and 5 arguments.

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D69014

llvm-svn: 375333
This commit is contained in:
Lawrence D'Anna 2019-10-19 07:05:33 +00:00
parent 4a5df7312e
commit 2386537c24
7 changed files with 167 additions and 19 deletions

View File

@ -4,7 +4,7 @@ Test lldb Python commands.
from __future__ import print_function
import sys
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@ -22,6 +22,21 @@ class CmdPythonTestCase(TestBase):
def pycmd_tests(self):
self.runCmd("command source py_import")
# Test a bunch of different kinds of python callables with
# both 4 and 5 positional arguments.
self.expect("foobar", substrs=["All good"])
self.expect("foobar4", substrs=["All good"])
self.expect("vfoobar", substrs=["All good"])
self.expect("v5foobar", substrs=["All good"])
self.expect("sfoobar", substrs=["All good"])
self.expect("cfoobar", substrs=["All good"])
self.expect("ifoobar", substrs=["All good"])
self.expect("sfoobar4", substrs=["All good"])
self.expect("cfoobar4", substrs=["All good"])
self.expect("ifoobar4", substrs=["All good"])
self.expect("ofoobar", substrs=["All good"])
self.expect("ofoobar4", substrs=["All good"])
# Verify command that specifies eCommandRequiresTarget returns failure
# without a target.
self.expect('targetname',

View File

@ -0,0 +1,63 @@
from __future__ import print_function
import lldb
# bunch of different kinds of python callables that should
# all work as commands.
def check(debugger, command, context, result, internal_dict):
if (not isinstance(debugger, lldb.SBDebugger) or
not isinstance(command, str) or
not isinstance(result, lldb.SBCommandReturnObject) or
not isinstance(internal_dict, dict) or
(not context is None and
not isinstance(context, lldb.SBExecutionContext))):
raise Exception()
result.AppendMessage("All good.")
def vfoobar(*args):
check(*args)
def v5foobar(debugger, command, context, result, internal_dict, *args):
check(debugger, command, context, result, internal_dict)
def foobar(debugger, command, context, result, internal_dict):
check(debugger, command, context, result, internal_dict)
def foobar4(debugger, command, result, internal_dict):
check(debugger, command, None, result, internal_dict)
class FooBar:
@staticmethod
def sfoobar(debugger, command, context, result, internal_dict):
check(debugger, command, context, result, internal_dict)
@classmethod
def cfoobar(cls, debugger, command, context, result, internal_dict):
check(debugger, command, context, result, internal_dict)
def ifoobar(self, debugger, command, context, result, internal_dict):
check(debugger, command, context, result, internal_dict)
def __call__(self, debugger, command, context, result, internal_dict):
check(debugger, command, context, result, internal_dict)
@staticmethod
def sfoobar4(debugger, command, result, internal_dict):
check(debugger, command, None, result, internal_dict)
@classmethod
def cfoobar4(cls, debugger, command, result, internal_dict):
check(debugger, command, None, result, internal_dict)
def ifoobar4(self, debugger, command, result, internal_dict):
check(debugger, command, None, result, internal_dict)
class FooBar4:
def __call__(self, debugger, command, result, internal_dict):
check(debugger, command, None, result, internal_dict)
FooBarObj = FooBar()
FooBar4Obj = FooBar4()

View File

@ -11,3 +11,22 @@ command script add tell_async --function welcome.check_for_synchro --synchronici
command script add tell_curr --function welcome.check_for_synchro --synchronicity curr
command script add takes_exe_ctx --function welcome.takes_exe_ctx
command script import decorated.py
command script import callables.py
command script add -f callables.foobar foobar
command script add -f callables.foobar4 foobar4
command script add -f callables.vfoobar vfoobar
command script add -f callables.v5foobar v5foobar
command script add -f callables.FooBar.sfoobar sfoobar
command script add -f callables.FooBar.cfoobar cfoobar
command script add -f callables.FooBarObj.ifoobar ifoobar
command script add -f callables.FooBar.sfoobar4 sfoobar4
command script add -f callables.FooBar.cfoobar4 cfoobar4
command script add -f callables.FooBarObj.ifoobar4 ifoobar4
command script add -f callables.FooBarObj ofoobar
command script add -f callables.FooBar4Obj ofoobar4

View File

@ -696,15 +696,19 @@ LLDBSwigPythonCallCommand
// pass the pointer-to cmd_retobj_sb or watch the underlying object disappear from under you
// see comment above for SBCommandReturnObjectReleaser for further details
auto argc = pfunc.GetNumArguments();
auto argc = pfunc.GetArgInfo();
if (!argc) {
llvm::consumeError(argc.takeError());
return false;
}
PythonObject debugger_arg(PyRefType::Owned, SBTypeToSWIGWrapper(debugger_sb));
PythonObject exe_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(exe_ctx_sb));
PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(&cmd_retobj_sb));
if (argc.count == 5 || argc.is_bound_method || argc.has_varargs)
pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg, dict);
else
if (argc.get().max_positional_args < 5u)
pfunc(debugger_arg, PythonString(args), cmd_retobj_arg, dict);
else
pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg, dict);
return true;
}

View File

@ -876,21 +876,23 @@ Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
result.count = cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
result.has_varargs =
cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs")));
result.is_bound_method =
bool is_method =
cantFail(As<bool>(pyarginfo.get().GetAttribute("is_bound_method")));
result.max_positional_args =
result.has_varargs ? ArgInfo::UNBOUNDED : result.count;
// FIXME emulate old broken behavior
if (result.is_bound_method)
if (is_method)
result.count++;
#else
bool is_bound_method = false;
PyObject *py_func_obj = m_py_obj;
if (PyMethod_Check(py_func_obj)) {
py_func_obj = PyMethod_GET_FUNCTION(py_func_obj);
PythonObject im_self = GetAttributeValue("im_self");
if (im_self.IsValid() && !im_self.IsNone())
result.is_bound_method = true;
is_bound_method = true;
} else {
// see if this is a callable object with an __call__ method
if (!PyFunction_Check(py_func_obj)) {
@ -899,9 +901,9 @@ Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
auto __callable__ = __call__.AsType<PythonCallable>();
if (__callable__.IsValid()) {
py_func_obj = PyMethod_GET_FUNCTION(__callable__.get());
PythonObject im_self = GetAttributeValue("im_self");
PythonObject im_self = __callable__.GetAttributeValue("im_self");
if (im_self.IsValid() && !im_self.IsNone())
result.is_bound_method = true;
is_bound_method = true;
}
}
}
@ -916,12 +918,18 @@ Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
result.count = code->co_argcount;
result.has_varargs = !!(code->co_flags & CO_VARARGS);
result.max_positional_args = result.has_varargs
? ArgInfo::UNBOUNDED
: (result.count - (int)is_bound_method);
#endif
return result;
}
constexpr unsigned
PythonCallable::ArgInfo::UNBOUNDED; // FIXME delete after c++17
PythonCallable::ArgInfo PythonCallable::GetNumArguments() const {
auto arginfo = GetArgInfo();
if (!arginfo) {

View File

@ -604,6 +604,11 @@ public:
using TypedPythonObject::TypedPythonObject;
struct ArgInfo {
/* the largest number of positional arguments this callable
* can accept, or UNBOUNDED, ie UINT_MAX if it's a varargs
* function and can accept an arbitrary number */
unsigned max_positional_args;
static constexpr unsigned UNBOUNDED = UINT_MAX; // FIXME c++17 inline
/* the number of positional arguments, including optional ones,
* and excluding varargs. If this is a bound method, then the
* count will still include a +1 for self.
@ -614,8 +619,6 @@ public:
int count;
/* does the callable have positional varargs? */
bool has_varargs : 1; // FIXME delete this
/* is the callable a bound method written in python? */
bool is_bound_method : 1; // FIXME delete this
};
static bool Check(PyObject *py_obj);

View File

@ -643,8 +643,8 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
auto arginfo = lambda.GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 1);
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
EXPECT_EQ(arginfo.get().has_varargs, false);
EXPECT_EQ(arginfo.get().is_bound_method, false);
}
{
@ -655,8 +655,8 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
auto arginfo = lambda.GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 2);
EXPECT_EQ(arginfo.get().max_positional_args, 2u);
EXPECT_EQ(arginfo.get().has_varargs, false);
EXPECT_EQ(arginfo.get().is_bound_method, false);
}
{
@ -667,6 +667,7 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
auto arginfo = lambda.GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 2);
EXPECT_EQ(arginfo.get().max_positional_args, 2u);
EXPECT_EQ(arginfo.get().has_varargs, false);
}
@ -678,8 +679,9 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
auto arginfo = lambda.GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 2);
EXPECT_EQ(arginfo.get().max_positional_args,
PythonCallable::ArgInfo::UNBOUNDED);
EXPECT_EQ(arginfo.get().has_varargs, true);
EXPECT_EQ(arginfo.get().is_bound_method, false);
}
{
@ -690,6 +692,8 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
auto arginfo = lambda.GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 2);
EXPECT_EQ(arginfo.get().max_positional_args,
PythonCallable::ArgInfo::UNBOUNDED);
EXPECT_EQ(arginfo.get().has_varargs, true);
}
@ -698,7 +702,18 @@ TEST_F(PythonDataObjectsTest, TestCallable) {
class Foo:
def bar(self, x):
return x
@classmethod
def classbar(cls, x):
return x
@staticmethod
def staticbar(x):
return x
def __call__(self, x):
return x
obj = Foo()
bar_bound = Foo().bar
bar_class = Foo().classbar
bar_static = Foo().staticbar
bar_unbound = Foo.bar
)";
PyObject *o =
@ -711,16 +726,37 @@ bar_unbound = Foo.bar
auto arginfo = bar_bound.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 2); // FIXME, wrong
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
EXPECT_EQ(arginfo.get().has_varargs, false);
EXPECT_EQ(arginfo.get().is_bound_method, true);
auto bar_unbound = As<PythonCallable>(globals.GetItem("bar_unbound"));
ASSERT_THAT_EXPECTED(bar_unbound, llvm::Succeeded());
arginfo = bar_unbound.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 2);
EXPECT_EQ(arginfo.get().max_positional_args, 2u);
EXPECT_EQ(arginfo.get().has_varargs, false);
auto bar_class = As<PythonCallable>(globals.GetItem("bar_class"));
ASSERT_THAT_EXPECTED(bar_class, llvm::Succeeded());
arginfo = bar_class.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
EXPECT_EQ(arginfo.get().has_varargs, false);
auto bar_static = As<PythonCallable>(globals.GetItem("bar_static"));
ASSERT_THAT_EXPECTED(bar_static, llvm::Succeeded());
arginfo = bar_static.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
EXPECT_EQ(arginfo.get().has_varargs, false);
auto obj = As<PythonCallable>(globals.GetItem("obj"));
ASSERT_THAT_EXPECTED(obj, llvm::Succeeded());
arginfo = obj.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
EXPECT_EQ(arginfo.get().has_varargs, false);
EXPECT_EQ(arginfo.get().is_bound_method, false);
}
#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
@ -734,8 +770,8 @@ bar_unbound = Foo.bar
auto arginfo = hex.get().GetArgInfo();
ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
EXPECT_EQ(arginfo.get().count, 1);
EXPECT_EQ(arginfo.get().max_positional_args, 1u);
EXPECT_EQ(arginfo.get().has_varargs, false);
EXPECT_EQ(arginfo.get().is_bound_method, false);
}
#endif