[Lldb-commits] [lldb] Convert ValueObject::Dump() to return llvm::Error() (NFCish) (PR #95857)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 17 15:14:59 PDT 2024
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, at object # @e
.section .rodata,"a", at 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
More information about the lldb-commits
mailing list