llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) <details> <summary>Changes</summary> This change by itself has no measurable effect on the LLDB testsuite. I'm making it in preparation for threading through more errors in the Swift language plugin. --- Patch is 20.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/95857.diff 20 Files Affected: - (modified) lldb/include/lldb/Core/ValueObject.h (+2-2) - (modified) lldb/include/lldb/DataFormatters/ValueObjectPrinter.h (+1-1) - (modified) lldb/source/API/SBValue.cpp (+4-1) - (modified) lldb/source/Breakpoint/Watchpoint.cpp (+6-2) - (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+7-2) - (modified) lldb/source/Commands/CommandObjectExpression.cpp (+5-1) - (modified) lldb/source/Commands/CommandObjectFrame.cpp (+13-5) - (modified) lldb/source/Commands/CommandObjectMemory.cpp (+4-1) - (modified) lldb/source/Commands/CommandObjectTarget.cpp (+2-1) - (modified) lldb/source/Commands/CommandObjectThread.cpp (+10-4) - (modified) lldb/source/Core/DumpRegisterValue.cpp (+2-1) - (modified) lldb/source/Core/FormatEntity.cpp (+9-2) - (modified) lldb/source/Core/ValueObject.cpp (+6-3) - (modified) lldb/source/DataFormatters/ValueObjectPrinter.cpp (+11-7) - (modified) lldb/source/Plugins/REPL/Clang/ClangREPL.cpp (+3-1) - (modified) lldb/test/API/commands/dwim-print/TestDWIMPrint.py (+12-4) - (modified) lldb/test/API/commands/dwim-print/main.c (+5-1) - (modified) lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s (+1-1) - (modified) lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s (+1-1) - (modified) lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp (+4-3) ``````````diff diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h index db1fdae170ed0..205d9ad9b1953 100644 --- a/lldb/include/lldb/Core/ValueObject.h +++ b/lldb/include/lldb/Core/ValueObject.h @@ -694,9 +694,9 @@ class ValueObject { virtual SymbolContextScope *GetSymbolContextScope(); - void Dump(Stream &s); + llvm::Error Dump(Stream &s); - void Dump(Stream &s, const DumpValueObjectOptions &options); + llvm::Error Dump(Stream &s, const DumpValueObjectOptions &options); static lldb::ValueObjectSP CreateValueObjectFromExpression(llvm::StringRef name, diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h index fb5d60ba30d77..7460370c77e80 100644 --- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h +++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h @@ -33,7 +33,7 @@ class ValueObjectPrinter { ~ValueObjectPrinter() = default; - bool PrintValueObject(); + llvm::Error PrintValueObject(); protected: typedef std::set<uint64_t> InstancePointersSet; diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp index 9d7efba024d11..1b9cae6e350aa 100644 --- a/lldb/source/API/SBValue.cpp +++ b/lldb/source/API/SBValue.cpp @@ -1233,7 +1233,10 @@ bool SBValue::GetDescription(SBStream &description) { DumpValueObjectOptions options; options.SetUseDynamicType(m_opaque_sp->GetUseDynamic()); options.SetUseSyntheticValue(m_opaque_sp->GetUseSynthetic()); - value_sp->Dump(strm, options); + if (llvm::Error error = value_sp->Dump(strm, options)) { + strm << "error: " << toString(std::move(error)); + return false; + } } else { strm.PutCString("No value"); } diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp index edb1a0e93460c..715e83c76697b 100644 --- a/lldb/source/Breakpoint/Watchpoint.cpp +++ b/lldb/source/Breakpoint/Watchpoint.cpp @@ -299,7 +299,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const { .SetHideRootType(true) .SetHideRootName(true) .SetHideName(true); - m_old_value_sp->Dump(strm, options); + if (llvm::Error error = m_old_value_sp->Dump(strm, options)) + strm << "error: " << toString(std::move(error)); + if (strm.GetData()) values_ss.Printf("old value: %s", strm.GetData()); } @@ -322,7 +324,9 @@ bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const { .SetHideRootType(true) .SetHideRootName(true) .SetHideName(true); - m_new_value_sp->Dump(strm, options); + if (llvm::Error error = m_new_value_sp->Dump(strm, options)) + strm << "error: " << toString(std::move(error)); + if (strm.GetData()) values_ss.Printf("new value: %s", strm.GetData()); } diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp index 57a372a762e15..c1549ca6933fc 100644 --- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp +++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp @@ -133,12 +133,17 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command, auto dump_val_object = [&](ValueObject &valobj) { if (is_po) { StreamString temp_result_stream; - valobj.Dump(temp_result_stream, dump_options); + if (llvm::Error error = valobj.Dump(temp_result_stream, dump_options)) { + result.AppendError(toString(std::move(error))); + return; + } llvm::StringRef output = temp_result_stream.GetString(); maybe_add_hint(output); result.GetOutputStream() << output; } else { - valobj.Dump(result.GetOutputStream(), dump_options); + if (llvm::Error error = + valobj.Dump(result.GetOutputStream(), dump_options)) + result.AppendError(toString(std::move(error))); } }; diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp index 2319ddd3c80af..eb76753d98efc 100644 --- a/lldb/source/Commands/CommandObjectExpression.cpp +++ b/lldb/source/Commands/CommandObjectExpression.cpp @@ -461,7 +461,11 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr, options.SetVariableFormatDisplayLanguage( result_valobj_sp->GetPreferredDisplayLanguage()); - result_valobj_sp->Dump(output_stream, options); + if (llvm::Error error = + result_valobj_sp->Dump(output_stream, options)) { + result.AppendError(toString(std::move(error))); + return false; + } if (suppress_result) if (auto result_var_sp = diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index b1d060b3c6cf2..3f4178c1a9595 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -170,7 +170,8 @@ class CommandObjectFrameDiagnose : public CommandObjectParsed { assert(valobj_sp.get() && "Must have a valid ValueObject to print"); ValueObjectPrinter printer(*valobj_sp, &result.GetOutputStream(), options); - printer.PrintValueObject(); + if (llvm::Error error = printer.PrintValueObject()) + result.AppendError(toString(std::move(error))); } CommandOptions m_options; @@ -555,7 +556,9 @@ may even involve JITing and running code in the target program.)"); show_module)) s.PutCString(": "); } - valobj_sp->Dump(result.GetOutputStream(), options); + auto &strm = result.GetOutputStream(); + if (llvm::Error error = valobj_sp->Dump(strm, options)) + result.AppendError(toString(std::move(error))); } } } else { @@ -597,7 +600,8 @@ may even involve JITing and running code in the target program.)"); Stream &output_stream = result.GetOutputStream(); options.SetRootValueObjectName( valobj_sp->GetParent() ? entry.c_str() : nullptr); - valobj_sp->Dump(output_stream, options); + if (llvm::Error error = valobj_sp->Dump(output_stream, options)) + result.AppendError(toString(std::move(error))); } else { if (auto error_cstr = error.AsCString(nullptr)) result.AppendError(error_cstr); @@ -648,7 +652,9 @@ may even involve JITing and running code in the target program.)"); valobj_sp->GetPreferredDisplayLanguage()); options.SetRootValueObjectName( var_sp ? var_sp->GetName().AsCString() : nullptr); - valobj_sp->Dump(result.GetOutputStream(), options); + if (llvm::Error error = + valobj_sp->Dump(result.GetOutputStream(), options)) + result.AppendError(toString(std::move(error))); } } } @@ -669,7 +675,9 @@ may even involve JITing and running code in the target program.)"); options.SetVariableFormatDisplayLanguage( rec_value_sp->GetPreferredDisplayLanguage()); options.SetRootValueObjectName(rec_value_sp->GetName().AsCString()); - rec_value_sp->Dump(result.GetOutputStream(), options); + if (llvm::Error error = + rec_value_sp->Dump(result.GetOutputStream(), options)) + result.AppendError(toString(std::move(error))); } } } diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp index 1c13484dede64..137b1ad981073 100644 --- a/lldb/source/Commands/CommandObjectMemory.cpp +++ b/lldb/source/Commands/CommandObjectMemory.cpp @@ -815,7 +815,10 @@ class CommandObjectMemoryRead : public CommandObjectParsed { DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions( eLanguageRuntimeDescriptionDisplayVerbosityFull, format)); - valobj_sp->Dump(*output_stream_p, options); + if (llvm::Error error = valobj_sp->Dump(*output_stream_p, options)) { + result.AppendError(toString(std::move(error))); + return; + } } else { result.AppendErrorWithFormat( "failed to create a value object for: (%s) %s\n", diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index ae6c6d5479a19..80181a9b3cb71 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -784,7 +784,8 @@ class CommandObjectTargetVariable : public CommandObjectParsed { options.SetRootValueObjectName(root_name); - valobj_sp->Dump(s, options); + if (llvm::Error error = valobj_sp->Dump(s, options)) + s << "error: " << toString(std::move(error)); } static size_t GetVariableCallback(void *baton, const char *name, diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index bb2be560ebfff..5e64dd2f8f084 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -1386,7 +1386,10 @@ class CommandObjectThreadException : public CommandObjectIterateOverThreads { Stream &strm = result.GetOutputStream(); ValueObjectSP exception_object_sp = thread_sp->GetCurrentException(); if (exception_object_sp) { - exception_object_sp->Dump(strm); + if (llvm::Error error = exception_object_sp->Dump(strm)) { + result.AppendError(toString(std::move(error))); + return false; + } } ThreadSP exception_thread_sp = thread_sp->GetCurrentExceptionBacktrace(); @@ -1438,9 +1441,12 @@ class CommandObjectThreadSiginfo : public CommandObjectIterateOverThreads { return false; } ValueObjectSP exception_object_sp = thread_sp->GetSiginfoValue(); - if (exception_object_sp) - exception_object_sp->Dump(strm); - else + if (exception_object_sp) { + if (llvm::Error error = exception_object_sp->Dump(strm)) { + result.AppendError(toString(std::move(error))); + return false; + } + } else strm.Printf("(no siginfo)\n"); strm.PutChar('\n'); diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp index 463aa59267725..90b31fd0e865e 100644 --- a/lldb/source/Core/DumpRegisterValue.cpp +++ b/lldb/source/Core/DumpRegisterValue.cpp @@ -54,7 +54,8 @@ static void dump_type_value(lldb_private::CompilerType &fields_type, T value, }; dump_options.SetChildPrintingDecider(decider).SetHideRootType(true); - vobj_sp->Dump(strm, dump_options); + if (llvm::Error error = vobj_sp->Dump(strm, dump_options)) + strm << "error: " << toString(std::move(error)); } void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream &s, diff --git a/lldb/source/Core/FormatEntity.cpp b/lldb/source/Core/FormatEntity.cpp index 1c3a4cb1062fe..fe95858f35c9f 100644 --- a/lldb/source/Core/FormatEntity.cpp +++ b/lldb/source/Core/FormatEntity.cpp @@ -1401,7 +1401,10 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, ValueObjectSP return_valobj_sp = StopInfo::GetReturnValueObject(stop_info_sp); if (return_valobj_sp) { - return_valobj_sp->Dump(s); + if (llvm::Error error = return_valobj_sp->Dump(s)) { + s << "error: " << toString(std::move(error)); + return false; + } return true; } } @@ -1418,7 +1421,11 @@ bool FormatEntity::Format(const Entry &entry, Stream &s, ExpressionVariableSP expression_var_sp = StopInfo::GetExpressionVariable(stop_info_sp); if (expression_var_sp && expression_var_sp->GetValueObject()) { - expression_var_sp->GetValueObject()->Dump(s); + if (llvm::Error error = + expression_var_sp->GetValueObject()->Dump(s)) { + s << "error: " << toString(std::move(error)); + return false; + } return true; } } diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 1c69145560de2..cf0f557fd9129 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -2713,11 +2713,14 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl( } } -void ValueObject::Dump(Stream &s) { Dump(s, DumpValueObjectOptions(*this)); } +llvm::Error ValueObject::Dump(Stream &s) { + return Dump(s, DumpValueObjectOptions(*this)); +} -void ValueObject::Dump(Stream &s, const DumpValueObjectOptions &options) { +llvm::Error ValueObject::Dump(Stream &s, + const DumpValueObjectOptions &options) { ValueObjectPrinter printer(*this, &s, options); - printer.PrintValueObject(); + return printer.PrintValueObject(); } ValueObjectSP ValueObject::CreateConstantValue(ConstString name) { diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp index c2933d8574583..a81a51720a495 100644 --- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp +++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp @@ -69,15 +69,13 @@ void ValueObjectPrinter::Init( SetupMostSpecializedValue(); } -bool ValueObjectPrinter::PrintValueObject() { +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 // the error and exit early. if (m_orig_valobj.GetError().Fail() && - !m_orig_valobj.GetCompilerType().IsValid()) { - m_stream->Printf("Error: '%s'", m_orig_valobj.GetError().AsCString()); - return true; - } + !m_orig_valobj.GetCompilerType().IsValid()) + return m_orig_valobj.GetError().ToError(); if (ShouldPrintValueObject()) { PrintLocationIfNeeded(); @@ -97,7 +95,7 @@ bool ValueObjectPrinter::PrintValueObject() { else m_stream->EOL(); - return true; + return llvm::Error::success(); } ValueObject &ValueObjectPrinter::GetMostSpecializedValue() { @@ -619,7 +617,13 @@ void ValueObjectPrinter::PrintChild( ValueObjectPrinter child_printer(*(child_sp.get()), m_stream, child_options, ptr_depth, m_curr_depth + 1, m_printed_instance_pointers); - child_printer.PrintValueObject(); + llvm::Error error = child_printer.PrintValueObject(); + if (error) { + if (m_stream) + *m_stream << "error: " << toString(std::move(error)); + else + llvm::consumeError(std::move(error)); + } } } diff --git a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp index 0aaddad53126e..0fb5490defc63 100644 --- a/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp +++ b/lldb/source/Plugins/REPL/Clang/ClangREPL.cpp @@ -95,7 +95,9 @@ bool ClangREPL::PrintOneVariable(Debugger &debugger, if (m_implicit_expr_result_regex.Execute(var->GetName().GetStringRef())) return true; } - valobj_sp->Dump(*output_sp); + if (llvm::Error error = valobj_sp->Dump(*output_sp)) + *output_sp << "error: " << toString(std::move(error)); + return true; } diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py index c650b1e3533e0..785a9621c99dc 100644 --- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py +++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py @@ -122,7 +122,7 @@ def test_nested_values(self): """Test dwim-print with nested values (structs, etc).""" self.build() lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.c") + self, "break here", lldb.SBFileSpec("main.c") ) self.runCmd("settings set auto-one-line-summaries false") self._expect_cmd(f"dwim-print s", "frame variable") @@ -132,7 +132,7 @@ def test_summary_strings(self): """Test dwim-print with nested values (structs, etc).""" self.build() lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.c") + self, "break here", lldb.SBFileSpec("main.c") ) self.runCmd("settings set auto-one-line-summaries false") self.runCmd("type summary add -e -s 'stub summary' Structure") @@ -143,7 +143,7 @@ def test_void_result(self): """Test dwim-print does not surface an error message for void expressions.""" self.build() lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.c") + self, "break here", lldb.SBFileSpec("main.c") ) self.expect("dwim-print (void)15", matching=False, patterns=["(?i)error"]) @@ -151,10 +151,18 @@ def test_preserves_persistent_variables(self): """Test dwim-print does not delete persistent variables.""" self.build() lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.c") + self, "break here", lldb.SBFileSpec("main.c") ) self.expect("dwim-print int $i = 15") # Run the same expression twice and verify success. This ensures the # first command does not delete the persistent variable. for _ in range(2): self.expect("dwim-print $i", startstr="(int) 15") + + def test_missing_type(self): + """The expected output of po opaque is its address (no error)""" + self.build() + lldbutil.run_to_source_breakpoint( + self, "break here", lldb.SBFileSpec("main.c") + ) + self.expect("dwim-print -O -- opaque", substrs=['0x']) diff --git a/lldb/test/API/commands/dwim-print/main.c b/lldb/test/API/commands/dwim-print/main.c index e2eccf3a88b4b..6bfe645c7c6e0 100644 --- a/lldb/test/API/commands/dwim-print/main.c +++ b/lldb/test/API/commands/dwim-print/main.c @@ -2,9 +2,13 @@ struct Structure { int number; }; +struct Opaque; +int puts(const char *s); + int main(int argc, char **argv) { struct Structure s; s.number = 30; - // break here + struct Opaque *opaque = &s; + puts("break here"); return 0; } diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s b/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s index c7aea06bf9098..4fbc1894c8c44 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/class-type-nullptr-deref.s @@ -4,7 +4,7 @@ # RUN: llvm-mc --triple x86_64-pc-linux %s --filetype=obj -o %t # RUN: %lldb %t -o "target variable x" -o exit 2>&1 | FileCheck %s -# CHECK: 'Unable to determine byte size.' +# CHECK: Unable to determine byte size. # This tests a fix for a crash. If things are working we don't get a segfault. diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s index 64b835353ed4e..64b22830e8f28 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s +++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-signature-loop.s @@ -4,7 +4,7 @@ # RUN: ld.lld %t.o -o %t # RUN: %lldb %t -o "target variable e" -b | FileCheck %s -# CHECK: Error: 'Unable to determine byte size.' +# CHECK: error: Unable to determine byte size. .type e,@object # @e .section .rodata,"a",@progbits diff --git a/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp b/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp index 31068a04d8dfe..6cb982d7f5980 100644 --- a/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp +++ b/lldb/unittests/ValueObject/DumpValueObjectOptionsTests.cpp @@ -98,9 +98,10 @@ class ValueObjectMockProcessTest : public ::testing::Test { ExecutionContextScope *exe_scope = m_exe_ctx.GetBestExecutionContextScope(); for (auto [value, options, expected] : tests) { Data... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/95857 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
