Author: Stefan Gränitz Date: 2023-03-21T19:05:23+01:00 New Revision: 32baf5c1c29b6b2f282354c9f5919865bc1ff958
URL: https://github.com/llvm/llvm-project/commit/32baf5c1c29b6b2f282354c9f5919865bc1ff958 DIFF: https://github.com/llvm/llvm-project/commit/32baf5c1c29b6b2f282354c9f5919865bc1ff958.diff LOG: [lldb][expr] Propagate ClangDynamicCheckerFunctions::Install() errors to caller I came accross this, because a lot of regression tests were saying: ``` (lldb) p argc error: expression failed to parse: error: couldn't install checkers, unknown error ``` With this change, error messages provide more detail: ``` (lldb) p argc error: expression failed to parse: error: couldn't install checkers: error: Couldn't lookup symbols: __objc_load ``` I didn't find a case where `Diagnostics()` is not empty. Also it looks like this isn't covered in any test (yet). Reviewed By: bulbazord, Michael137 Differential Revision: https://reviews.llvm.org/D146541 Added: Modified: lldb/include/lldb/Expression/DynamicCheckerFunctions.h lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h Removed: ################################################################################ diff --git a/lldb/include/lldb/Expression/DynamicCheckerFunctions.h b/lldb/include/lldb/Expression/DynamicCheckerFunctions.h index 02bce5abdf4cb..57a93ca30586a 100644 --- a/lldb/include/lldb/Expression/DynamicCheckerFunctions.h +++ b/lldb/include/lldb/Expression/DynamicCheckerFunctions.h @@ -11,6 +11,8 @@ #include "lldb/lldb-types.h" +#include "llvm/Support/Error.h" + namespace lldb_private { class DiagnosticManager; @@ -46,10 +48,10 @@ class DynamicCheckerFunctions { /// The execution context to install the functions into. /// /// \return - /// True on success; false on failure, or if the functions have - /// already been installed. - virtual bool Install(DiagnosticManager &diagnostic_manager, - ExecutionContext &exe_ctx) = 0; + /// Either llvm::ErrorSuccess or Error with llvm::ErrorInfo + /// + virtual llvm::Error Install(DiagnosticManager &diagnostic_manager, + ExecutionContext &exe_ctx) = 0; virtual bool DoCheckersExplainStop(lldb::addr_t addr, Stream &message) = 0; DynamicCheckerFunctionsKind GetKind() const { return m_kind; } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 0b40df141f098..9852bbc62aa43 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -1403,14 +1403,12 @@ lldb_private::Status ClangExpressionParser::PrepareForExecution( ClangDynamicCheckerFunctions *dynamic_checkers = new ClangDynamicCheckerFunctions(); - DiagnosticManager install_diagnostics; - - if (!dynamic_checkers->Install(install_diagnostics, exe_ctx)) { - if (install_diagnostics.Diagnostics().size()) - err.SetErrorString(install_diagnostics.GetString().c_str()); - else - err.SetErrorString("couldn't install checkers, unknown error"); - + DiagnosticManager install_diags; + if (Error Err = dynamic_checkers->Install(install_diags, exe_ctx)) { + std::string ErrMsg = "couldn't install checkers: " + toString(std::move(Err)); + if (install_diags.Diagnostics().size()) + ErrMsg = ErrMsg + "\n" + install_diags.GetString().c_str(); + err.SetErrorString(ErrMsg); return err; } diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp index 0549868274685..cd7d1ff6148b3 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp @@ -47,33 +47,30 @@ ClangDynamicCheckerFunctions::ClangDynamicCheckerFunctions() ClangDynamicCheckerFunctions::~ClangDynamicCheckerFunctions() = default; -bool ClangDynamicCheckerFunctions::Install( +llvm::Error ClangDynamicCheckerFunctions::Install( DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) { - auto utility_fn_or_error = exe_ctx.GetTargetRef().CreateUtilityFunction( - g_valid_pointer_check_text, VALID_POINTER_CHECK_NAME, - lldb::eLanguageTypeC, exe_ctx); - if (!utility_fn_or_error) { - llvm::consumeError(utility_fn_or_error.takeError()); - return false; - } - m_valid_pointer_check = std::move(*utility_fn_or_error); + Expected<std::unique_ptr<UtilityFunction>> utility_fn = + exe_ctx.GetTargetRef().CreateUtilityFunction( + g_valid_pointer_check_text, VALID_POINTER_CHECK_NAME, + lldb::eLanguageTypeC, exe_ctx); + if (!utility_fn) + return utility_fn.takeError(); + m_valid_pointer_check = std::move(*utility_fn); if (Process *process = exe_ctx.GetProcessPtr()) { ObjCLanguageRuntime *objc_language_runtime = ObjCLanguageRuntime::Get(*process); if (objc_language_runtime) { - auto utility_fn_or_error = objc_language_runtime->CreateObjectChecker( - VALID_OBJC_OBJECT_CHECK_NAME, exe_ctx); - if (!utility_fn_or_error) { - llvm::consumeError(utility_fn_or_error.takeError()); - return false; - } - m_objc_object_check = std::move(*utility_fn_or_error); + Expected<std::unique_ptr<UtilityFunction>> checker_fn = + objc_language_runtime->CreateObjectChecker(VALID_OBJC_OBJECT_CHECK_NAME, exe_ctx); + if (!checker_fn) + return checker_fn.takeError(); + m_objc_object_check = std::move(*checker_fn); } } - return true; + return Error::success(); } bool ClangDynamicCheckerFunctions::DoCheckersExplainStop(lldb::addr_t addr, diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h index 4abd16c5c3261..ff20c1f08be0c 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h @@ -46,10 +46,10 @@ class ClangDynamicCheckerFunctions /// The execution context to install the functions into. /// /// \return - /// True on success; false on failure, or if the functions have - /// already been installed. - bool Install(DiagnosticManager &diagnostic_manager, - ExecutionContext &exe_ctx) override; + /// Either llvm::ErrorSuccess or Error with llvm::ErrorInfo + /// + llvm::Error Install(DiagnosticManager &diagnostic_manager, + ExecutionContext &exe_ctx) override; bool DoCheckersExplainStop(lldb::addr_t addr, Stream &message) override; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits