https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/198153
>From aeeb0238f8b658671ae1d32b61c21467c1b4fd0c Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Tue, 30 Jun 2026 23:00:14 -0700 Subject: [PATCH] [lldb/Interpreter] Surface Python exceptions from scripted extensions Python exceptions in scripted affordance methods were silently consumed by llvm::consumeError, leaving users with a generic "Failed to create script object." or just a null result. Dispatch and CallStaticMethod now extract the Python backtrace via PythonException::ReadBacktrace and populate the Status out-parameter with "Python exception in <script-class> method '<method>':\n<traceback>" (class name pulled from ScriptedInterface's metadata accessor, or "<unknown>" if unset). ErrorWithMessage no longer overwrites a Status that already carries a detailed message. For entry points with no error-return channel - ScriptedProcess and OperatingSystemPython CreateInstance/ctor, ScriptedThreadPlan::DidPush, BreakpointResolverScripted ctor - the detailed error is broadcast via Debugger::ReportError. ScriptedThread::Create propagates it through its existing llvm::Expected return via llvm::toString instead of a generic message. ScriptedProcess::CreateInstance returns nullptr quietly when the launch info doesn't request a scripted process, so the diagnostic only fires on real failures. ScriptedProcess::DoAttach / DoReadMemory / DoWriteMemory wrap their affordance Status with the failing operation. ScriptedInterface gains an optional ErrorCallback for command handlers that want to forward errors directly to a CommandReturnObject; if unset, Status / ReportError remain the surfacing path. ScriptedBreakpointInterface, ScriptedFrameProviderInterface and ScriptedStopHookInterface now inherit virtually from ScriptedInterface to disambiguate the metadata subobject in their *PythonInterface subclasses. A new test under lldb/test/API/functionalities/scripted_extensions/ listens for eBroadcastBitError and asserts each malformed extension produces a user-visible error. Signed-off-by: Med Ismail Bennani <[email protected]> --- .../Interfaces/ScriptedBreakpointInterface.h | 2 +- .../ScriptedFrameProviderInterface.h | 2 +- .../Interfaces/ScriptedInterface.h | 35 +++- .../Interfaces/ScriptedStopHookInterface.h | 2 +- .../lldb/Interpreter/ScriptInterpreter.h | 4 + .../Breakpoint/BreakpointResolverScripted.cpp | 8 +- .../Python/OperatingSystemPython.cpp | 8 +- .../Process/scripted/ScriptedProcess.cpp | 60 +++++- .../Process/scripted/ScriptedProcess.h | 14 +- .../Process/scripted/ScriptedThread.cpp | 7 +- .../Interfaces/ScriptedPythonInterface.h | 132 +++++++++++- lldb/source/Target/ScriptedThreadPlan.cpp | 5 + .../scripted_extensions/Makefile | 3 + .../TestScriptedExtensionsDiagnostics.py | 124 +++++++++++ .../scripted_extensions/main.c | 1 + .../malformed_scripted_extensions.py | 195 ++++++++++++++++++ 16 files changed, 566 insertions(+), 36 deletions(-) create mode 100644 lldb/test/API/functionalities/scripted_extensions/Makefile create mode 100644 lldb/test/API/functionalities/scripted_extensions/TestScriptedExtensionsDiagnostics.py create mode 100644 lldb/test/API/functionalities/scripted_extensions/main.c create mode 100644 lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h index 982f231d9f0b2..7328aa26db65d 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedBreakpointInterface.h @@ -15,7 +15,7 @@ #include "lldb/lldb-private.h" namespace lldb_private { -class ScriptedBreakpointInterface : public ScriptedInterface { +class ScriptedBreakpointInterface : virtual public ScriptedInterface { public: virtual llvm::Expected<StructuredData::GenericSP> CreatePluginObject(const ScriptedMetadata &scripted_metadata, diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h index 757a99cb2387b..8c94df430c103 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h @@ -14,7 +14,7 @@ #include "ScriptedInterface.h" namespace lldb_private { -class ScriptedFrameProviderInterface : public ScriptedInterface { +class ScriptedFrameProviderInterface : virtual public ScriptedInterface { public: virtual bool AppliesToThread(llvm::StringRef class_name, lldb::ThreadSP thread_sp) { diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h index 3dbc009a58311..4fc1e5860702b 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h @@ -20,6 +20,7 @@ #include "llvm/Support/Compiler.h" +#include <functional> #include <optional> #include <string> @@ -37,6 +38,17 @@ class ScriptedInterface { return m_scripted_metadata; } + /// Set error callback to surface Python exceptions directly to users. + /// + /// This allows command handlers to receive Python exception details + /// immediately rather than relying on diagnostic broadcasts. + /// + /// \param callback Function to call with Status containing exception details. + virtual void SetErrorCallback(std::function<void(const Status &)> callback) {} + + /// Clear the error callback. + virtual void ClearErrorCallback() {} + struct AbstractMethodRequirement { llvm::StringLiteral name; size_t min_arg_count = 0; @@ -62,18 +74,20 @@ class ScriptedInterface { static Ret ErrorWithMessage(llvm::StringRef caller_name, llvm::StringRef error_msg, Status &error, LLDBLog log_category = LLDBLog::Process) { + // Log the error for debugging (includes function signature for context). LLDB_LOGF(GetLog(log_category), "%s ERROR = %s", caller_name.data(), error_msg.data()); - std::string full_error_message = - llvm::Twine(caller_name + llvm::Twine(" ERROR = ") + - llvm::Twine(error_msg)) - .str(); - if (const char *detailed_error = error.AsCString()) - full_error_message += - llvm::Twine(llvm::Twine(" (") + llvm::Twine(detailed_error) + - llvm::Twine(")")) - .str(); - error = Status(std::move(full_error_message)); + + // For user-facing messages, just pass through the Status if it already + // has detailed information (like Python tracebacks); otherwise set it. + llvm::StringRef existing_error = error.AsCString(); + if (!error.Fail() || existing_error.empty()) { + // Status is empty, populate it with the simple error message. + error = Status::FromErrorString(error_msg.data()); + } + // If Status already has content, leave it as-is (it has the Python + // traceback). + return {}; } @@ -105,4 +119,5 @@ class ScriptedInterface { std::optional<ScriptedMetadata> m_scripted_metadata; }; } // namespace lldb_private + #endif // LLDB_INTERPRETER_INTERFACES_SCRIPTEDINTERFACE_H diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedStopHookInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedStopHookInterface.h index c68498cba1632..ba325cfdf38b2 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedStopHookInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedStopHookInterface.h @@ -14,7 +14,7 @@ #include "ScriptedInterface.h" namespace lldb_private { -class ScriptedStopHookInterface : public ScriptedInterface { +class ScriptedStopHookInterface : virtual public ScriptedInterface { public: virtual llvm::Expected<StructuredData::GenericSP> CreatePluginObject(const ScriptedMetadata &scripted_metadata, diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h index 58af82fb48390..2cc3890d86916 100644 --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -663,6 +663,10 @@ class ScriptInterpreter : public PluginInterface { lldb::TargetSP GetOpaqueTypeFromSBTarget(const lldb::SBTarget &target) const; + /// Get the debugger associated with this script interpreter. + Debugger &GetDebugger() { return m_debugger; } + const Debugger &GetDebugger() const { return m_debugger; } + protected: Debugger &m_debugger; lldb::ScriptLanguage m_script_lang; diff --git a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp index 0719c8b634cd3..0fdb185537c9a 100644 --- a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp +++ b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp @@ -20,6 +20,7 @@ #include "lldb/Target/Target.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" +#include "llvm/Support/FormatVariadic.h" using namespace lldb; using namespace lldb_private; @@ -94,7 +95,12 @@ void BreakpointResolverScripted::CreateImplementationIfNeeded( m_interface_sp->CreatePluginObject(scripted_metadata, breakpoint_sp); if (!obj_or_err) { m_interface_sp.reset(); - m_error = Status::FromError(obj_or_err.takeError()); + std::string msg = llvm::toString(obj_or_err.takeError()); + Debugger::ReportError( + llvm::formatv("Failed to create BreakpointResolverScripted: {0}", msg) + .str(), + target.GetDebugger().GetID()); + m_error = Status(msg); return; } StructuredData::ObjectSP object_sp = *obj_or_err; diff --git a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp index 49df19906929f..662b8117041bc 100644 --- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp +++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp @@ -33,6 +33,7 @@ #include "lldb/Utility/StreamString.h" #include "lldb/Utility/StructuredData.h" #include "lldb/ValueObject/ValueObjectVariable.h" +#include "llvm/Support/FormatVariadic.h" #include <memory> @@ -120,7 +121,12 @@ OperatingSystemPython::OperatingSystemPython(lldb_private::Process *process, scripted_metadata, exe_ctx, nullptr); if (!obj_or_err) { - llvm::consumeError(obj_or_err.takeError()); + std::string msg = llvm::toString(obj_or_err.takeError()); + if (process) + Debugger::ReportError( + llvm::formatv("Failed to create OperatingSystemPython: {0}", msg) + .str(), + process->GetTarget().GetDebugger().GetID()); return; } diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp index 842eef9524328..17a2d376b6bc0 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp @@ -27,6 +27,10 @@ #include "Plugins/ObjectFile/Placeholder/ObjectFilePlaceholder.h" +#include "llvm/Support/Error.h" + +#include <string> + using namespace lldb; using namespace lldb_private; @@ -57,11 +61,27 @@ lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp, ScriptedMetadata scripted_metadata(target_sp->GetProcessLaunchInfo()); + // CreateInstance is invoked for every process plugin during process + // creation; if the launch info doesn't request a scripted process, bail + // out silently rather than treating the missing class name as an error. + if (!scripted_metadata) + return nullptr; + Status error; auto process_sp = std::shared_ptr<ScriptedProcess>( new ScriptedProcess(target_sp, listener_sp, scripted_metadata, error)); if (error.Fail() || !process_sp || !process_sp->m_interface_up) { + // CreateInstance returns nullptr on failure with no Status output + // parameter, so we must report the error via the diagnostic system for + // users to see it. + if (error.Fail()) { + Debugger::ReportError( + llvm::formatv("Failed to create ScriptedProcess: {0}", + error.AsCString()) + .str(), + target_sp->GetDebugger().GetID()); + } LLDB_LOGF(GetLog(LLDBLog::Process), "%s", error.AsCString()); return nullptr; } @@ -112,8 +132,10 @@ ScriptedProcess::ScriptedProcess(lldb::TargetSP target_sp, GetInterface().CreatePluginObject(m_scripted_metadata, exe_ctx); if (!obj_or_err) { - llvm::consumeError(obj_or_err.takeError()); - error = Status::FromErrorString("Failed to create script object."); + // Extract the detailed error message including the Python backtrace. + std::string error_msg = llvm::toString(obj_or_err.takeError()); + error = Status::FromErrorStringWithFormatv( + "Failed to create script object: {0}", error_msg); return; } @@ -189,10 +211,13 @@ Status ScriptedProcess::DoResume(RunDirection direction) { Status ScriptedProcess::DoAttach(const ProcessAttachInfo &attach_info) { Status error = GetInterface().Attach(attach_info); + if (error.Fail()) { + error = Status::FromErrorStringWithFormatv( + "Failed to attach to scripted process: {0}", error.AsCString()); + return error; + } SetPrivateState(eStateRunning); SetPrivateState(eStateStopped); - if (error.Fail()) - return error; // NOTE: We need to set the PID before finishing to attach otherwise we will // hit an assert when calling the attach completion handler. DidLaunch(); @@ -224,8 +249,14 @@ size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size, lldb::DataExtractorSP data_extractor_sp = GetInterface().ReadMemoryAtAddress(addr, size, error); - if (!data_extractor_sp || !data_extractor_sp->HasData() || error.Fail()) + if (!data_extractor_sp || !data_extractor_sp->HasData() || error.Fail()) { + if (error.Fail()) { + error = Status::FromErrorStringWithFormatv( + "Failed to read memory from scripted process at 0x{0:x}: {1}", addr, + error.AsCString()); + } return 0; + } offset_t bytes_copied = data_extractor_sp->CopyByteOrderedData( 0, data_extractor_sp->GetByteSize(), buf, size, GetByteOrder()); @@ -251,9 +282,15 @@ size_t ScriptedProcess::DoWriteMemory(lldb::addr_t vm_addr, const void *buf, lldb::offset_t bytes_written = GetInterface().WriteMemoryAtAddress(vm_addr, data_extractor_sp, error); - if (!bytes_written || bytes_written == LLDB_INVALID_OFFSET) + if (!bytes_written || bytes_written == LLDB_INVALID_OFFSET) { + if (error.Fail()) { + error = Status::FromErrorStringWithFormatv( + "Failed to write memory to scripted process at 0x{0:x}: {1}", vm_addr, + error.AsCString()); + } return ScriptedInterface::ErrorWithMessage<size_t>( LLVM_PRETTY_FUNCTION, "Failed to copy write buffer to memory.", error); + } // FIXME: We should use the diagnostic system to report a warning if the // `bytes_written` is different from `size`. @@ -566,3 +603,14 @@ void *ScriptedProcess::GetImplementation() { return object_instance_sp->GetAsGeneric()->GetValue(); return nullptr; } + +void ScriptedProcess::SetScriptedInterfaceErrorCallback( + std::function<void(const Status &)> callback) { + if (m_interface_up) + m_interface_up->SetErrorCallback(std::move(callback)); +} + +void ScriptedProcess::ClearScriptedInterfaceErrorCallback() { + if (m_interface_up) + m_interface_up->ClearErrorCallback(); +} diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h index 8371180734217..9ce4a3f8d3cc9 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h +++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h @@ -17,8 +17,6 @@ #include "ScriptedThread.h" -#include <mutex> - namespace lldb_private { class ScriptedProcess : public Process { public: @@ -93,6 +91,18 @@ class ScriptedProcess : public Process { void *GetImplementation() override; + /// Set error callback to surface Python exceptions directly to users. + /// + /// This allows command handlers to receive Python exception details + /// immediately rather than relying on diagnostic broadcasts. + /// + /// \param callback Function to call with Status containing exception details. + void SetScriptedInterfaceErrorCallback( + std::function<void(const Status &)> callback); + + /// Clear the error callback. + void ClearScriptedInterfaceErrorCallback(); + void ForceScriptedState(lldb::StateType state) override { // If we're about to stop, we should fetch the loaded dynamic libraries // dictionary before emitting the private stop event to avoid having the diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp index f87b37ca67e09..ac5dd00483f3b 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp @@ -69,11 +69,8 @@ ScriptedThread::Create(ScriptedProcess &process, auto obj_or_err = scripted_thread_interface->CreatePluginObject( thread_metadata, exe_ctx, script_object); - if (!obj_or_err) { - llvm::consumeError(obj_or_err.takeError()); - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "Failed to create script object."); - } + if (!obj_or_err) + return obj_or_err.takeError(); StructuredData::GenericSP owned_script_object_sp = *obj_or_err; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h index 7d0d4cdd3c6d1..5608d5d35d654 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h @@ -9,12 +9,14 @@ #ifndef LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_INTERFACES_SCRIPTEDPYTHONINTERFACE_H #define LLDB_SOURCE_PLUGINS_SCRIPTINTERPRETER_PYTHON_INTERFACES_SCRIPTEDPYTHONINTERFACE_H +#include <functional> #include <optional> #include <sstream> #include <tuple> #include <type_traits> #include <utility> +#include "lldb/Core/Debugger.h" #include "lldb/Interpreter/Interfaces/ScriptedInterface.h" #include "lldb/Utility/DataBufferHeap.h" @@ -29,6 +31,22 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { ScriptedPythonInterface(ScriptInterpreterPythonImpl &interpreter); ~ScriptedPythonInterface() override = default; + /// Set callback to surface Python exceptions to CommandReturnObject. + /// + /// When set, this callback will be invoked whenever a Python exception occurs + /// in scripting affordance methods, allowing errors to be surfaced directly + /// to the user via CommandReturnObject::AppendError(). + /// + /// If no callback is registered, errors will be reported via + /// Debugger::ReportError() instead. + /// + /// \param callback Function to call with Status containing exception details. + using ErrorCallback = std::function<void(const Status &)>; + void SetErrorCallback(ErrorCallback callback) override { + m_error_callback = std::move(callback); + } + void ClearErrorCallback() override { m_error_callback = nullptr; } + enum class AbstractMethodCheckerCases { eNotImplemented, eNotAllocated, @@ -130,7 +148,15 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { llvm::Expected<PythonObject> callable_or_err = class_dict.GetItem(method_name); if (!callable_or_err) { - llvm::consumeError(callable_or_err.takeError()); + Log *log = GetLog(LLDBLog::Script); + if (log) { + std::string error_msg = + ExtractPythonError(callable_or_err.takeError()); + LLDB_LOGF(log, "Failed to get method '%s': %s", method_name.data(), + error_msg.c_str()); + } else { + llvm::consumeError(callable_or_err.takeError()); + } SET_CASE_AND_CONTINUE(method_name, AbstractMethodCheckerCases::eNotAllocated) } @@ -145,7 +171,15 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { auto arg_info_or_err = callable.GetArgInfo(); if (!arg_info_or_err) { - llvm::consumeError(arg_info_or_err.takeError()); + Log *log = GetLog(LLDBLog::Script); + if (log) { + std::string error_msg = + ExtractPythonError(arg_info_or_err.takeError()); + LLDB_LOGF(log, "Failed to get arg info for method '%s': %s", + method_name.data(), error_msg.c_str()); + } else { + llvm::consumeError(arg_info_or_err.takeError()); + } SET_CASE_AND_CONTINUE(method_name, AbstractMethodCheckerCases::eUnknownArgumentCount) } @@ -258,14 +292,18 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { std::apply( [&init, &expected_return_object](auto &&...args) { - llvm::consumeError(expected_return_object.takeError()); + // Consume placeholder error (expected initial state). + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); expected_return_object = init(args...); }, std::tuple_cat(transformed_args, std::make_tuple(dict))); } else { std::apply( [&init, &expected_return_object](auto &&...args) { - llvm::consumeError(expected_return_object.takeError()); + // Consume placeholder error (expected initial state). + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); expected_return_object = init(args...); }, transformed_args); @@ -467,13 +505,41 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { llvm::createStringError("not initialized"); std::apply( [&method, &expected_return_object](auto &&...args) { - llvm::consumeError(expected_return_object.takeError()); + // Consume placeholder error (expected initial state). + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); expected_return_object = method(args...); }, transformed_args); if (llvm::Error e = expected_return_object.takeError()) { - error = Status::FromError(std::move(e)); + // Extract Python backtrace and log it. + std::string detailed_error = ExtractPythonError(std::move(e)); + + Log *log = GetLog(LLDBLog::Script); + if (log) { + LLDB_LOGF(log, "%s: Python exception in static method %s:\n%s", + caller_signature.c_str(), method_name.data(), + detailed_error.c_str()); + } + + // Create Status with full context including the interface type. + // TODO: Stringify `args` and include them in the message so users + // can see what was passed to the failing call (e.g. + // `read_memory_at_address(0x500000000, 4)`). Requires a SFINAE + // helper that falls back to a placeholder for types without a + // format_provider / operator<<. + error = Status::FromErrorStringWithFormatv( + "Python exception in {0} method '{1}':\n{2}", + GetScriptedMetadata() ? GetScriptedMetadata()->GetClassName() + : "<unknown>", + method_name, detailed_error); + + // Surface error to user: use callback if available. + if (m_error_callback) { + m_error_callback(error); + } + return ErrorWithMessage<T>( caller_signature, "python static method could not be called", error); } @@ -492,6 +558,23 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { } protected: + /// Extract detailed error message including Python backtrace if available. + /// + /// This helper processes llvm::Error objects that may contain PythonException + /// instances, extracting full Python backtraces when available. + /// + /// \param error The llvm::Error to extract information from. + /// \return A string containing the error message, including full Python + /// backtrace if the error was a PythonException. + static std::string ExtractPythonError(llvm::Error error) { + std::string error_msg; + llvm::handleAllErrors( + std::move(error), + [&](python::PythonException &E) { error_msg = E.ReadBacktrace(); }, + [&](const llvm::ErrorInfoBase &E) { error_msg = E.message(); }); + return error_msg; + } + template <typename T = StructuredData::ObjectSP> T ExtractValueFromPythonObject(python::PythonObject &p, Status &error) { return p.CreateStructuredObject(); @@ -530,14 +613,42 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { llvm::createStringError("not initialized"); std::apply( [&implementor, &method_name, &expected_return_object](auto &&...args) { - llvm::consumeError(expected_return_object.takeError()); + // Consume placeholder error (expected initial state). + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); expected_return_object = implementor.CallMethod(method_name.data(), args...); }, transformed_args); if (llvm::Error e = expected_return_object.takeError()) { - error = Status::FromError(std::move(e)); + // Extract Python backtrace and log it. + std::string detailed_error = ExtractPythonError(std::move(e)); + + Log *log = GetLog(LLDBLog::Script); + if (log) { + LLDB_LOGF(log, "%s: Python exception in %s:\n%s", + caller_signature.c_str(), method_name.data(), + detailed_error.c_str()); + } + + // Create Status with full context including the interface type. + // TODO: Stringify `args` and include them in the message so users + // can see what was passed to the failing call (e.g. + // `read_memory_at_address(0x500000000, 4)`). Requires a SFINAE + // helper that falls back to a placeholder for types without a + // format_provider / operator<<. + error = Status::FromErrorStringWithFormatv( + "Python exception in {0} method '{1}':\n{2}", + GetScriptedMetadata() ? GetScriptedMetadata()->GetClassName() + : "<unknown>", + method_name, detailed_error); + + // Surface error to user: use callback if available. + if (m_error_callback) { + m_error_callback(error); + } + return ErrorWithMessage<T>(caller_signature, "python method could not be called", error); } @@ -738,6 +849,11 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { // The lifetime is managed by the ScriptInterpreter ScriptInterpreterPythonImpl &m_interpreter; + + // Optional callback for surfacing errors to users (e.g., + // CommandReturnObject). If not set, errors are reported via + // Debugger::ReportError(). + ErrorCallback m_error_callback; }; template <> diff --git a/lldb/source/Target/ScriptedThreadPlan.cpp b/lldb/source/Target/ScriptedThreadPlan.cpp index 7b0a6bd3a7279..ffdd3cc018acb 100644 --- a/lldb/source/Target/ScriptedThreadPlan.cpp +++ b/lldb/source/Target/ScriptedThreadPlan.cpp @@ -20,6 +20,7 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/State.h" +#include "llvm/Support/FormatVariadic.h" using namespace lldb; using namespace lldb_private; @@ -89,6 +90,10 @@ void ScriptedThreadPlan::DidPush() { this->shared_from_this()); if (!obj_or_err) { m_error_str = llvm::toString(obj_or_err.takeError()); + Debugger::ReportError( + llvm::formatv("Failed to create ScriptedThreadPlan: {0}", m_error_str) + .str(), + m_process.GetTarget().GetDebugger().GetID()); SetPlanComplete(false); } else m_implementation_sp = *obj_or_err; diff --git a/lldb/test/API/functionalities/scripted_extensions/Makefile b/lldb/test/API/functionalities/scripted_extensions/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/scripted_extensions/TestScriptedExtensionsDiagnostics.py b/lldb/test/API/functionalities/scripted_extensions/TestScriptedExtensionsDiagnostics.py new file mode 100644 index 0000000000000..5ffe7c316546f --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/TestScriptedExtensionsDiagnostics.py @@ -0,0 +1,124 @@ +""" +Verify that exceptions raised inside scripted extension affordance methods +(or missing required abstract methods) are surfaced to the user. + +For entry points with no return-channel for errors +(`ScriptedProcess::CreateInstance`, `OperatingSystemPython` ctor, +`ScriptedThreadPlan::DidPush`, `BreakpointResolverScripted` ctor) the +diagnostic is broadcast via `Debugger::ReportError` and asserted on a +listener. + +Entry points that already return an `llvm::Expected` / `Status` +(`ScriptedThread::Create`, `ScriptedFrameProvider::CreateInstance`, +`StopHookScripted::SetScriptCallback`) propagate the detailed error +through their return type; tests for those are tracked as follow-up. +""" + +import os + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import expectedFailureAll +from lldbsuite.test.lldbtest import TestBase + + +class TestScriptedExtensionsDiagnostics(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def setUp(self): + TestBase.setUp(self) + self.broadcaster = self.dbg.GetBroadcaster() + self.listener = lldbutil.start_listening_from( + self.broadcaster, + lldb.SBDebugger.eBroadcastBitWarning | lldb.SBDebugger.eBroadcastBitError, + ) + script_path = os.path.join( + self.getSourceDir(), "malformed_scripted_extensions.py" + ) + self.runCmd("command script import " + script_path) + + def assert_diagnostic(self, expected_substring): + event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster) + data = lldb.SBDebugger.GetDiagnosticFromEvent(event) + self.assertTrue(data.IsValid(), "event has diagnostic data") + message = data.GetValueForKey("message").GetStringValue(4096) + self.assertIn(expected_substring, message) + + def create_target(self): + self.build() + target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) + self.assertTrue(target, "valid target") + return target + + # ------------------------------------------------------------------ + # ScriptedProcess - reports via ScriptedProcess::CreateInstance + # ------------------------------------------------------------------ + + def test_scripted_process_missing_methods(self): + """A ScriptedProcess missing abstract methods should emit a + diagnostic naming the missing method.""" + target = self.create_target() + launch_info = lldb.SBLaunchInfo(None) + launch_info.SetProcessPluginName("ScriptedProcess") + launch_info.SetScriptedProcessClassName( + "malformed_scripted_extensions.MissingMethodsScriptedProcess" + ) + error = lldb.SBError() + target.Launch(launch_info, error) + self.assertTrue(error.Fail(), "launch should fail") + self.assert_diagnostic("read_memory_at_address") + + # ------------------------------------------------------------------ + # BreakpointResolverScripted - reports via + # BreakpointResolverScripted::CreateImplementationIfNeeded. + # `m_error` is set but never surfaced to the user, so ReportError is + # the only user-visible channel. + # ------------------------------------------------------------------ + + # TODO: malformed_scripted_extensions needs a class whose __init__ + # raises (the current ExceptionScriptedBreakpointResolver only raises + # from __callback__, which goes through Dispatch/Status). Tracked as + # follow-up. + @expectedFailureAll(bugnumber="needs an init-raising malformed class") + def test_scripted_breakpoint_resolver_init_failure(self): + target = self.create_target() + target.BreakpointCreateFromScript( + "malformed_scripted_extensions.ExceptionScriptedBreakpointResolver", + lldb.SBStructuredData(), + lldb.SBFileSpecList(), + lldb.SBFileSpecList(), + ) + self.assert_diagnostic("ExceptionScriptedBreakpointResolver") + + # ------------------------------------------------------------------ + # ScriptedThreadPlan - reports via ScriptedThreadPlan::DidPush. The + # plan stores the error in m_error_str but never surfaces it; the + # diagnostic is the user-visible channel. + # ------------------------------------------------------------------ + + @expectedFailureAll(bugnumber="needs an init-raising malformed class") + def test_scripted_thread_plan_init_failure(self): + target = self.create_target() + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + self.assertTrue(process, "valid process") + thread = process.GetSelectedThread() + self.assertTrue(thread, "valid thread") + thread.StepUsingScriptedThreadPlan( + "malformed_scripted_extensions.ExceptionScriptedThreadPlan" + ) + self.assert_diagnostic("ExceptionScriptedThreadPlan") + + # ------------------------------------------------------------------ + # The remaining entry points need a live inferior process to trigger + # their creation paths, or have no plugin implementation yet. + # ------------------------------------------------------------------ + + @expectedFailureAll( + bugnumber="OperatingSystemPython needs a live process to load the OS plugin" + ) + def test_operating_system_missing_methods(self): + self.assert_diagnostic("get_thread_info") + + @expectedFailureAll(bugnumber="ScriptedPlatform has no plugin implementation yet") + def test_scripted_platform_missing_methods(self): + self.assert_diagnostic("list_processes") diff --git a/lldb/test/API/functionalities/scripted_extensions/main.c b/lldb/test/API/functionalities/scripted_extensions/main.c new file mode 100644 index 0000000000000..237c8ce181774 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/main.c @@ -0,0 +1 @@ +int main() {} diff --git a/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py b/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py new file mode 100644 index 0000000000000..e9ff0fd17a57c --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py @@ -0,0 +1,195 @@ +""" +Intentionally malformed scripted extensions used by +TestScriptedExtensionsDiagnostics. + +Each class either omits a required abstract method or raises a Python +exception from one of its affordance methods. The corresponding test asserts +that LLDB surfaces those errors to the user instead of silently swallowing +them. +""" + +# --------------------------------------------------------------------------- +# Scripted Process +# --------------------------------------------------------------------------- + + +class MissingMethodsScriptedProcess: + """Missing required abstract method `read_memory_at_address`.""" + + def __init__(self, exe_ctx, args): + self.exe_ctx = exe_ctx + self.args = args + + def get_scripted_thread_plugin(self): + return None + + def is_alive(self): + return True + + +class ExceptionScriptedProcess: + """All abstract methods present, but `launch` raises.""" + + def __init__(self, exe_ctx, args): + self.exe_ctx = exe_ctx + self.args = args + + def get_scripted_thread_plugin(self): + return None + + def is_alive(self): + return True + + def read_memory_at_address(self, addr, size, error): + return None + + def launch(self): + raise RuntimeError("intentional exception from launch()") + + +# --------------------------------------------------------------------------- +# Scripted Thread +# --------------------------------------------------------------------------- + + +class ExceptionScriptedThread: + def __init__(self, process, args): + self.process = process + self.args = args + + def get_thread_id(self): + raise ValueError("intentional exception from get_thread_id()") + + def get_register_context(self): + return "" + + def get_name(self): + return "ExceptionScriptedThread" + + def get_state(self): + return 0 + + +# --------------------------------------------------------------------------- +# Scripted Platform +# --------------------------------------------------------------------------- + + +class MissingMethodsScriptedPlatform: + """Missing required abstract method `list_processes`.""" + + def __init__(self, exe_ctx, args): + self.exe_ctx = exe_ctx + self.args = args + + +class ExceptionScriptedPlatform: + def __init__(self, exe_ctx, args): + self.exe_ctx = exe_ctx + self.args = args + + def list_processes(self): + raise RuntimeError("intentional exception from list_processes()") + + def get_process_info(self, pid): + return None + + def launch_process(self, launch_info): + return None + + def kill_process(self, pid): + return None + + +# --------------------------------------------------------------------------- +# Scripted Frame Provider +# --------------------------------------------------------------------------- + + +class ExceptionScriptedFrameProvider: + def __init__(self, frames, args): + self.frames = frames + self.args = args + + def get_num_frames(self): + raise RuntimeError("intentional exception from get_num_frames()") + + def get_frame_at_index(self, idx): + return None + + +# --------------------------------------------------------------------------- +# Scripted Thread Plan +# --------------------------------------------------------------------------- + + +class ExceptionScriptedThreadPlan: + def __init__(self, thread_plan, args): + self.thread_plan = thread_plan + self.args = args + + def explains_stop(self, event): + raise RuntimeError("intentional exception from explains_stop()") + + def should_stop(self, event): + return True + + def is_stale(self): + return False + + +# --------------------------------------------------------------------------- +# Scripted Breakpoint Resolver +# --------------------------------------------------------------------------- + + +class ExceptionScriptedBreakpointResolver: + def __init__(self, bkpt, args): + self.bkpt = bkpt + self.args = args + + def __callback__(self, sym_ctx): + raise RuntimeError("intentional exception from __callback__()") + + def get_short_help(self): + return "Exception breakpoint resolver" + + +# --------------------------------------------------------------------------- +# Scripted Stop Hook +# --------------------------------------------------------------------------- + + +class ExceptionScriptedStopHook: + def __init__(self, target, args): + self.target = target + self.args = args + + def handle_stop(self, exe_ctx, stream): + raise RuntimeError("intentional exception from handle_stop()") + + +# --------------------------------------------------------------------------- +# Operating System +# --------------------------------------------------------------------------- + + +class MissingMethodsOperatingSystem: + """Missing required abstract method `get_thread_info`.""" + + def __init__(self, process): + self.process = process + + +class ExceptionOperatingSystem: + def __init__(self, process): + self.process = process + + def get_thread_info(self): + raise RuntimeError("intentional exception from get_thread_info()") + + def get_register_info(self): + return {} + + def get_register_data(self, tid): + return b"" _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
