Skip to content
Permalink
Browse files

[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
  • Loading branch information
Lawrence D'Anna
Lawrence D'Anna committed Oct 19, 2019
1 parent 4a5df73 commit 2386537c2469a97501a305c6b3138231b907a67f
@@ -4,7 +4,7 @@

from __future__ import print_function


import sys
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -22,6 +22,21 @@ def test(self):
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',
@@ -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()
@@ -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
@@ -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;
}
@@ -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) {
@@ -604,6 +604,11 @@ class PythonCallable : public TypedPythonObject<PythonCallable> {
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 @@ class PythonCallable : public TypedPythonObject<PythonCallable> {
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);
@@ -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

0 comments on commit 2386537

Please sign in to comment.
You can’t perform that action at this time.