Author: Lawrence D'Anna
Date: 2019-10-19T07:05:33Z
New Revision: 2386537c2469a97501a305c6b3138231b907a67f
URL:
https://github.com/llvm/llvm-project/commit/2386537c2469a97501a305c6b3138231b907a67f
DIFF:
https://github.com/llvm/llvm-project/commit/2386537c2469a97501a305c6b3138231b907a67f.diff
LOG: [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
Added:
lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
Modified:
lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
lldb/scripts/Python/python-wrapper.swig
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
Removed:
################################################################################
diff --git
a/lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
b/lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
index 6531cd672792..9542d0264a6b 100644
---
a/lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
+++
b/lldb/packages/Python/lldbsuite/test/commands/command/script/TestCommandScript.py
@@ -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
diff erent 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',
diff --git
a/lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
b/lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
new file mode 100644
index 000000000000..21e599b82e5b
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/commands/command/script/callables.py
@@ -0,0 +1,63 @@
+
+from __future__ import print_function
+
+import lldb
+
+# bunch of
diff erent 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()
\ No newline at end of file
diff --git
a/lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
b/lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
index 6c1f7b8185f6..4372d32b0ad1 100644
--- a/lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
+++ b/lldb/packages/Python/lldbsuite/test/commands/command/script/py_import
@@ -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
diff --git a/lldb/scripts/Python/python-wrapper.swig
b/lldb/scripts/Python/python-wrapper.swig
index 7d507b31c5cd..c50f9d1fab92 100644
--- a/lldb/scripts/Python/python-wrapper.swig
+++ b/lldb/scripts/Python/python-wrapper.swig
@@ -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;
}
diff --git
a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
index 1fb9355b9ee3..2b70762e368f 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
@@ -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) {
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
index 5823f740a530..0cdb63f17c99 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
@@ -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);
diff --git
a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
index d9e4435bf93e..e51c6f0eb0c8 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
+++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp
@@ -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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
