https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/198153
>From f51dbf65bb1707ce04804540d521ff132362bc8b Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Thu, 30 Jul 2026 16:35:02 -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>". `ErrorWithMessage` no longer overwrites a Status that already carries a detailed message. For entry points with no error-return channel, the detailed error is broadcast via `Debugger::ReportError`. A new test under lldb/test/API/functionalities/scripted_extensions/ listens for eBroadcastBitError and asserts each malformed extension produces a user-visible error. rdar://161191350 Signed-off-by: Med Ismail Bennani <[email protected]> # Conflicts: # lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h --- .../Interfaces/ScriptedBreakpointInterface.h | 2 +- .../ScriptedFrameProviderInterface.h | 2 +- .../Interfaces/ScriptedHookInterface.h | 2 +- .../Interfaces/ScriptedInterface.h | 26 +- .../lldb/Interpreter/ScriptInterpreter.h | 4 + .../Breakpoint/BreakpointResolverScripted.cpp | 8 +- .../Python/OperatingSystemPython.cpp | 8 +- .../Process/scripted/ScriptedProcess.cpp | 56 ++- .../Process/scripted/ScriptedProcess.h | 2 - .../Process/scripted/ScriptedThread.cpp | 23 +- .../Interfaces/ScriptedPythonInterface.h | 96 ++++-- .../Python/ScriptInterpreterPython.cpp | 6 +- lldb/source/Target/ScriptedThreadPlan.cpp | 5 + lldb/source/Target/StackFrameRecognizer.cpp | 11 +- .../command/script/TestCommandScript.py | 13 + .../scripted_extensions/Makefile | 3 + .../TestScriptedExtensionsDiagnostics.py | 227 ++++++++++++ .../scripted_extensions/main.c | 3 + .../malformed_scripted_extensions.py | 325 ++++++++++++++++++ .../os_plugin_missing_methods.py | 16 + .../scripted_process/TestScriptedProcess.py | 6 +- 21 files changed, 779 insertions(+), 65 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 create mode 100644 lldb/test/API/functionalities/scripted_extensions/os_plugin_missing_methods.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/ScriptedHookInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedHookInterface.h index 54d335122300d..55a61a90f3bfd 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedHookInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedHookInterface.h @@ -14,7 +14,7 @@ #include "ScriptedInterface.h" namespace lldb_private { -class ScriptedHookInterface : public ScriptedInterface { +class ScriptedHookInterface : virtual public ScriptedInterface { public: /// Describes which hook callback methods the Python class implements. struct SupportedHookMethods { diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h index 3dbc009a58311..21bb91960f777 100644 --- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h +++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedInterface.h @@ -60,20 +60,21 @@ class ScriptedInterface { template <typename Ret> static Ret ErrorWithMessage(llvm::StringRef caller_name, - llvm::StringRef error_msg, Status &error, + llvm::StringRef user_msg, Status &error, LLDBLog log_category = LLDBLog::Process) { 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)); + user_msg.data()); + + // If `error` already has detailed content (e.g. a Python traceback), + // prepend this call's friendlier message to it instead of discarding + // either one. + std::string existing_error = error.Fail() ? error.AsCString() : ""; + if (existing_error.empty()) + error = Status::FromErrorString(user_msg.data()); + else + error = Status::FromErrorStringWithFormatv("{0}: {1}", user_msg, + existing_error); + return {}; } @@ -105,4 +106,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/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h index 7df5055cfea86..925b7b08e3291 100644 --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -579,6 +579,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..ea2602ae10e3c 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..354e7ea30059e 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..5710a57c15684 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,24 @@ lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp, ScriptedMetadata scripted_metadata(target_sp->GetProcessLaunchInfo()); + 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 +129,9 @@ 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."); + std::string error_msg = llvm::toString(obj_or_err.takeError()); + error = Status::FromErrorStringWithFormatv( + "failed to create script object: {0}", error_msg); return; } @@ -189,10 +207,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 +245,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 +278,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`. @@ -374,9 +407,14 @@ bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list, auto thread_or_error = ScriptedThread::Create(*this, object_sp->GetAsGeneric()); - if (!thread_or_error) - return ScriptedInterface::ErrorWithMessage<bool>( - LLVM_PRETTY_FUNCTION, toString(thread_or_error.takeError()), error); + if (!thread_or_error) { + Debugger::ReportError( + llvm::formatv("failed to create scripted thread ({0}): {1}", idx, + llvm::toString(thread_or_error.takeError())) + .str(), + GetTarget().GetDebugger().GetID()); + return false; + } ThreadSP thread_sp = thread_or_error.get(); lldbassert(thread_sp && "Couldn't initialize scripted thread."); diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h index 8371180734217..9510f2f06dabd 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: diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp index f87b37ca67e09..9ba9beaff66d1 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp @@ -11,6 +11,7 @@ #include "Plugins/Process/Utility/RegisterContextThreadMemory.h" #include "Plugins/Process/Utility/StopInfoMachException.h" +#include "lldb/Core/Debugger.h" #include "lldb/Target/OperatingSystem.h" #include "lldb/Target/Process.h" #include "lldb/Target/RegisterContext.h" @@ -69,11 +70,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; @@ -270,14 +268,15 @@ bool ScriptedThread::LoadArtificialStackFrames() { auto frame_from_script_obj_or_err = create_frame_from_script_object(idx); if (!frame_from_script_obj_or_err) { - return ScriptedInterface::ErrorWithMessage<bool>( - LLVM_PRETTY_FUNCTION, - llvm::Twine( - "Couldn't add artificial frame (" + llvm::Twine(idx) + - llvm::Twine(") to ScriptedThread StackFrameList: ") + - llvm::toString(frame_from_script_obj_or_err.takeError())) + llvm::consumeError(frame_from_dict_or_err.takeError()); + Debugger::ReportError( + llvm::formatv( + "couldn't add artificial frame ({0}) to ScriptedThread " + "StackFrameList: {1}", + idx, llvm::toString(frame_from_script_obj_or_err.takeError())) .str(), - error, LLDBLog::Thread); + GetProcess()->GetTarget().GetDebugger().GetID()); + return false; } else { llvm::consumeError(frame_from_dict_or_err.takeError()); synth_frame_sp = *frame_from_script_obj_or_err; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h index 3223d53e25c62..ce48f2468d380 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedPythonInterface.h @@ -50,7 +50,8 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { }; AbstractMethodCheckerCases checker_case; - std::variant<std::monostate, InvalidArgumentCountPayload> payload; + std::variant<std::monostate, InvalidArgumentCountPayload, std::string> + payload; }; llvm::Expected<FileSpec> GetScriptedModulePath() override { @@ -145,9 +146,10 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { auto arg_info_or_err = callable.GetArgInfo(); if (!arg_info_or_err) { - llvm::consumeError(arg_info_or_err.takeError()); - SET_CASE_AND_CONTINUE(method_name, - AbstractMethodCheckerCases::eUnknownArgumentCount) + checker[method_name] = { + AbstractMethodCheckerCases::eUnknownArgumentCount, + ExtractPythonError(arg_info_or_err.takeError())}; + continue; } PythonCallable::ArgInfo arg_info = *arg_info_or_err; @@ -263,21 +265,28 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { std::apply( [&init, &expected_return_object](auto &&...args) { - llvm::consumeError(expected_return_object.takeError()); - expected_return_object = init(args...); + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); + expected_return_object = init.Call(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()); - expected_return_object = init(args...); + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); + expected_return_object = init.Call(args...); }, transformed_args); } if (!expected_return_object) - return expected_return_object.takeError(); + // Drain the Python exception into a plain string while the GIL is + // still held: `PythonException` owns raw `PyObject*` references, and + // `py_lock` (and the GIL it holds) is released as this function + // returns, before the caller gets a chance to touch the error. + return llvm::createStringError( + ExtractPythonError(expected_return_object.takeError())); result = expected_return_object.get(); } @@ -323,13 +332,16 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { obj_class_name.GetString(), method_checker.first))); break; - case AbstractMethodCheckerCases::eUnknownArgumentCount: + 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.", - obj_class_name.GetString(), method_checker.first))); - break; + "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< @@ -453,15 +465,26 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { llvm::createStringError("not initialized"); std::apply( [&method, &expected_return_object](auto &&...args) { - llvm::consumeError(expected_return_object.takeError()); - expected_return_object = method(args...); + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); + expected_return_object = method.Call(args...); }, transformed_args); if (llvm::Error e = expected_return_object.takeError()) { - error = Status::FromError(std::move(e)); + // 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::FromErrorString(ExtractPythonError(std::move(e)).c_str()); + return ErrorWithMessage<T>( - caller_signature, "python static method could not be called", error); + caller_signature, + llvm::formatv("python exception in {0} method '{1}'", class_name, + method_name) + .str(), + error); } PythonObject py_return = std::move(expected_return_object.get()); @@ -478,6 +501,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(); @@ -538,15 +578,29 @@ class ScriptedPythonInterface : virtual public ScriptedInterface { std::make_index_sequence<sizeof...(Args) + 1>{}, [&implementor, &method_name, &expected_return_object](auto &&...call_args) { - llvm::consumeError(expected_return_object.takeError()); + if (!expected_return_object) + llvm::consumeError(expected_return_object.takeError()); expected_return_object = implementor.CallMethod( method_name.data(), call_args...); }); if (llvm::Error e = expected_return_object.takeError()) { - error = Status::FromError(std::move(e)); - return ErrorWithMessage<T>(caller_signature, - "python method could not be called", error); + // 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::FromErrorString(ExtractPythonError(std::move(e)).c_str()); + + return ErrorWithMessage<T>( + caller_signature, + llvm::formatv("python exception in {0} method '{1}'", + GetScriptedMetadata() + ? GetScriptedMetadata()->GetClassName() + : "<unknown>", + method_name) + .str(), + error); } PythonObject py_return = std::move(expected_return_object.get()); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index c7065df8212a4..010a0ad015c12 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -2745,9 +2745,11 @@ bool ScriptInterpreterPythonImpl::RunScriptBasedCommand( cmd_retobj, exe_ctx_ref_sp); } - if (!ret_val) + if (!ret_val) { error = Status::FromErrorString("unable to execute script function"); - else if (cmd_retobj.GetStatus() == eReturnStatusFailed) + return false; + } + if (cmd_retobj.GetStatus() == eReturnStatusFailed) return false; error.Clear(); diff --git a/lldb/source/Target/ScriptedThreadPlan.cpp b/lldb/source/Target/ScriptedThreadPlan.cpp index 7b0a6bd3a7279..499a6df21f1d9 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/source/Target/StackFrameRecognizer.cpp b/lldb/source/Target/StackFrameRecognizer.cpp index 788efff98f06e..99ef837ec580b 100644 --- a/lldb/source/Target/StackFrameRecognizer.cpp +++ b/lldb/source/Target/StackFrameRecognizer.cpp @@ -8,6 +8,7 @@ #include "lldb/Target/StackFrameRecognizer.h" #include "lldb/Core/Architecture.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/Module.h" #include "lldb/Interpreter/Interfaces/ScriptedStackFrameRecognizerInterface.h" #include "lldb/Interpreter/ScriptInterpreter.h" @@ -16,6 +17,7 @@ #include "lldb/Target/Target.h" #include "lldb/Utility/RegularExpression.h" #include "lldb/Utility/ScriptedMetadata.h" +#include "llvm/Support/FormatVariadic.h" using namespace lldb; using namespace lldb_private; @@ -55,7 +57,14 @@ ScriptedStackFrameRecognizer::ScriptedStackFrameRecognizer( ScriptedMetadata scripted_metadata(m_python_class, nullptr); auto obj_or_err = m_interface_sp->CreatePluginObject(scripted_metadata); if (!obj_or_err) { - llvm::consumeError(obj_or_err.takeError()); + // CreatePluginObject has no error-return channel back to the command + // handler that requested this recognizer, so report the detailed error + // (which may include a Python backtrace) via the diagnostic system. + Debugger::ReportError( + llvm::formatv("failed to create ScriptedStackFrameRecognizer: {0}", + llvm::toString(obj_or_err.takeError())) + .str(), + interpreter->GetDebugger().GetID()); m_interface_sp.reset(); } } diff --git a/lldb/test/API/commands/command/script/TestCommandScript.py b/lldb/test/API/commands/command/script/TestCommandScript.py index eb1584c64c90d..05fbb632ab197 100644 --- a/lldb/test/API/commands/command/script/TestCommandScript.py +++ b/lldb/test/API/commands/command/script/TestCommandScript.py @@ -201,6 +201,19 @@ def cleanup(): substrs=[bad_class_name], ) + # `-f` doesn't validate the function reference until the command is + # actually invoked. Make sure a bogus reference (e.g. a typo) produces + # a visible error at invocation time instead of silently doing + # nothing. + self.runCmd( + "command script add -f nonexistent_module.nonexistent_function typo_cmd" + ) + self.expect( + "typo_cmd", + error=True, + substrs=["unable to execute script function"], + ) + def test_persistence(self): """ Ensure that function arguments meaningfully persist (and do not crash!) 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..0812483621f2d --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/TestScriptedExtensionsDiagnostics.py @@ -0,0 +1,227 @@ +""" +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, +`ScriptedStackFrameRecognizer` ctor) the diagnostic is broadcast via +`Debugger::ReportError` and asserted on a listener. + +`ScriptedThread::Create` and `ScriptedFrame::Create` also return an +`llvm::Expected`, but their only callers +(`ScriptedProcess::DoUpdateThreadList` and +`ScriptedThread::LoadArtificialStackFrames`, respectively) have no +return-channel of their own, so those errors are likewise broadcast via +`Debugger::ReportError` rather than propagated further. + +Entry points that already return an `llvm::Expected` / `Status` all the way +to a user-visible surface (`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") + + # ------------------------------------------------------------------ + # ScriptedProcess::launch() - reports via the normal SBError return + # channel (Process::DoLaunch), unlike CreateInstance's construction + # failures above, which have no return channel and go through + # Debugger::ReportError instead. + # ------------------------------------------------------------------ + + def test_scripted_process_launch_exception(self): + """An explicitly raised exception from `launch()` should surface + through the normal SBError return channel.""" + target = self.create_target() + launch_info = lldb.SBLaunchInfo(None) + launch_info.SetProcessPluginName("ScriptedProcess") + launch_info.SetScriptedProcessClassName( + "malformed_scripted_extensions.ExceptionScriptedProcess" + ) + error = lldb.SBError() + target.Launch(launch_info, error) + self.assertTrue(error.Fail(), "launch should fail") + self.assertIn("intentional exception from launch()", error.GetCString()) + + def test_scripted_process_launch_runtime_error(self): + """A natural Python runtime error (e.g. a typo'd name), as opposed + to a deliberately raised exception, should surface just as + readably: this is the other half of what users hit in practice -- + either an implementation function returns something that doesn't + make sense, or (this case) they mistyped a name and Python + complains about it at runtime.""" + target = self.create_target() + launch_info = lldb.SBLaunchInfo(None) + launch_info.SetProcessPluginName("ScriptedProcess") + launch_info.SetScriptedProcessClassName( + "malformed_scripted_extensions.TypoScriptedProcess" + ) + error = lldb.SBError() + target.Launch(launch_info, error) + self.assertTrue(error.Fail(), "launch should fail") + self.assertIn("NameError", error.GetCString()) + self.assertIn("this_name_is_never_defined", error.GetCString()) + + # ------------------------------------------------------------------ + # BreakpointResolverScripted - reports via + # BreakpointResolverScripted::CreateImplementationIfNeeded. + # `m_error` is set but never surfaced to the user, so ReportError is + # the only user-visible channel. + # ------------------------------------------------------------------ + + def test_scripted_breakpoint_resolver_init_failure(self): + target = self.create_target() + target.BreakpointCreateFromScript( + "malformed_scripted_extensions.ExceptionInitScriptedBreakpointResolver", + lldb.SBStructuredData(), + lldb.SBFileSpecList(), + lldb.SBFileSpecList(), + ) + self.assert_diagnostic("intentional exception from __init__()") + + # ------------------------------------------------------------------ + # 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. + # ------------------------------------------------------------------ + + 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.ExceptionInitScriptedThreadPlan" + ) + self.assert_diagnostic("intentional exception from __init__()") + + # ------------------------------------------------------------------ + # ScriptedThread - reports via ScriptedProcess::DoUpdateThreadList, + # which propagates ScriptedThread::Create's Expected error through + # Debugger::ReportError (DoUpdateThreadList has no return-channel back + # to its caller). + # ------------------------------------------------------------------ + + def test_scripted_thread_missing_methods(self): + """A scripted thread object returned from `get_threads_info()` that + is missing a required abstract method 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.ThreadListScriptedProcess" + ) + error = lldb.SBError() + target.Launch(launch_info, error) + self.assert_diagnostic("get_stop_reason") + + # ------------------------------------------------------------------ + # ScriptedFrame - reports via ScriptedThread::LoadArtificialStackFrames, + # which propagates ScriptedFrame::Create's Expected error through + # Debugger::ReportError (LoadArtificialStackFrames' return value is + # discarded by its only caller, RefreshStateAfterStop). + # ------------------------------------------------------------------ + + def test_scripted_frame_missing_methods(self): + """A scripted frame object returned from a thread's + `get_stackframes()` that is missing a required abstract method + 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.StackFrameScriptedProcess" + ) + error = lldb.SBError() + target.Launch(launch_info, error) + self.assert_diagnostic("get_id") + + # ------------------------------------------------------------------ + # ScriptedStackFrameRecognizer - reports via + # ScriptedStackFrameRecognizer's constructor, which has no + # error-return channel back to `frame recognizer add`. + # ------------------------------------------------------------------ + + def test_scripted_stack_frame_recognizer_init_failure(self): + target = self.create_target() + self.runCmd( + "frame recognizer add -l " + "malformed_scripted_extensions.ExceptionScriptedStackFrameRecognizer " + "-s a.out -n main" + ) + self.assert_diagnostic("intentional exception from __init__()") + + # ------------------------------------------------------------------ + # The remaining entry point has no plugin implementation yet. + # ------------------------------------------------------------------ + + def test_operating_system_missing_methods(self): + self.build() + lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c")) + os_plugin_path = os.path.join( + self.getSourceDir(), "os_plugin_missing_methods.py" + ) + self.runCmd( + "settings set target.process.python-os-plugin-path " + os_plugin_path + ) + self.runCmd("thread list") + 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..ba45ee316cd42 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/main.c @@ -0,0 +1,3 @@ +int main() { + return 0; // break here +} 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..d980e46792733 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/malformed_scripted_extensions.py @@ -0,0 +1,325 @@ +""" +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. +""" + +import lldb +from lldb.plugins.scripted_process import ScriptedProcess, ScriptedThread + +# --------------------------------------------------------------------------- +# 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()") + + +class TypoScriptedProcess: + """All abstract methods present, but `launch` has a typo (references an + undefined name) instead of an explicit `raise`, exercising a natural + Python runtime error (NameError) rather than a deliberately raised + exception.""" + + 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): + return this_name_is_never_defined + + +# --------------------------------------------------------------------------- +# Scripted Thread +# --------------------------------------------------------------------------- + + +class ExceptionScriptedThread: + """Missing required abstract method `get_stop_reason`.""" + + 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 + + +class ThreadListScriptedProcess: + """All abstract methods present, but `get_threads_info` hands back a + thread object (`ExceptionScriptedThread`) missing a required abstract + method.""" + + 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): + return lldb.SBError() + + def get_threads_info(self): + return {1: ExceptionScriptedThread(self, None)} + + +class MissingMethodsScriptedFrame: + """Missing required abstract method `get_id`.""" + + def __init__(self, thread, args): + self.thread = thread + self.args = args + + +class StackFrameScriptedThread(ScriptedThread): + """A real, valid scripted thread (borrows the base class's default + register-info plumbing so construction succeeds), but `get_stackframes` + returns a scripted frame object (`MissingMethodsScriptedFrame`) missing a + required abstract method.""" + + def __init__(self, process, args): + super().__init__(process, args) + + def get_stop_reason(self): + return {"type": lldb.eStopReasonTrace, "data": {}} + + def get_register_context(self): + total_bytes = sum( + r["bitsize"] // 8 for r in self.get_register_info()["registers"] + ) + return "\x00" * total_bytes + + def get_stackframes(self): + return [MissingMethodsScriptedFrame(self, None)] + + +class StackFrameScriptedProcess(ScriptedProcess): + """All abstract methods present, but `get_threads_info` hands back a + thread (`StackFrameScriptedThread`) whose `get_stackframes` yields a + malformed scripted frame.""" + + def __init__(self, exe_ctx, args): + super().__init__(exe_ctx, 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 get_threads_info(self): + return {1: StackFrameScriptedThread(self, None)} + + +# --------------------------------------------------------------------------- +# 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 + + +class ExceptionInitScriptedThreadPlan: + """`__init__` raises.""" + + def __init__(self, thread_plan, args): + raise RuntimeError("intentional exception from __init__()") + + +# --------------------------------------------------------------------------- +# 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" + + +class ExceptionInitScriptedBreakpointResolver: + """`__init__` raises.""" + + def __init__(self, bkpt, args): + raise RuntimeError("intentional exception from __init__()") + + +# --------------------------------------------------------------------------- +# 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()") + + +# --------------------------------------------------------------------------- +# Scripted Stack Frame Recognizer +# --------------------------------------------------------------------------- + + +class ExceptionScriptedStackFrameRecognizer: + """`__init__` raises.""" + + def __init__(self): + raise RuntimeError("intentional exception from __init__()") + + +# --------------------------------------------------------------------------- +# 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"" diff --git a/lldb/test/API/functionalities/scripted_extensions/os_plugin_missing_methods.py b/lldb/test/API/functionalities/scripted_extensions/os_plugin_missing_methods.py new file mode 100644 index 0000000000000..a559d611855e1 --- /dev/null +++ b/lldb/test/API/functionalities/scripted_extensions/os_plugin_missing_methods.py @@ -0,0 +1,16 @@ +""" +Standalone OperatingSystem plugin module for +TestScriptedExtensionsDiagnostics.test_operating_system_missing_methods. + +`OperatingSystemPython` always resolves the plugin class as +`<module>.OperatingSystemPlugIn`, derived from the +`target.process.python-os-plugin-path` setting's basename, so this scenario +needs its own file rather than a class in malformed_scripted_extensions.py. +""" + + +class OperatingSystemPlugIn: + """Missing required abstract method `get_thread_info`.""" + + def __init__(self, process): + self.process = process diff --git a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py index e8449c0faa928..f90a7422d270a 100644 --- a/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py +++ b/lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py @@ -158,7 +158,11 @@ def cleanup(): buff = process.ReadMemory(addr, 4, error) self.assertEqual(buff, None) self.assertTrue(error.Fail()) - self.assertEqual(error.GetCString(), "This is an invalid scripted process!") + self.assertEqual( + error.GetCString(), + "Failed to read memory from scripted process at 0x500000000: " + "This is an invalid scripted process!", + ) with open(log_file, "r") as f: log = f.read() _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
