Author: Dave Lee Date: 2025-08-15T08:37:26-07:00 New Revision: ae7e1b82fe97f184fdc042f339784a64f28d5c08
URL: https://github.com/llvm/llvm-project/commit/ae7e1b82fe97f184fdc042f339784a64f28d5c08 DIFF: https://github.com/llvm/llvm-project/commit/ae7e1b82fe97f184fdc042f339784a64f28d5c08.diff LOG: [lldb] Print ValueObject when GetObjectDescription fails (#152417) This fixes a few bugs, effectively through a fallback to `p` when `po` fails. The motivating bug this fixes is when an error within the compiler causes `po` to fail. Previously when that happened, only its value (typically an object's address) was printed – and problematically, no compiler diagnostics were shown. With this change, compiler diagnostics are shown, _and_ the object is fully printed (ie `p`). Another bug this fixes is when `po` is used on a type that doesn't provide an object description (such as a struct). Again, the normal `ValueObject` printing is used. Additionally, this also improves how lldb handles an object description method that fails in some way. Now an error will be shown (it wasn't before), and the value will be printed normally. Added: lldb/test/API/lang/objc/failing-description/Makefile lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py lldb/test/API/lang/objc/failing-description/main.m lldb/test/API/lang/objc/struct-description/Makefile lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py lldb/test/API/lang/objc/struct-description/main.m Modified: lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h lldb/include/lldb/DataFormatters/ValueObjectPrinter.h lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h lldb/source/Commands/CommandObjectDWIMPrint.cpp lldb/source/Commands/CommandObjectExpression.cpp lldb/source/DataFormatters/DumpValueObjectOptions.cpp lldb/source/DataFormatters/ValueObjectPrinter.cpp lldb/source/Expression/REPL.cpp lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h index cdb620e2148de..70166f33cfc45 100644 --- a/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h +++ b/lldb/include/lldb/DataFormatters/DumpValueObjectOptions.h @@ -76,7 +76,9 @@ class DumpValueObjectOptions { DumpValueObjectOptions &SetShowLocation(bool show = false); - DumpValueObjectOptions &SetUseObjectiveC(bool use = false); + DumpValueObjectOptions &DisableObjectDescription(); + + DumpValueObjectOptions &SetUseObjectDescription(bool use = false); DumpValueObjectOptions &SetShowSummary(bool show = true); @@ -143,13 +145,14 @@ class DumpValueObjectOptions { ChildPrintingDecider m_child_printing_decider; PointerAsArraySettings m_pointer_as_array; unsigned m_expand_ptr_type_flags = 0; + // The following flags commonly default to false. bool m_use_synthetic : 1; bool m_scope_already_checked : 1; bool m_flat_output : 1; bool m_ignore_cap : 1; bool m_show_types : 1; bool m_show_location : 1; - bool m_use_objc : 1; + bool m_use_object_desc : 1; bool m_hide_root_type : 1; bool m_hide_root_name : 1; bool m_hide_name : 1; diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h index f9deb6ebf8d6d..fdea3682f8844 100644 --- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h +++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h @@ -75,8 +75,6 @@ class ValueObjectPrinter { void SetupMostSpecializedValue(); - llvm::Expected<std::string> GetDescriptionForDisplay(); - const char *GetRootNameForDisplay(); bool ShouldPrintValueObject(); @@ -108,8 +106,7 @@ class ValueObjectPrinter { bool PrintValueAndSummaryIfNeeded(bool &value_printed, bool &summary_printed); - llvm::Error PrintObjectDescriptionIfNeeded(bool value_printed, - bool summary_printed); + void PrintObjectDescriptionIfNeeded(std::optional<std::string> object_desc); bool ShouldPrintChildren(DumpValueObjectOptions::PointerDepth &curr_ptr_depth); @@ -141,6 +138,7 @@ class ValueObjectPrinter { private: bool ShouldShowName() const; + bool ShouldPrintObjectDescription(); ValueObject &m_orig_valobj; /// Cache the current "most specialized" value. Don't use this diff --git a/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h b/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h index ebf26cea95cf3..4fd552a6700b8 100644 --- a/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h +++ b/lldb/include/lldb/Interpreter/OptionGroupValueObjectDisplay.h @@ -31,7 +31,7 @@ class OptionGroupValueObjectDisplay : public OptionGroup { bool AnyOptionWasSet() const { return show_types || no_summary_depth != 0 || show_location || - flat_output || use_objc || max_depth != UINT32_MAX || + flat_output || use_object_desc || max_depth != UINT32_MAX || ptr_depth != 0 || !use_synth || be_raw || ignore_cap || run_validator; } @@ -42,7 +42,7 @@ class OptionGroupValueObjectDisplay : public OptionGroup { lldb::Format format = lldb::eFormatDefault, lldb::TypeSummaryImplSP summary_sp = lldb::TypeSummaryImplSP()); - bool show_types : 1, show_location : 1, flat_output : 1, use_objc : 1, + bool show_types : 1, show_location : 1, flat_output : 1, use_object_desc : 1, use_synth : 1, be_raw : 1, ignore_cap : 1, run_validator : 1, max_depth_is_default : 1; diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp index d7589cc0396f6..5e864a4cc52c2 100644 --- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp +++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp @@ -93,7 +93,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, dump_options.SetHideRootName(suppress_result) .SetExpandPointerTypeFlags(lldb::eTypeIsObjC); - bool is_po = m_varobj_options.use_objc; + bool is_po = m_varobj_options.use_object_desc; StackFrame *frame = m_exe_ctx.GetFramePtr(); diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index c5b91678103d5..197bffe9c982f 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -202,7 +202,7 @@ EvaluateExpressionOptions CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions( const Target &target, const OptionGroupValueObjectDisplay &display_opts) { EvaluateExpressionOptions options; - options.SetCoerceToId(display_opts.use_objc); + options.SetCoerceToId(display_opts.use_object_desc); options.SetUnwindOnError(unwind_on_error); options.SetIgnoreBreakpoints(ignore_breakpoints); options.SetKeepInMemory(true); @@ -241,11 +241,11 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions( bool CommandObjectExpression::CommandOptions::ShouldSuppressResult( const OptionGroupValueObjectDisplay &display_opts) const { // Explicitly disabling persistent results takes precedence over the - // m_verbosity/use_objc logic. + // m_verbosity/use_object_desc logic. if (suppress_persistent_result != eLazyBoolCalculate) return suppress_persistent_result == eLazyBoolYes; - return display_opts.use_objc && + return display_opts.use_object_desc && m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact; } @@ -332,7 +332,7 @@ Options *CommandObjectExpression::GetOptions() { return &m_option_group; } void CommandObjectExpression::HandleCompletion(CompletionRequest &request) { EvaluateExpressionOptions options; - options.SetCoerceToId(m_varobj_options.use_objc); + options.SetCoerceToId(m_varobj_options.use_object_desc); options.SetLanguage(m_command_options.language); options.SetExecutionPolicy(lldb_private::eExecutionPolicyNever); options.SetAutoApplyFixIts(false); diff --git a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp index c343e6083df19..e1df9522256fa 100644 --- a/lldb/source/DataFormatters/DumpValueObjectOptions.cpp +++ b/lldb/source/DataFormatters/DumpValueObjectOptions.cpp @@ -17,7 +17,7 @@ DumpValueObjectOptions::DumpValueObjectOptions() : m_summary_sp(), m_root_valobj_name(), m_decl_printing_helper(), m_child_printing_decider(), m_pointer_as_array(), m_use_synthetic(true), m_scope_already_checked(false), m_flat_output(false), m_ignore_cap(false), - m_show_types(false), m_show_location(false), m_use_objc(false), + m_show_types(false), m_show_location(false), m_use_object_desc(false), m_hide_root_type(false), m_hide_root_name(false), m_hide_name(false), m_hide_value(false), m_run_validator(false), m_use_type_display_name(true), m_allow_oneliner_mode(true), @@ -65,8 +65,19 @@ DumpValueObjectOptions &DumpValueObjectOptions::SetShowLocation(bool show) { return *this; } -DumpValueObjectOptions &DumpValueObjectOptions::SetUseObjectiveC(bool use) { - m_use_objc = use; +DumpValueObjectOptions &DumpValueObjectOptions::DisableObjectDescription() { + // Reset these options to their default values. + SetUseObjectDescription(false); + SetHideRootType(false); + SetHideName(false); + SetHideValue(false); + SetShowSummary(true); + return *this; +} + +DumpValueObjectOptions & +DumpValueObjectOptions::SetUseObjectDescription(bool use) { + m_use_object_desc = use; return *this; } diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp index c2f8bb3ea05bc..e20051140806b 100644 --- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp +++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp @@ -14,9 +14,11 @@ #include "lldb/Target/Target.h" #include "lldb/Utility/Stream.h" #include "lldb/ValueObject/ValueObject.h" +#include "llvm/Support/Error.h" #include "llvm/Support/MathExtras.h" #include <cstdint> #include <memory> +#include <optional> using namespace lldb; using namespace lldb_private; @@ -69,6 +71,18 @@ void ValueObjectPrinter::Init( SetupMostSpecializedValue(); } +static const char *maybeNewline(const std::string &s) { + // If the string already ends with a \n don't add another one. + if (s.empty() || s.back() != '\n') + return "\n"; + return ""; +} + +bool ValueObjectPrinter::ShouldPrintObjectDescription() { + return ShouldPrintValueObject() && m_options.m_use_object_desc && !IsNil() && + !IsUninitialized() && !m_options.m_pointer_as_array; +} + llvm::Error ValueObjectPrinter::PrintValueObject() { // If the incoming ValueObject is in an error state, the best we're going to // get out of it is its type. But if we don't even have that, just print @@ -77,6 +91,25 @@ llvm::Error ValueObjectPrinter::PrintValueObject() { !m_orig_valobj.GetCompilerType().IsValid()) return m_orig_valobj.GetError().ToError(); + std::optional<std::string> object_desc; + if (ShouldPrintObjectDescription()) { + // The object description is invoked now, but not printed until after + // value/summary. Calling GetObjectDescription at the outset of printing + // allows for early discovery of errors. In the case of an error, the value + // object is printed normally. + llvm::Expected<std::string> object_desc_or_err = + GetMostSpecializedValue().GetObjectDescription(); + if (!object_desc_or_err) { + auto error_msg = toString(object_desc_or_err.takeError()); + *m_stream << "error: " << error_msg << maybeNewline(error_msg); + + // Print the value object directly. + m_options.DisableObjectDescription(); + } else { + object_desc = *object_desc_or_err; + } + } + if (ShouldPrintValueObject()) { PrintLocationIfNeeded(); m_stream->Indent(); @@ -90,8 +123,10 @@ llvm::Error ValueObjectPrinter::PrintValueObject() { m_val_summary_ok = PrintValueAndSummaryIfNeeded(value_printed, summary_printed); - if (m_val_summary_ok) + if (m_val_summary_ok) { + PrintObjectDescriptionIfNeeded(object_desc); return PrintChildrenIfNeeded(value_printed, summary_printed); + } m_stream->EOL(); return llvm::Error::success(); @@ -144,24 +179,6 @@ void ValueObjectPrinter::SetupMostSpecializedValue() { "SetupMostSpecialized value must compute a valid ValueObject"); } -llvm::Expected<std::string> ValueObjectPrinter::GetDescriptionForDisplay() { - ValueObject &valobj = GetMostSpecializedValue(); - llvm::Expected<std::string> maybe_str = valobj.GetObjectDescription(); - if (maybe_str) - return maybe_str; - - const char *str = nullptr; - if (!str) - str = valobj.GetSummaryAsCString(); - if (!str) - str = valobj.GetValueAsCString(); - - if (!str) - return maybe_str; - llvm::consumeError(maybe_str.takeError()); - return str; -} - const char *ValueObjectPrinter::GetRootNameForDisplay() { const char *root_valobj_name = m_options.m_root_valobj_name.empty() @@ -468,38 +485,14 @@ bool ValueObjectPrinter::PrintValueAndSummaryIfNeeded(bool &value_printed, return !error_printed; } -llvm::Error -ValueObjectPrinter::PrintObjectDescriptionIfNeeded(bool value_printed, - bool summary_printed) { - if (ShouldPrintValueObject()) { - // let's avoid the overly verbose no description error for a nil thing - if (m_options.m_use_objc && !IsNil() && !IsUninitialized() && - (!m_options.m_pointer_as_array)) { - if (!m_options.m_hide_value || ShouldShowName()) - *m_stream << ' '; - llvm::Expected<std::string> object_desc = - (value_printed || summary_printed) - ? GetMostSpecializedValue().GetObjectDescription() - : GetDescriptionForDisplay(); - if (!object_desc) { - // If no value or summary was printed, surface the error. - if (!value_printed && !summary_printed) - return object_desc.takeError(); - // Otherwise gently nudge the user that they should have used - // `p` instead of `po`. Unfortunately we cannot be more direct - // about this, because we don't actually know what the user did. - *m_stream << "warning: no object description available\n"; - llvm::consumeError(object_desc.takeError()); - } else { - *m_stream << *object_desc; - // If the description already ends with a \n don't add another one. - if (object_desc->empty() || object_desc->back() != '\n') - *m_stream << '\n'; - } - return llvm::Error::success(); - } - } - return llvm::Error::success(); +void ValueObjectPrinter::PrintObjectDescriptionIfNeeded( + std::optional<std::string> object_desc) { + if (!object_desc) + return; + + if (!m_options.m_hide_value || ShouldShowName()) + *m_stream << ' '; + *m_stream << *object_desc << maybeNewline(*object_desc); } bool DumpValueObjectOptions::PointerDepth::CanAllowExpansion() const { @@ -524,7 +517,7 @@ bool ValueObjectPrinter::ShouldPrintChildren( if (m_options.m_pointer_as_array) return true; - if (m_options.m_use_objc) + if (m_options.m_use_object_desc) return false; bool print_children = true; @@ -819,9 +812,6 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool hide_names) { llvm::Error ValueObjectPrinter::PrintChildrenIfNeeded(bool value_printed, bool summary_printed) { - auto error = PrintObjectDescriptionIfNeeded(value_printed, summary_printed); - if (error) - return error; ValueObject &valobj = GetMostSpecializedValue(); diff --git a/lldb/source/Expression/REPL.cpp b/lldb/source/Expression/REPL.cpp index e5377d3114af3..92017d271a2df 100644 --- a/lldb/source/Expression/REPL.cpp +++ b/lldb/source/Expression/REPL.cpp @@ -321,7 +321,7 @@ void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) { const bool colorize_err = error_sp->GetFile().GetIsTerminalWithColors(); EvaluateExpressionOptions expr_options = m_expr_options; - expr_options.SetCoerceToId(m_varobj_options.use_objc); + expr_options.SetCoerceToId(m_varobj_options.use_object_desc); expr_options.SetKeepInMemory(true); expr_options.SetUseDynamic(m_varobj_options.use_dynamic); expr_options.SetGenerateDebugInfo(true); diff --git a/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp b/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp index d633c469e603e..1939d79fec6a9 100644 --- a/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp +++ b/lldb/source/Interpreter/OptionGroupValueObjectDisplay.cpp @@ -90,7 +90,7 @@ Status OptionGroupValueObjectDisplay::SetOptionValue( flat_output = true; break; case 'O': - use_objc = true; + use_object_desc = true; break; case 'R': be_raw = true; @@ -163,7 +163,7 @@ void OptionGroupValueObjectDisplay::OptionParsingStarting( no_summary_depth = 0; show_location = false; flat_output = false; - use_objc = false; + use_object_desc = false; max_depth = UINT32_MAX; max_depth_is_default = true; ptr_depth = 0; @@ -191,14 +191,14 @@ DumpValueObjectOptions OptionGroupValueObjectDisplay::GetAsDumpOptions( lldb::Format format, lldb::TypeSummaryImplSP summary_sp) { DumpValueObjectOptions options; options.SetMaximumPointerDepth(ptr_depth); - if (use_objc) + if (use_object_desc) options.SetShowSummary(false); else options.SetOmitSummaryDepth(no_summary_depth); options.SetMaximumDepth(max_depth, max_depth_is_default) .SetShowTypes(show_types) .SetShowLocation(show_location) - .SetUseObjectiveC(use_objc) + .SetUseObjectDescription(use_object_desc) .SetUseDynamicType(use_dynamic) .SetUseSyntheticValue(use_synth) .SetFlatOutput(flat_output) @@ -208,8 +208,9 @@ DumpValueObjectOptions OptionGroupValueObjectDisplay::GetAsDumpOptions( if (lang_descr_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact) - options.SetHideRootType(use_objc).SetHideName(use_objc).SetHideValue( - use_objc); + options.SetHideRootType(use_object_desc) + .SetHideName(use_object_desc) + .SetHideValue(use_object_desc); if (be_raw) options.SetRawDisplay(); diff --git a/lldb/test/API/lang/objc/failing-description/Makefile b/lldb/test/API/lang/objc/failing-description/Makefile new file mode 100644 index 0000000000000..c8d46cd4cd21a --- /dev/null +++ b/lldb/test/API/lang/objc/failing-description/Makefile @@ -0,0 +1,3 @@ +OBJC_SOURCES := main.m +LD_EXTRAS := -lobjc -framework Foundation +include Makefile.rules diff --git a/lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py b/lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py new file mode 100644 index 0000000000000..a06ff577e7f7d --- /dev/null +++ b/lldb/test/API/lang/objc/failing-description/TestObjCFailingDescription.py @@ -0,0 +1,17 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + def test(self): + self.build() + lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m")) + self.expect( + "expr -O -- bad", substrs=["error:", "expression interrupted", "(Bad *) 0x"] + ) + self.expect( + "dwim-print -O -- bad", + substrs=["error:", "expression interrupted", "_lookHere = NO"], + ) diff --git a/lldb/test/API/lang/objc/failing-description/main.m b/lldb/test/API/lang/objc/failing-description/main.m new file mode 100644 index 0000000000000..7416e0a76544d --- /dev/null +++ b/lldb/test/API/lang/objc/failing-description/main.m @@ -0,0 +1,21 @@ +#import <Foundation/Foundation.h> + +@interface Bad : NSObject +@end + +@implementation Bad { + BOOL _lookHere; +} + +- (NSString *)description { + int *i = NULL; + *i = 0; + return @"surprise"; +} +@end + +int main() { + Bad *bad = [Bad new]; + printf("break here\n"); + return 0; +} diff --git a/lldb/test/API/lang/objc/struct-description/Makefile b/lldb/test/API/lang/objc/struct-description/Makefile new file mode 100644 index 0000000000000..d0aadc1af9e58 --- /dev/null +++ b/lldb/test/API/lang/objc/struct-description/Makefile @@ -0,0 +1,2 @@ +OBJC_SOURCES := main.m +include Makefile.rules diff --git a/lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py b/lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py new file mode 100644 index 0000000000000..d152b2690c663 --- /dev/null +++ b/lldb/test/API/lang/objc/struct-description/TestObjCStructDescription.py @@ -0,0 +1,18 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCase(TestBase): + def test(self): + self.build() + lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.m")) + self.expect( + "vo pair", + substrs=["error:", "not a pointer type", "(Pair) pair = (f = 2, e = 3)"], + ) + self.expect( + "expr -O -- pair", + substrs=["error:", "not a pointer type", "(Pair) (f = 2, e = 3)"], + ) diff --git a/lldb/test/API/lang/objc/struct-description/main.m b/lldb/test/API/lang/objc/struct-description/main.m new file mode 100644 index 0000000000000..5031a28437e60 --- /dev/null +++ b/lldb/test/API/lang/objc/struct-description/main.m @@ -0,0 +1,12 @@ +#include <stdio.h> + +typedef struct Pair { + int f; + int e; +} Pair; + +int main() { + Pair pair = {2, 3}; + printf("break here\n"); + return 0; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
