https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/213378
>From 88908c2463ba1961c1a252287e559d59202fb4bf Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Fri, 31 Jul 2026 18:18:38 -0700 Subject: [PATCH 1/2] Revert "Disable this test on Darwin to give the author a chance to fix it. (#213388)" This reverts commit 956e24199792651f33b1cad39f611d5f03ba27b5. --- .../test/API/commands/register/register_command/TestRegisters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/test/API/commands/register/register_command/TestRegisters.py b/lldb/test/API/commands/register/register_command/TestRegisters.py index d476728f33381..5a2ac7b0ebbf3 100644 --- a/lldb/test/API/commands/register/register_command/TestRegisters.py +++ b/lldb/test/API/commands/register/register_command/TestRegisters.py @@ -717,7 +717,6 @@ def test_process_must_be_stopped(self): 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" >From c83963b6634f1556c86f01659f11458de49434cf Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Sat, 1 Aug 2026 14:38:37 -0700 Subject: [PATCH 2/2] [lldb] Reimplement PythonCallable::GetArgInfo without executing Python code `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. Signed-off-by: Med Ismail Bennani <[email protected]> --- .../Python/PythonDataObjects.cpp | 91 +++++++++++++++++-- .../Python/PythonDataObjects.h | 9 ++ .../Python/ScriptInterpreterPython.cpp | 8 ++ .../Python/PythonDataObjectsTests.cpp | 37 +++++++- 4 files changed, 137 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index fba98abf9e83b..000c1578036d9 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -840,22 +840,99 @@ 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; +} + +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; } diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h index 3f2b869bcfb0a..7b29d002fa503 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -615,6 +615,15 @@ class PythonCallable : public TypedPythonObject<PythonCallable> { 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); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 010a0ad015c12..8abdf41cb112e 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/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; diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp index 24ed721049e67..b46d672656f21 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ b/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); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
