From dd5c5f72e00dca0e2458139fec09b1a715cfb238 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Fri, 14 Oct 2022 16:45:01 -0700 Subject: [PATCH] Make sure Target::EvaluateExpression() passes up an error instead of silently dropping it. When UserExpression::Evaluate() fails and doesn't return a ValueObject there is no vehicle for returning the error in the return value. This behavior can be observed by applying the following patch: diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index f1a311b7252c..58c03ccdb068 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -2370,6 +2370,7 @@ UserExpression *Target::GetUserExpressionForLanguage( Expression::ResultType desired_type, const EvaluateExpressionOptions &options, ValueObject *ctx_obj, Status &error) { + error.SetErrorStringWithFormat("Ha ha!"); return nullptr; auto type_system_or_err = GetScratchTypeSystemForLanguage(language); if (auto err = type_system_or_err.takeError()) { error.SetErrorStringWithFormat( and then running $ lldb -o "p 1" (lldb) p 1 (lldb) This patch fixes this by creating an empty result ValueObject that wraps the error. Differential Revision: https://reviews.llvm.org/D135998 --- lldb/source/Commands/CommandObjectExpression.cpp | 2 ++ lldb/source/Target/Target.cpp | 5 +++++ .../commands/expression/context-object/TestContextObject.py | 8 ++++---- lldb/test/API/commands/expression/fixits/TestFixIts.py | 11 ++++++----- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index b7d129e..be306f2 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -461,6 +461,8 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr, result.SetStatus(eReturnStatusFailed); } } + } else { + error_stream.Printf("error: unknown error\n"); } return (success != eExpressionSetupError && diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index c567407..f1a311b 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -26,6 +26,7 @@ #include "lldb/Core/StreamFile.h" #include "lldb/Core/StructuredDataImpl.h" #include "lldb/Core/ValueObject.h" +#include "lldb/Core/ValueObjectConstResult.h" #include "lldb/Expression/DiagnosticManager.h" #include "lldb/Expression/ExpressionVariable.h" #include "lldb/Expression/REPL.h" @@ -2528,6 +2529,10 @@ ExpressionResults Target::EvaluateExpression( execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix, result_valobj_sp, error, fixed_expression, ctx_obj); + // Pass up the error by wrapping it inside an error result. + if (error.Fail() && !result_valobj_sp) + result_valobj_sp = ValueObjectConstResult::Create( + exe_ctx.GetBestExecutionContextScope(), error); } if (execution_results == eExpressionCompleted) diff --git a/lldb/test/API/commands/expression/context-object/TestContextObject.py b/lldb/test/API/commands/expression/context-object/TestContextObject.py index 45f7a00..7c963eb 100644 --- a/lldb/test/API/commands/expression/context-object/TestContextObject.py +++ b/lldb/test/API/commands/expression/context-object/TestContextObject.py @@ -67,7 +67,7 @@ class ContextObjectTestCase(TestBase): # Test an expression evaluation value = obj_val.EvaluateExpression("1") - self.assertFalse(value.IsValid()) + self.assertTrue(value.IsValid()) self.assertFalse(value.GetError().Success()) # @@ -79,7 +79,7 @@ class ContextObjectTestCase(TestBase): # Test an expression evaluation value = obj_val.EvaluateExpression("1") - self.assertFalse(value.IsValid()) + self.assertTrue(value.IsValid()) self.assertFalse(value.GetError().Success()) # Test retrieveing of an element's field @@ -97,7 +97,7 @@ class ContextObjectTestCase(TestBase): # Test an expression evaluation value = obj_val.EvaluateExpression("1") - self.assertFalse(value.IsValid()) + self.assertTrue(value.IsValid()) self.assertFalse(value.GetError().Success()) # Test retrieveing of a dereferenced object's field @@ -127,7 +127,7 @@ class ContextObjectTestCase(TestBase): # Test an expression evaluation value = obj_val.EvaluateExpression("1") - self.assertFalse(value.IsValid()) + self.assertTrue(value.IsValid()) self.assertFalse(value.GetError().Success()) # Test retrieveing of a dereferenced object's field diff --git a/lldb/test/API/commands/expression/fixits/TestFixIts.py b/lldb/test/API/commands/expression/fixits/TestFixIts.py index bcc7e61..cd82a2f 100644 --- a/lldb/test/API/commands/expression/fixits/TestFixIts.py +++ b/lldb/test/API/commands/expression/fixits/TestFixIts.py @@ -47,10 +47,11 @@ class ExprCommandWithFixits(TestBase): # The Fix-It changes "ptr.m" to "ptr->m". expr = "struct MyTy { int m; }; MyTy x; MyTy *ptr = &x; int m = ptr.m;" value = frame.EvaluateExpression(expr, top_level_options) - # A successfully parsed top-level expression will yield an error - # that there is 'no value'. If a parsing error would have happened we - # would get a different error kind, so let's check the error kind here. - self.assertEquals(value.GetError().GetCString(), "error: No value") + # A successfully parsed top-level expression will yield an + # unknown error . If a parsing error would have happened we + # would get a different error kind, so let's check the error + # kind here. + self.assertEquals(value.GetError().GetCString(), "unknown error") # Try with two errors: two_error_expression = "my_pointer.second->a" @@ -170,7 +171,7 @@ class ExprCommandWithFixits(TestBase): multiple_runs_options.SetRetriesWithFixIts(2) value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options) # This error signals success for top level expressions. - self.assertEquals(value.GetError().GetCString(), "error: No value") + self.assertEquals(value.GetError().GetCString(), "unknown error") # Test that the code above compiles to the right thing. self.expect_expr("test_X(1)", result_type="int", result_value="123") -- 2.7.4