[lldb] Reimplement PythonCallable::GetArgInfo without executing Python code (#213378)

`b05a5d0a` added an arity-trimming step to the shared
`ScriptedPythonInterface::Dispatch`: extensions are now allowed to
define methods with trailing parameters as optional
(`num_children(self)` vs. `num_children(self, max_count)`), so before
calling into a method, `Dispatch` needs to know how many positional
arguments it actually accepts and drop any trailing ones we'd otherwise
pass.

That check calls `PythonCallable::GetArgInfo`, which ran a whole
embedded Python script through `inspect.signature` on every call, since
every scripted-extension dispatch goes through it.

For the common case `GetArgInfo()` actually needs to handle fast (plain
Python functions/methods, classes used as constructors, and callable
instances defining `__call__`, i.e. everything `Dispatch<T>()` and
`CreatePluginObject()` ever pass it), the answer is available as plain
data attributes, with no Python bytecode execution required:
`__func__`/`__self__` to unwrap bound methods, and `__code__`'s
`co_argcount`/`co_flags` for the actual positional-argument count and
varargs bit. For a class, that means `__init__` -- except a class may
customize `__new__` instead and leave `__init__` untouched, in which
case `object.__init__` becomes lenient about extra arguments.

Anything else still lacking `__code__` can fall back to the original
`inspect.signature()`-based implementation, now exposed as
`PythonCallable::GetArgInfoFromInspectSignature()` so it can be used
separately.

rdar://183776556

---------

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp

@@ -840,22 +840,116 @@ def main(f):
     return ArgInfo(count, varargs)
 )";
 
-Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
-  ArgInfo result = {};
-  if (!IsValid())
-    return nullDeref();
-
+// inspect.signature() is deeply recursive and expensive in C-stack terms;
+// reentrant scripted callbacks dispatched through GetArgInfo() can turn
+// that into a fatal stack overflow instead of a catchable Python
+// RecursionError. GetArgInfo() never calls this itself; callers fall back
+// to it explicitly when they need to handle callables its cheaper,
+// attribute-only approach can't (e.g. builtins).
+Expected<PythonCallable::ArgInfo>
+PythonCallable::GetArgInfoFromInspectSignature(const PythonCallable &callable) {
+  PythonCallable::ArgInfo result = {};
   // no need to synchronize access to this global, we already have the GIL
   static PythonScript get_arg_info(get_arg_info_script);
-  Expected<PythonObject> pyarginfo = get_arg_info(*this);
+  Expected<PythonObject> pyarginfo = get_arg_info(callable);
   if (!pyarginfo)
     return pyarginfo.takeError();
   long long count =
       cantFail(As<long long>(pyarginfo.get().GetAttribute("count")));
   bool has_varargs =
       cantFail(As<bool>(pyarginfo.get().GetAttribute("has_varargs")));
-  result.max_positional_args = has_varargs ? ArgInfo::UNBOUNDED : count;
+  result.max_positional_args =
+      has_varargs ? PythonCallable::ArgInfo::UNBOUNDED : count;
+  return result;
+}
 
+// GetArgInfo()'s branches, top to bottom (`func` is what each branch ends up
+// introspecting; the final step is always func.__code__.co_argcount/co_flags):
+//
+//   callable
+//   |-- has __self__          -> func = __func__           (bound method;
+//   |                             fails for slot wrappers, e.g. (1).__add__)
+//   |-- has __code__ already  -> func = callable            (plain function)
+//   `-- neither
+//       |-- is a class
+//       |   |-- __init__ has __code__ -> func = __init__
+//       |   `-- else: check __new__ too (object.__init__ is lenient about
+//       |       extra args once __new__ is overridden)
+//       |       |-- __new__ has __code__ -> func = __new__
+//       |       `-- else                  -> ArgInfo{0} (object's defaults)
+//       `-- is an instance
+//           `-- func = __call__ (unwrap __func__ if bound)
+//               `-- no __code__ -> error (e.g. a builtin)
+Expected<PythonCallable::ArgInfo> PythonCallable::GetArgInfo() const {
+  if (!IsValid())
+    return nullDeref();
+
+  PythonObject func = *this;
+  bool implicit_first_arg = false;
+  if (HasAttribute("__self__")) {
+    implicit_first_arg = true;
+    Expected<PythonObject> func_or_err = GetAttribute("__func__");
+    if (!func_or_err)
+      return func_or_err.takeError();
+    func = *func_or_err;
+  } else if (!HasAttribute("__code__")) {
+    implicit_first_arg = true;
+    if (PyType_Check(m_py_obj)) {
+      Expected<PythonObject> init_or_err = GetAttribute("__init__");
+      if (!init_or_err)
+        return init_or_err.takeError();
+      func = *init_or_err;
+      if (!func.HasAttribute("__code__")) {
+        // __init__ is still object.__init__. A class may customize
+        // __new__ instead and leave __init__ untouched, which makes
+        // object.__init__ lenient about extra arguments -- so check
+        // __new__ too before concluding there are none.
+        Expected<PythonObject> new_or_err = GetAttribute("__new__");
+        if (!new_or_err)
+          return new_or_err.takeError();
+        func = *new_or_err;
+        if (!func.HasAttribute("__code__"))
+          return ArgInfo{0};
+      }
+    } else {
+      Expected<PythonObject> call_or_err = GetAttribute("__call__");
+      if (!call_or_err)
+        return call_or_err.takeError();
+      func = *call_or_err;
+      if (func.HasAttribute("__self__")) {
+        Expected<PythonObject> inner_or_err = func.GetAttribute("__func__");
+        if (!inner_or_err)
+          return inner_or_err.takeError();
+        func = *inner_or_err;
+      }
+      if (!func.HasAttribute("__code__"))
+        return llvm::createStringError("__call__ has no __code__");
+    }
+  }
+
+  Expected<PythonObject> code_or_err = func.GetAttribute("__code__");
+  if (!code_or_err)
+    return code_or_err.takeError();
+  PythonObject code = *code_or_err;
+
+  Expected<long long> argcount =
+      As<long long>(code.GetAttribute("co_argcount"));
+  if (!argcount)
+    return argcount.takeError();
+  Expected<long long> flags = As<long long>(code.GetAttribute("co_flags"));
+  if (!flags)
+    return flags.takeError();
+
+  ArgInfo result = {};
+  // Mirrors CPython's CO_VARARGS from <code.h>, which isn't reliably
+  // visible across the Python versions/platforms this file builds against.
+  constexpr long long kCoFlagVarArgs = 0x04;
+  if (*flags & kCoFlagVarArgs) {
+    result.max_positional_args = ArgInfo::UNBOUNDED;
+  } else {
+    long long count = *argcount - (implicit_first_arg ? 1 : 0);
+    result.max_positional_args = count > 0 ? static_cast<unsigned>(count) : 0;
+  }
   return result;
 }
 

lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h

@@ -615,6 +615,15 @@ public:
 
   llvm::Expected<ArgInfo> GetArgInfo() const;
 
+  // Always derives ArgInfo via a Python-level inspect.signature() call,
+  // regardless of whether the callable's argument count/varargs bit could
+  // have been read directly off its data attributes. GetArgInfo() prefers
+  // the cheaper attribute-based path and only falls back to this for
+  // callables it can't introspect that way (e.g. builtins); exposed
+  // separately so that fallback behavior can be tested directly.
+  static llvm::Expected<ArgInfo>
+  GetArgInfoFromInspectSignature(const PythonCallable &callable);
+
   PythonObject operator()();
 
   PythonObject operator()(std::initializer_list<PyObject *> args);

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

@@ -1300,6 +1300,14 @@ ScriptInterpreterPythonImpl::GetMaxPositionalArgumentsForCallable(
                                    callable_name.str().c_str());
   }
   llvm::Expected<PythonCallable::ArgInfo> arg_info = pfunc.GetArgInfo();
+  if (!arg_info) {
+    // `-f` may point at a builtin, unlike other GetArgInfo() callers.
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Script), arg_info.takeError(),
+                   "GetArgInfo failed for callable {1}, falling back to "
+                   "inspect.signature: {0}",
+                   callable_name);
+    arg_info = PythonCallable::GetArgInfoFromInspectSignature(pfunc);
+  }
   if (!arg_info)
     return arg_info.takeError();
   return arg_info.get().max_positional_args;

lldb/test/API/commands/register/register_command/TestRegisters.py

@@ -717,7 +717,6 @@ class RegisterCommandsTestCase(TestBase):
         self.expect("register write pc 0", substrs=[err_msg], error=True)
         self.expect("register info pc", substrs=[err_msg], error=True)
 
-    @expectedFailureDarwin(bugnumber="github.com/llvm/llvm-project/issues/213386")
     def test_case_insensitivity(self):
         """
         Register names, their aliases and any generic names like "sp" and "ra"

lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp

@@ -735,6 +735,17 @@ class NewStyle(object):
   def __init__(self, one, two, three):
     pass
 
+class NoConstructorAtAll:
+  pass
+
+class NewOnlyVarArgs:
+  def __new__(cls, *args, **kwargs):
+    return super().__new__(cls)
+
+class NewOnlyFixedArgs:
+  def __new__(cls, a, b):
+    return super().__new__(cls)
+
 )";
     PyObject *o =
         RunString(script, Py_file_input, globals.get(), globals.get());
@@ -782,13 +793,37 @@ class NewStyle(object):
     arginfo = newstyle.get().GetArgInfo();
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().max_positional_args, 3u);
+
+    // Neither __init__ nor __new__ overridden: object.__init__ truly takes
+    // no extra arguments here.
+    auto no_ctor = As<PythonCallable>(globals.GetItem("NoConstructorAtAll"));
+    ASSERT_THAT_EXPECTED(no_ctor, llvm::Succeeded());
+    arginfo = no_ctor.get().GetArgInfo();
+    ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+    EXPECT_EQ(arginfo.get().max_positional_args, 0u);
+
+    // __init__ not overridden, but __new__ is: object.__init__ becomes
+    // lenient about extra arguments once __new__ is overridden, so the
+    // argument count has to come from __new__, not from __init__ alone.
+    auto new_varargs = As<PythonCallable>(globals.GetItem("NewOnlyVarArgs"));
+    ASSERT_THAT_EXPECTED(new_varargs, llvm::Succeeded());
+    arginfo = new_varargs.get().GetArgInfo();
+    ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+    EXPECT_EQ(arginfo.get().max_positional_args,
+              PythonCallable::ArgInfo::UNBOUNDED);
+
+    auto new_fixed = As<PythonCallable>(globals.GetItem("NewOnlyFixedArgs"));
+    ASSERT_THAT_EXPECTED(new_fixed, llvm::Succeeded());
+    arginfo = new_fixed.get().GetArgInfo();
+    ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
+    EXPECT_EQ(arginfo.get().max_positional_args, 2u);
   }
 
   {
     auto builtins = PythonModule::BuiltinsModule();
     auto hex = As<PythonCallable>(builtins.GetAttribute("hex"));
     ASSERT_THAT_EXPECTED(hex, llvm::Succeeded());
-    auto arginfo = hex.get().GetArgInfo();
+    auto arginfo = PythonCallable::GetArgInfoFromInspectSignature(hex.get());
     ASSERT_THAT_EXPECTED(arginfo, llvm::Succeeded());
     EXPECT_EQ(arginfo.get().max_positional_args, 1u);
   }