[Lldb-commits] [lldb] draft: [lldb] Upgrade ValueObject::GetData to return llvm::Expected (PR #130516)
Julius Alexandre via lldb-commits
lldb-commits at lists.llvm.org
Sun Mar 9 15:10:42 PDT 2025
https://github.com/wizardengineer updated https://github.com/llvm/llvm-project/pull/130516
>From 161bdb32b284d2370b138e72a8a1ad560b258ba9 Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Sun, 9 Mar 2025 16:20:47 -0400
Subject: [PATCH 1/4] Change ValueObject::GetData to return llvm::Expected
---
lldb/include/lldb/ValueObject/ValueObject.h | 2 +-
.../AppleObjCRuntime/AppleObjCRuntime.cpp | 5 ++--
.../TypeSystem/Clang/TypeSystemClang.cpp | 12 ++++----
lldb/source/ValueObject/ValueObject.cpp | 28 +++++++++++++------
4 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h
index 06d2589002ed0..2ee7f99718416 100644
--- a/lldb/include/lldb/ValueObject/ValueObject.h
+++ b/lldb/include/lldb/ValueObject/ValueObject.h
@@ -757,7 +757,7 @@ class ValueObject {
virtual size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
uint32_t item_count = 1);
- virtual uint64_t GetData(DataExtractor &data, Status &error);
+ virtual llvm::Expected<DataExtractor> GetData();
virtual bool SetData(DataExtractor &data, Status &error);
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index ad60290382c02..69856d4592843 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -551,9 +551,8 @@ ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
DataExtractor data;
data.SetAddressByteSize(dict_entry->GetProcessSP()->GetAddressByteSize());
- Status error;
- dict_entry->GetData(data, error);
- if (error.Fail()) return ThreadSP();
+ auto data_or_err = dict_entry->GetData();
+ if (!data_or_err) return ThreadSP();
lldb::offset_t data_offset = 0;
auto dict_entry_key = data.GetAddress(&data_offset);
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 4ca4752310868..763a80faa914a 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -247,13 +247,15 @@ static lldb::addr_t GetVTableAddress(Process &process,
// We have an object already read from process memory,
// so just extract VTable pointer from it
- DataExtractor data;
- Status err;
- auto size = valobj.GetData(data, err);
- if (err.Fail() || vbtable_ptr_offset + data.GetAddressByteSize() > size)
+ auto data_or_err = valobj.GetData();
+ if (!data_or_err)
+ return LLDB_INVALID_ADDRESS;
+
+ auto size = data_or_err->GetByteSize();
+ if (vbtable_ptr_offset + data_or_err->GetAddressByteSize() > size)
return LLDB_INVALID_ADDRESS;
- return data.GetAddress(&vbtable_ptr_offset);
+ return data_or_err->GetAddress(&vbtable_ptr_offset);
}
static int64_t ReadVBaseOffsetFromVTable(Process &process,
diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index eac24353de90b..05cbc5489d25e 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -691,13 +691,20 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
ValueObjectSP pointee_sp = Dereference(error);
if (error.Fail() || pointee_sp.get() == nullptr)
return 0;
- return pointee_sp->GetData(data, error);
+ auto data_or_err = pointee_sp->GetData();
+ if (!data_or_err)
+ return 0;
+ data = *data_or_err;
+ return data.GetByteSize();
} else {
ValueObjectSP child_sp = GetChildAtIndex(0);
if (child_sp.get() == nullptr)
return 0;
- Status error;
- return child_sp->GetData(data, error);
+ auto data_or_err = child_sp->GetData();
+ if (!data_or_err)
+ return 0;
+ data = *data_or_err;
+ return data.GetByteSize();
}
return true;
} else /* (items > 1) */
@@ -764,22 +771,27 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
return 0;
}
-uint64_t ValueObject::GetData(DataExtractor &data, Status &error) {
+llvm::Expected<DataExtractor> ValueObject::GetData() {
UpdateValueIfNeeded(false);
ExecutionContext exe_ctx(GetExecutionContextRef());
- error = m_value.GetValueAsData(&exe_ctx, data, GetModule().get());
+ DataExtractor data;
+ Status error = m_value.GetValueAsData(&exe_ctx, data, GetModule().get());
if (error.Fail()) {
if (m_data.GetByteSize()) {
data = m_data;
error.Clear();
- return data.GetByteSize();
+ data.SetAddressByteSize(m_data.GetAddressByteSize());
+ data.SetByteOrder(m_data.GetByteOrder());
+ return data;
} else {
- return 0;
+ return llvm::createStringError(
+ "GetData failed: %s",
+ error.AsCString());
}
}
data.SetAddressByteSize(m_data.GetAddressByteSize());
data.SetByteOrder(m_data.GetByteOrder());
- return data.GetByteSize();
+ return data;
}
bool ValueObject::SetData(DataExtractor &data, Status &error) {
>From 7d886e2bee159d6bbe00cec9fd4004f4d71993bf Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Sun, 9 Mar 2025 16:43:56 -0400
Subject: [PATCH 2/4] clang format
---
.../ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp | 3 ++-
lldb/source/ValueObject/ValueObject.cpp | 4 +---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index 69856d4592843..648e061f3e3c5 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -552,7 +552,8 @@ ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
DataExtractor data;
data.SetAddressByteSize(dict_entry->GetProcessSP()->GetAddressByteSize());
auto data_or_err = dict_entry->GetData();
- if (!data_or_err) return ThreadSP();
+ if (!data_or_err)
+ return ThreadSP();
lldb::offset_t data_offset = 0;
auto dict_entry_key = data.GetAddress(&data_offset);
diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index 05cbc5489d25e..4d13e460a2ca7 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -784,9 +784,7 @@ llvm::Expected<DataExtractor> ValueObject::GetData() {
data.SetByteOrder(m_data.GetByteOrder());
return data;
} else {
- return llvm::createStringError(
- "GetData failed: %s",
- error.AsCString());
+ return llvm::createStringError("GetData failed: %s", error.AsCString());
}
}
data.SetAddressByteSize(m_data.GetAddressByteSize());
>From c6d271b8b3e966c3a0f8548b2362668b05fc6f33 Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Sun, 9 Mar 2025 17:17:50 -0400
Subject: [PATCH 3/4] more changes
---
.../Language/CPlusPlus/CxxStringTypes.cpp | 16 ++++++----------
.../Plugins/Language/CPlusPlus/LibCxxList.cpp | 16 ++++++----------
lldb/source/Plugins/Language/ObjC/Cocoa.cpp | 8 +++-----
lldb/source/Plugins/Language/ObjC/NSString.cpp | 8 +++-----
.../ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp | 7 +++----
5 files changed, 21 insertions(+), 34 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
index fc17b76804d9f..c7d4b650d7819 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
@@ -68,11 +68,9 @@ static bool CharStringSummaryProvider(ValueObject &valobj, Stream &stream) {
template <StringElementType ElemType>
static bool CharSummaryProvider(ValueObject &valobj, Stream &stream) {
- DataExtractor data;
- Status error;
- valobj.GetData(data, error);
+ auto data_or_err = valobj.GetData();
- if (error.Fail())
+ if (!data_or_err)
return false;
std::string value;
@@ -84,7 +82,7 @@ static bool CharSummaryProvider(ValueObject &valobj, Stream &stream) {
if (!value.empty())
stream.Printf("%s ", value.c_str());
- options.SetData(std::move(data));
+ options.SetData(std::move(*data_or_err));
options.SetStream(&stream);
options.SetPrefixToken(ElemTraits.first);
options.SetQuote('\'');
@@ -169,11 +167,9 @@ bool lldb_private::formatters::Char32SummaryProvider(
bool lldb_private::formatters::WCharSummaryProvider(
ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
- DataExtractor data;
- Status error;
- valobj.GetData(data, error);
+ auto data_or_err = valobj.GetData();
- if (error.Fail())
+ if (!data_or_err)
return false;
// Get a wchar_t basic type from the current type system
@@ -191,7 +187,7 @@ bool lldb_private::formatters::WCharSummaryProvider(
const uint32_t wchar_size = *size;
StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
- options.SetData(std::move(data));
+ options.SetData(std::move(*data_or_err));
options.SetStream(&stream);
options.SetPrefixToken("L");
options.SetQuote('\'');
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
index ae1ad2bfe7200..12a5ff39382ca 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -273,13 +273,11 @@ ValueObjectSP ForwardListFrontEnd::GetChildAtIndex(uint32_t idx) {
// we need to copy current_sp into a new object otherwise we will end up with
// all items named __value_
- DataExtractor data;
- Status error;
- current_sp->GetData(data, error);
- if (error.Fail())
+ auto data_or_err = current_sp->GetData();
+ if (!data_or_err)
return nullptr;
- return CreateValueObjectFromData(llvm::formatv("[{0}]", idx).str(), data,
+ return CreateValueObjectFromData(llvm::formatv("[{0}]", idx).str(), *data_or_err,
m_backend.GetExecutionContextRef(),
m_element_type);
}
@@ -394,15 +392,13 @@ lldb::ValueObjectSP ListFrontEnd::GetChildAtIndex(uint32_t idx) {
// we need to copy current_sp into a new object otherwise we will end up with
// all items named __value_
- DataExtractor data;
- Status error;
- current_sp->GetData(data, error);
- if (error.Fail())
+ auto data_or_err = current_sp->GetData();
+ if (!data_or_err)
return lldb::ValueObjectSP();
StreamString name;
name.Printf("[%" PRIu64 "]", (uint64_t)idx);
- return CreateValueObjectFromData(name.GetString(), data,
+ return CreateValueObjectFromData(name.GetString(), *data_or_err,
m_backend.GetExecutionContextRef(),
m_element_type);
}
diff --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index 1d79edbede5d6..8c2582355e524 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -1205,13 +1205,11 @@ bool lldb_private::formatters::ObjCSELSummaryProvider(
valobj_sp = ValueObject::CreateValueObjectFromAddress("text", data_address,
exe_ctx, charstar);
} else {
- DataExtractor data;
- Status error;
- valobj.GetData(data, error);
- if (error.Fail())
+ auto data_or_err = valobj.GetData();
+ if (!data_or_err)
return false;
valobj_sp =
- ValueObject::CreateValueObjectFromData("text", data, exe_ctx, charstar);
+ ValueObject::CreateValueObjectFromData("text", *data_or_err, exe_ctx, charstar);
}
if (!valobj_sp)
diff --git a/lldb/source/Plugins/Language/ObjC/NSString.cpp b/lldb/source/Plugins/Language/ObjC/NSString.cpp
index a99d042572bfe..5b6a22c6e6b6c 100644
--- a/lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -291,13 +291,11 @@ bool lldb_private::formatters::NSAttributedStringSummaryProvider(
"string_ptr", pointer_value, exe_ctx, type));
if (!child_ptr_sp)
return false;
- DataExtractor data;
- Status error;
- child_ptr_sp->GetData(data, error);
- if (error.Fail())
+ auto data_or_err = child_ptr_sp->GetData();
+ if (!data_or_err)
return false;
ValueObjectSP child_sp(child_ptr_sp->CreateValueObjectFromData(
- "string_data", data, exe_ctx, type));
+ "string_data", *data_or_err, exe_ctx, type));
child_sp->GetValueAsUnsigned(0);
if (child_sp)
return NSStringSummaryProvider(*child_sp, stream, options);
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index 648e061f3e3c5..b82bdf52e186e 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -549,15 +549,14 @@ ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
idx++) {
ValueObjectSP dict_entry = reserved_dict->GetChildAtIndex(idx);
- DataExtractor data;
- data.SetAddressByteSize(dict_entry->GetProcessSP()->GetAddressByteSize());
auto data_or_err = dict_entry->GetData();
if (!data_or_err)
return ThreadSP();
+ data_or_err->SetAddressByteSize(dict_entry->GetProcessSP()->GetAddressByteSize());
lldb::offset_t data_offset = 0;
- auto dict_entry_key = data.GetAddress(&data_offset);
- auto dict_entry_value = data.GetAddress(&data_offset);
+ auto dict_entry_key = data_or_err->GetAddress(&data_offset);
+ auto dict_entry_value = data_or_err->GetAddress(&data_offset);
auto key_nsstring = objc_object_from_address(dict_entry_key, "key");
StreamString key_summary;
>From ac8dacc3b351dafb65987e67711a689970af4a4d Mon Sep 17 00:00:00 2001
From: medievalghoul <61852278+medievalghoul at users.noreply.github.com>
Date: Sun, 9 Mar 2025 18:10:30 -0400
Subject: [PATCH 4/4] format
---
lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp | 6 +++---
lldb/source/Plugins/Language/ObjC/Cocoa.cpp | 4 ++--
.../ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp | 3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
index 12a5ff39382ca..4e2775c5ca266 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -277,9 +277,9 @@ ValueObjectSP ForwardListFrontEnd::GetChildAtIndex(uint32_t idx) {
if (!data_or_err)
return nullptr;
- return CreateValueObjectFromData(llvm::formatv("[{0}]", idx).str(), *data_or_err,
- m_backend.GetExecutionContextRef(),
- m_element_type);
+ return CreateValueObjectFromData(
+ llvm::formatv("[{0}]", idx).str(), *data_or_err,
+ m_backend.GetExecutionContextRef(), m_element_type);
}
lldb::ChildCacheState ForwardListFrontEnd::Update() {
diff --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index 8c2582355e524..ebb36b52af0dd 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -1208,8 +1208,8 @@ bool lldb_private::formatters::ObjCSELSummaryProvider(
auto data_or_err = valobj.GetData();
if (!data_or_err)
return false;
- valobj_sp =
- ValueObject::CreateValueObjectFromData("text", *data_or_err, exe_ctx, charstar);
+ valobj_sp = ValueObject::CreateValueObjectFromData("text", *data_or_err,
+ exe_ctx, charstar);
}
if (!valobj_sp)
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index b82bdf52e186e..c35e29773ecfe 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -552,7 +552,8 @@ ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
auto data_or_err = dict_entry->GetData();
if (!data_or_err)
return ThreadSP();
- data_or_err->SetAddressByteSize(dict_entry->GetProcessSP()->GetAddressByteSize());
+ data_or_err->SetAddressByteSize(
+ dict_entry->GetProcessSP()->GetAddressByteSize());
lldb::offset_t data_offset = 0;
auto dict_entry_key = data_or_err->GetAddress(&data_offset);
More information about the lldb-commits
mailing list