draft: [lldb] Upgrade ValueObject::GetData to return llvm::Expected

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);
 

lldb/source/DataFormatters/TypeFormat.cpp

@@ -46,19 +46,27 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
     Value &value(valobj->GetValue());
     const Value::ContextType context_type = value.GetContextType();
     ExecutionContext exe_ctx(valobj->GetExecutionContextRef());
-    DataExtractor data;
 
+    auto data_or_err = valobj->GetData();
+    if (!data_or_err) {
+      LLDB_LOG_ERRORV(GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                      "Failed to extract data: {0}");
+      return false;
+    }
     if (context_type == Value::ContextType::RegisterInfo) {
       const RegisterInfo *reg_info = value.GetRegisterInfo();
       if (reg_info) {
-        Status error;
-        valobj->GetData(data, error);
-        if (error.Fail())
+        auto data_or_err = valobj->GetData();
+        if (!data_or_err) {
+          LLDB_LOG_ERRORV(GetLog(LLDBLog::Process), data_or_err.takeError(),
+                          "Failed to extract data for register info: {0}");
           return false;
+        }
 
         StreamString reg_sstr;
-        DumpDataExtractor(data, &reg_sstr, 0, GetFormat(), reg_info->byte_size,
-                          1, UINT32_MAX, LLDB_INVALID_ADDRESS, 0, 0,
+        DumpDataExtractor(*data_or_err, &reg_sstr, 0, GetFormat(),
+                          reg_info->byte_size, 1, UINT32_MAX,
+                          LLDB_INVALID_ADDRESS, 0, 0,
                           exe_ctx.GetBestExecutionContextScope());
         dest = std::string(reg_sstr.GetString());
       }
@@ -84,14 +92,16 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
               target_sp->ReadCStringFromMemory(
                   address, (char *)buffer_sp->GetBytes(), max_len, error);
               if (error.Success())
-                data.SetData(buffer_sp);
+                data_or_err->SetData(buffer_sp);
             }
           }
         } else {
-          Status error;
-          valobj->GetData(data, error);
-          if (error.Fail())
-            return false;
+          auto data_or_err = valobj->GetData();
+          if (!data_or_err)
+            LLDB_LOG_ERRORV(
+                GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                "Failed to extract data for CString info to display: {0}:");
+          return false;
         }
 
         ExecutionContextScope *exe_scope =
@@ -107,7 +117,7 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
         compiler_type.DumpTypeValue(
             &sstr,                          // The stream to use for display
             GetFormat(),                    // Format to display this type with
-            data,                           // Data to extract from
+            *data_or_err,                   // Data to extract from
             0,                              // Byte offset into "m_data"
             *size_or_err,                   // Byte size of item in "m_data"
             valobj->GetBitfieldBitSize(),   // Bitfield bit size
@@ -184,15 +194,16 @@ bool TypeFormatImpl_EnumType::FormatObject(ValueObject *valobj,
     valobj_enum_type = iter->second;
   if (!valobj_enum_type.IsValid())
     return false;
-  DataExtractor data;
-  Status error;
-  valobj->GetData(data, error);
-  if (error.Fail())
+  auto data_or_err = valobj->GetData();
+  if (!data_or_err) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                    "Can't extract data related to enum type info: {0}");
     return false;
+  }
   ExecutionContext exe_ctx(valobj->GetExecutionContextRef());
   StreamString sstr;
-  valobj_enum_type.DumpTypeValue(&sstr, lldb::eFormatEnum, data, 0,
-                                 data.GetByteSize(), 0, 0,
+  valobj_enum_type.DumpTypeValue(&sstr, lldb::eFormatEnum, *data_or_err, 0,
+                                 data_or_err->GetByteSize(), 0, 0,
                                  exe_ctx.GetBestExecutionContextScope());
   if (!sstr.GetString().empty())
     dest = std::string(sstr.GetString());

lldb/source/Expression/Materializer.cpp

@@ -483,19 +483,18 @@ class EntityVariableBase : public Materializer::Entity {
     }
 
     if (m_is_reference) {
-      DataExtractor valobj_extractor;
-      Status extract_error;
-      valobj_sp->GetData(valobj_extractor, extract_error);
+      auto valobj_extractor_or_err = valobj_sp->GetData();
 
-      if (!extract_error.Success()) {
+      if (auto error = valobj_extractor_or_err.takeError()) {
         err = Status::FromErrorStringWithFormat(
             "couldn't read contents of reference variable %s: %s",
-            GetName().AsCString(), extract_error.AsCString());
+            GetName().AsCString(), llvm::toString(std::move(error)).c_str());
         return;
       }
 
       lldb::offset_t offset = 0;
-      lldb::addr_t reference_addr = valobj_extractor.GetAddress(&offset);
+      lldb::addr_t reference_addr =
+          valobj_extractor_or_err->GetAddress(&offset);
 
       Status write_error;
       map.WritePointerToMemory(load_addr, reference_addr, write_error);
@@ -523,13 +522,11 @@ class EntityVariableBase : public Materializer::Entity {
           return;
         }
       } else {
-        DataExtractor data;
-        Status extract_error;
-        valobj_sp->GetData(data, extract_error);
-        if (!extract_error.Success()) {
+        auto data_or_err = valobj_sp->GetData();
+        if (auto error = data_or_err.takeError()) {
           err = Status::FromErrorStringWithFormat(
               "couldn't get the value of %s: %s", GetName().AsCString(),
-              extract_error.AsCString());
+              llvm::toString(std::move(error)).c_str());
           return;
         }
 
@@ -540,9 +537,9 @@ class EntityVariableBase : public Materializer::Entity {
           return;
         }
 
-        if (data.GetByteSize() <
+        if (data_or_err->GetByteSize() <
             llvm::expectedToOptional(GetByteSize(scope)).value_or(0)) {
-          if (data.GetByteSize() == 0 && !LocationExpressionIsValid()) {
+          if (data_or_err->GetByteSize() == 0 && !LocationExpressionIsValid()) {
             err = Status::FromErrorStringWithFormat(
                 "the variable '%s' has no location, "
                 "it may have been optimized out",
@@ -553,7 +550,7 @@ class EntityVariableBase : public Materializer::Entity {
                 ") is larger than the ValueObject's size (%" PRIu64 ")",
                 GetName().AsCString(),
                 llvm::expectedToOptional(GetByteSize(scope)).value_or(0),
-                data.GetByteSize());
+                data_or_err->GetByteSize());
           }
           return;
         }
@@ -571,14 +568,14 @@ class EntityVariableBase : public Materializer::Entity {
         const bool zero_memory = false;
 
         m_temporary_allocation = map.Malloc(
-            data.GetByteSize(), byte_align,
+            data_or_err->GetByteSize(), byte_align,
             lldb::ePermissionsReadable | lldb::ePermissionsWritable,
             IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
 
-        m_temporary_allocation_size = data.GetByteSize();
+        m_temporary_allocation_size = data_or_err->GetByteSize();
 
-        m_original_data = std::make_shared<DataBufferHeap>(data.GetDataStart(),
-                                                           data.GetByteSize());
+        m_original_data = std::make_shared<DataBufferHeap>(
+            data_or_err->GetDataStart(), data_or_err->GetByteSize());
 
         if (!alloc_error.Success()) {
           err = Status::FromErrorStringWithFormat(
@@ -589,8 +586,8 @@ class EntityVariableBase : public Materializer::Entity {
 
         Status write_error;
 
-        map.WriteMemory(m_temporary_allocation, data.GetDataStart(),
-                        data.GetByteSize(), write_error);
+        map.WriteMemory(m_temporary_allocation, data_or_err->GetDataStart(),
+                        data_or_err->GetByteSize(), write_error);
 
         if (!write_error.Success()) {
           err = Status::FromErrorStringWithFormat(

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

@@ -68,12 +68,13 @@ 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) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+                    "Cannot extract data: {0}");
     return false;
+  }
 
   std::string value;
   StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
@@ -84,7 +85,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,12 +170,13 @@ 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) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::DataFormatters), data_or_err.takeError(),
+        "Can't extract data for WChar Summary: {0}")
     return false;
+  }
 
   // Get a wchar_t basic type from the current type system
   CompilerType wchar_compiler_type =
@@ -191,7 +193,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('\'');

lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp

@@ -273,15 +273,13 @@ 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,
-                                   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() {
@@ -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);
 }

lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp

@@ -207,15 +207,13 @@ lldb::ValueObjectSP lldb_private::formatters::
     return lldb::ValueObjectSP();
   StreamString stream;
   stream.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  DataExtractor data;
-  Status error;
-  val_hash.first->GetData(data, error);
-  if (error.Fail())
+  auto data_or_err = val_hash.first->GetData();
+  if (!data_or_err)
     return lldb::ValueObjectSP();
   const bool thread_and_frame_only_if_stopped = true;
   ExecutionContext exe_ctx = val_hash.first->GetExecutionContextRef().Lock(
       thread_and_frame_only_if_stopped);
-  return CreateValueObjectFromData(stream.GetString(), data, exe_ctx,
+  return CreateValueObjectFromData(stream.GetString(), *data_or_err, exe_ctx,
                                    m_element_type);
 }
 

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);
+    valobj_sp = ValueObject::CreateValueObjectFromData("text", *data_or_err,
+                                                       exe_ctx, charstar);
   }
 
   if (!valobj_sp)

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);

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp

@@ -549,15 +549,15 @@ ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
        idx++) {
     ValueObjectSP dict_entry = reserved_dict->GetChildAtIndex(idx);
 
-    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();
+    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;

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

@@ -247,13 +247,18 @@ 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) {
+    LLDB_LOG_ERRORV(GetLog(LLDBLog::Types), data_or_err.takeError(),
+                    "Extracted data is an invalid address: {0}");
+    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,