[Lldb-commits] [lldb] [lldb] Validate a scripted class abstract methods before running its constructor (PR #225246)
Med Ismail Bennani via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 22 18:18:05 PDT 2026
https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/225246
>From c232dd3a7c214795c9d0cc37de916f24d024c34b Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Tue, 22 Sep 2026 18:17:44 -0700
Subject: [PATCH] [lldb] Validate a scripted class abstract methods before
running its constructor
`CreatePluginObject` called the class's `__init__` and only then checked
that the class implements the methods the interface requires. So a class
we were always going to reject had already executed arbitrary user code,
side effects included, and if `__init__` happened to raise as well, the
constructor's exception masked the real, structural problem.
Lift the abstract-method diagnosis into `CheckAbstractMethods` and run it
against the class object before calling it. The check already operated on
a class rather than an instance - it just happened to reach it through
`instance.__class__` - so nothing about it needed to change. The
pre-existing-object path (`script_obj`) has no constructor to guard, so it
keeps validating after the fact, gated on a flag so the check runs once.
`ExceptionInitScriptedBreakpointResolver` in the test fixtures claimed to
test a raising `__init__` but also omitted `__callback__`, so it was only
reporting the init exception because construction used to come first. It
now implements `__callback__`, leaving `__init__` as the single thing wrong
with it, which is what the test means to exercise.
Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
.../Interfaces/ScriptedPythonInterface.h | 180 ++++++++++--------
.../malformed_scripted_extensions.py | 8 +-
2 files changed, 112 insertions(+), 76 deletions(-)
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
index e6668de66f889c..eb74e8caea40e7 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h
@@ -169,6 +169,102 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
return checker;
}
+ /// Diagnose every abstract-method violation on \a obj_class at once.
+ ///
+ /// \a obj_class is a class object, whether resolved by name or taken from an
+ /// instance's `__class__`. Resolving it by name means this can run before any
+ /// instance exists (see CreatePluginObject), which uses it to reject a
+ /// malformed class without executing its `__init__`.
+ llvm::Error CheckAbstractMethods(const python::PythonObject &obj_class,
+ llvm::StringRef qualified_class_name) const {
+ Log *log = GetLog(LLDBLog::Script);
+
+ // Per-method diagnostics name the class the way Python does, unqualified.
+ python::PythonString obj_class_name =
+ obj_class.GetAttributeValue("__name__").AsType<python::PythonString>();
+ llvm::StringRef class_name = obj_class_name.IsValid()
+ ? obj_class_name.GetString()
+ : qualified_class_name;
+ auto create_error = [](llvm::StringLiteral format, auto &&...ts) {
+ return llvm::createStringError(
+ llvm::formatv(format.data(), std::forward<decltype(ts)>(ts)...)
+ .str());
+ };
+
+ auto checker_or_err = CheckAbstractMethodImplementation(obj_class);
+ if (!checker_or_err)
+ return checker_or_err.takeError();
+
+ llvm::Error abstract_method_errors = llvm::Error::success();
+ for (const auto &method_checker : *checker_or_err)
+ switch (method_checker.second.checker_case) {
+ case AbstractMethodCheckerCases::eNotImplemented:
+ abstract_method_errors = llvm::joinErrors(
+ std::move(abstract_method_errors),
+ create_error("abstract method {0}.{1} not implemented", class_name,
+ method_checker.first));
+ break;
+ case AbstractMethodCheckerCases::eNotAllocated:
+ abstract_method_errors = llvm::joinErrors(
+ std::move(abstract_method_errors),
+ create_error("abstract method {0}.{1} not allocated", class_name,
+ method_checker.first));
+ break;
+ case AbstractMethodCheckerCases::eNotCallable:
+ abstract_method_errors = llvm::joinErrors(
+ std::move(abstract_method_errors),
+ create_error("abstract method {0}.{1} not callable", class_name,
+ method_checker.first));
+ break;
+ case AbstractMethodCheckerCases::eUnknownArgumentCount: {
+ const std::string *py_error =
+ std::get_if<std::string>(&method_checker.second.payload);
+ abstract_method_errors = llvm::joinErrors(
+ std::move(abstract_method_errors),
+ create_error(
+ "abstract method {0}.{1} has unknown argument count: {2}",
+ class_name, method_checker.first,
+ py_error ? *py_error : "<no further information>"));
+ } break;
+ case AbstractMethodCheckerCases::eInvalidArgumentCount: {
+ auto &payload_variant = method_checker.second.payload;
+ if (!std::holds_alternative<
+ AbstractMethodCheckerPayload::InvalidArgumentCountPayload>(
+ payload_variant)) {
+ abstract_method_errors = llvm::joinErrors(
+ std::move(abstract_method_errors),
+ create_error(
+ "abstract method {0}.{1} has unexpected argument count",
+ class_name, method_checker.first));
+ } else {
+ auto payload = std::get<
+ AbstractMethodCheckerPayload::InvalidArgumentCountPayload>(
+ payload_variant);
+ abstract_method_errors = llvm::joinErrors(
+ std::move(abstract_method_errors),
+ create_error("abstract method {0}.{1} has unexpected "
+ "argument count (expected {2} but has {3})",
+ class_name, method_checker.first,
+ payload.required_argument_count,
+ payload.actual_argument_count));
+ }
+ } break;
+ case AbstractMethodCheckerCases::eValid:
+ LLDB_LOG(log, "Abstract method {0}.{1} implemented & valid.",
+ class_name, method_checker.first);
+ break;
+ }
+
+ if (abstract_method_errors) {
+ Status error = Status::FromError(std::move(abstract_method_errors));
+ LLDB_LOG(log, "Abstract method error in {0}:\n{1}", qualified_class_name,
+ error.AsCString());
+ return error.ToError();
+ }
+
+ return llvm::Error::success();
+ }
+
template <typename... Args>
llvm::Expected<StructuredData::GenericSP>
CreatePluginObject(const ScriptedMetadata &scripted_metadata,
@@ -176,7 +272,6 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
using namespace python;
using Locker = ScriptInterpreterPythonImpl::Locker;
- Log *log = GetLog(LLDBLog::Script);
auto create_error = [](llvm::StringLiteral format, auto &&...ts) {
return llvm::createStringError(
llvm::formatv(format.data(), std::forward<decltype(ts)>(ts)...)
@@ -240,6 +335,9 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
error_string);
}
+ if (llvm::Error error = CheckAbstractMethods(init, class_name))
+ return std::move(error);
+
llvm::Expected<PythonObject> expected_return_object =
create_error("resulting object is not initialized");
@@ -309,80 +407,12 @@ class ScriptedPythonInterface : virtual public ScriptedInterface {
PythonString obj_class_name =
obj_class.GetAttributeValue("__name__").AsType<PythonString>();
- auto checker_or_err = CheckAbstractMethodImplementation(obj_class);
- if (!checker_or_err)
- return checker_or_err.takeError();
-
- llvm::Error abstract_method_errors = llvm::Error::success();
- for (const auto &method_checker : *checker_or_err)
- switch (method_checker.second.checker_case) {
- case AbstractMethodCheckerCases::eNotImplemented:
- abstract_method_errors = llvm::joinErrors(
- std::move(abstract_method_errors),
- std::move(create_error("abstract method {0}.{1} not implemented",
- obj_class_name.GetString(),
- method_checker.first)));
- break;
- case AbstractMethodCheckerCases::eNotAllocated:
- abstract_method_errors = llvm::joinErrors(
- std::move(abstract_method_errors),
- std::move(create_error("abstract method {0}.{1} not allocated",
- obj_class_name.GetString(),
- method_checker.first)));
- break;
- case AbstractMethodCheckerCases::eNotCallable:
- abstract_method_errors = llvm::joinErrors(
- std::move(abstract_method_errors),
- std::move(create_error("abstract method {0}.{1} not callable",
- obj_class_name.GetString(),
- method_checker.first)));
- break;
- case AbstractMethodCheckerCases::eUnknownArgumentCount: {
- const std::string *py_error =
- std::get_if<std::string>(&method_checker.second.payload);
- abstract_method_errors = llvm::joinErrors(
- std::move(abstract_method_errors),
- std::move(create_error(
- "abstract method {0}.{1} has unknown argument count: {2}",
- obj_class_name.GetString(), method_checker.first,
- py_error ? *py_error : "<no further information>")));
- } break;
- case AbstractMethodCheckerCases::eInvalidArgumentCount: {
- auto &payload_variant = method_checker.second.payload;
- if (!std::holds_alternative<
- AbstractMethodCheckerPayload::InvalidArgumentCountPayload>(
- payload_variant)) {
- abstract_method_errors = llvm::joinErrors(
- std::move(abstract_method_errors),
- std::move(create_error(
- "abstract method {0}.{1} has unexpected argument count",
- obj_class_name.GetString(), method_checker.first)));
- } else {
- auto payload = std::get<
- AbstractMethodCheckerPayload::InvalidArgumentCountPayload>(
- payload_variant);
- abstract_method_errors = llvm::joinErrors(
- std::move(abstract_method_errors),
- std::move(
- create_error("abstract method {0}.{1} has unexpected "
- "argument count (expected {2} but has {3})",
- obj_class_name.GetString(), method_checker.first,
- payload.required_argument_count,
- payload.actual_argument_count)));
- }
- } break;
- case AbstractMethodCheckerCases::eValid:
- LLDB_LOG(log, "Abstract method {0}.{1} implemented & valid.",
- obj_class_name.GetString(), method_checker.first);
- break;
- }
-
- if (abstract_method_errors) {
- Status error = Status::FromError(std::move(abstract_method_errors));
- LLDB_LOG(log, "Abstract method error in {0}:\n{1}", class_name,
- error.AsCString());
- return error.ToError();
- }
+ // We were handed an instance rather than building one, so there was no
+ // constructor to run the check ahead of; validate it now.
+ if (script_obj)
+ if (llvm::Error error =
+ CheckAbstractMethods(obj_class, obj_class_name.GetString()))
+ return std::move(error);
m_object_instance_sp = StructuredData::GenericSP(
new StructuredPythonObject(std::move(result)));
diff --git a/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py b/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py
index d980e46792733a..661c9456ee48ac 100644
--- a/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py
+++ b/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py
@@ -267,11 +267,17 @@ def get_short_help(self):
class ExceptionInitScriptedBreakpointResolver:
- """`__init__` raises."""
+ """`__init__` raises. Everything else is implemented, so `__init__` is the
+ only thing wrong with this class: LLDB validates the class before it
+ constructs an instance, and a missing abstract method would otherwise be
+ reported first."""
def __init__(self, bkpt, args):
raise RuntimeError("intentional exception from __init__()")
+ def __callback__(self, sym_ctx):
+ return False
+
# ---------------------------------------------------------------------------
# Scripted Stop Hook
More information about the lldb-commits
mailing list