Remove unused and undocumented data_offset parameter

[aprantl]

Summary

Value::GetValueAsData() takes an undocumented parameter called data_offset that is always 0. I couldn't figure out what it is supposed to be used for and it is never used, so here's a patch to remove it.

[labath]

From the looks of it, this appears to be the offset in the data extractor that you want to start extracting from (so, e.g. if you have a large data extractor, and you know that the value you're interested in is at byte 47, you would say GetValueAsData(large_extractor, 47).

That said, I think it would be good to remove this, as it does not seem to be a common use case, and one can always create a sub-data extractor, if he really wants to start extracting from a random offset.

lldb/include/lldb/Core/Value.h

public:
 
   void Dump(Stream *strm);
 
   lldb::Format GetValueDefaultFormat();
 
   uint64_t GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx);
 
   Status GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data,
-                        uint32_t data_offset,
                         Module *module); // Can be nullptr
 
   static const char *GetValueTypeAsCString(ValueType context_type);
 
   static const char *GetContextTypeAsCString(ContextType context_type);
 
   /// Convert this value's file address to a load address, if possible.
   void ConvertToLoadAddress(Module *module, Target *target);

lldb/source/Core/Value.cpp

case eValueTypeHostAddress:
     }
     break;
   }
 
   return false;
 }
 
 Status Value::GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data,
-                             uint32_t data_offset, Module *module) {
+                             Module *module) {
   data.Clear();
 
   Status error;
   lldb::addr_t address = LLDB_INVALID_ADDRESS;
   AddressType address_type = eAddressTypeFile;
   Address file_so_addr;
   const CompilerType &ast_type = GetCompilerType();
   switch (m_value_type) {
if (error.Fail())
     return error;
 
   // No memory to read for zero-sized types.
   if (byte_size == 0)
     return error;
 
   // Make sure we have enough room within "data", and if we don't make
   // something large enough that does
-  if (!data.ValidOffsetForDataOfSize(data_offset, byte_size)) {
-    auto data_sp =
-        std::make_shared<DataBufferHeap>(data_offset + byte_size, '\0');
+  if (!data.ValidOffsetForDataOfSize(0, byte_size)) {
+    auto data_sp = std::make_shared<DataBufferHeap>(byte_size, '\0');
     data.SetData(data_sp);
   }
 
-  uint8_t *dst = const_cast<uint8_t *>(data.PeekData(data_offset, byte_size));
+  uint8_t *dst = const_cast<uint8_t *>(data.PeekData(0, byte_size));
   if (dst != nullptr) {
     if (address_type == eAddressTypeHost) {
       // The address is an address in this process, so just copy it.
       if (address == 0) {
         error.SetErrorStringWithFormat(
             "trying to read from host address of 0.");
         return error;
       }
if (compiler_type.IsValid()) {
     default:
     case eValueTypeFileAddress:
     case eValueTypeLoadAddress: // load address value
     case eValueTypeHostAddress: // host address value (for memory in the process
                                 // that is using liblldb)
     {
       DataExtractor data;
       lldb::addr_t addr = m_value.ULongLong(LLDB_INVALID_ADDRESS);
-      Status error(GetValueAsData(exe_ctx, data, 0, nullptr));
+      Status error(GetValueAsData(exe_ctx, data, nullptr));
       if (error.Success()) {
         Scalar scalar;
         if (compiler_type.GetValueAsScalar(data, 0, data.GetByteSize(),
                                            scalar)) {
           m_value = scalar;
           m_value_type = eValueTypeScalar;
         } else {
           if ((uintptr_t)addr != (uintptr_t)m_data_buffer.GetBytes()) {

lldb/source/Core/ValueObject.cpp

if (item_idx == 0 && item_count == 1) // simply a deref
     }
   }
   return 0;
 }
 
 uint64_t ValueObject::GetData(DataExtractor &data, Status &error) {
   UpdateValueIfNeeded(false);
   ExecutionContext exe_ctx(GetExecutionContextRef());
-  error = m_value.GetValueAsData(&exe_ctx, data, 0, GetModule().get());
+  error = m_value.GetValueAsData(&exe_ctx, data, GetModule().get());
   if (error.Fail()) {
     if (m_data.GetByteSize()) {
       data = m_data;
       error.Clear();
       return data.GetByteSize();
     } else {
       return 0;
     }
if (UpdateValueIfNeeded(false) && m_error.Success()) {
     ExecutionContext exe_ctx(GetExecutionContextRef());
 
     DataExtractor data;
     data.SetByteOrder(m_data.GetByteOrder());
     data.SetAddressByteSize(m_data.GetAddressByteSize());
 
     if (IsBitfield()) {
       Value v(Scalar(GetValueAsUnsigned(UINT64_MAX)));
-      m_error = v.GetValueAsData(&exe_ctx, data, 0, GetModule().get());
+      m_error = v.GetValueAsData(&exe_ctx, data, GetModule().get());
     } else
-      m_error = m_value.GetValueAsData(&exe_ctx, data, 0, GetModule().get());
+      m_error = m_value.GetValueAsData(&exe_ctx, data, GetModule().get());
 
     valobj_sp = ValueObjectConstResult::Create(
         exe_ctx.GetBestExecutionContextScope(), GetCompilerType(), name, data,
         GetAddressOf());
   }
 
   if (!valobj_sp) {
     ExecutionContext exe_ctx(GetExecutionContextRef());

lldb/source/Core/ValueObjectCast.cpp

if (m_parent->UpdateValueIfNeeded(false)) {
     if (!CanProvideValue()) {
       // this value object represents an aggregate type whose children have
       // values, but this object does not. So we say we are changed if our
       // location has changed.
       SetValueDidChange(m_value.GetValueType() != old_value.GetValueType() ||
                         m_value.GetScalar() != old_value.GetScalar());
     }
     ExecutionContext exe_ctx(GetExecutionContextRef());
-    m_error = m_value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+    m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
     SetValueDidChange(m_parent->GetValueDidChange());
     return true;
   }
 
   // The dynamic value failed to get an error, pass the error along
   if (m_error.Success() && m_parent->GetError().Fail())
     m_error = m_parent->GetError();
   SetValueIsValid(false);
   return false;
 }
 
 bool ValueObjectCast::IsInScope() { return m_parent->IsInScope(); }

lldb/source/Core/ValueObjectChild.cpp

if (parent->UpdateValueIfNeeded(false)) {
 
       if (m_error.Success()) {
         const bool thread_and_frame_only_if_stopped = true;
         ExecutionContext exe_ctx(
             GetExecutionContextRef().Lock(thread_and_frame_only_if_stopped));
         if (GetCompilerType().GetTypeInfo() & lldb::eTypeHasValue) {
           Value &value = is_instance_ptr_base ? m_parent->GetValue() : m_value;
           m_error =
-              value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+              value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
         } else {
           m_error.Clear(); // No value so nothing to read...
         }
       }
 
     } else {
       m_error.SetErrorStringWithFormat("parent failed to evaluate: %s",
                                        parent->GetError().AsCString());

lldb/source/Core/ValueObjectConstResult.cpp

ValueObjectConstResult::ValueObjectConstResult(ExecutionContextScope *exe_scope,
                                                const Value &value,
                                                ConstString name,
                                                Module *module)
     : ValueObject(exe_scope), m_type_name(), m_byte_size(0), m_impl(this) {
   m_value = value;
   m_name = name;
   ExecutionContext exe_ctx;
   exe_scope->CalculateExecutionContext(exe_ctx);
-  m_error = m_value.GetValueAsData(&exe_ctx, m_data, 0, module);
+  m_error = m_value.GetValueAsData(&exe_ctx, m_data, module);
 }
 
 ValueObjectConstResult::~ValueObjectConstResult() {}
 
 CompilerType ValueObjectConstResult::GetCompilerTypeImpl() {
   return m_value.GetCompilerType();
 }
 

lldb/source/Core/ValueObjectDynamicValue.cpp

bool ValueObjectDynamicValue::UpdateValue() {
   // parent. Or we could return false, and make ourselves an echo of our
   // parent?
   if (!found_dynamic_type) {
     if (m_dynamic_type_info)
       SetValueDidChange(true);
     ClearDynamicTypeInformation();
     m_dynamic_type_info.Clear();
     m_value = m_parent->GetValue();
-    m_error = m_value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+    m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
     return m_error.Success();
   }
 
   Value old_value(m_value);
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES));
 
   bool has_changed_type = false;
bool ValueObjectDynamicValue::UpdateValue() {
 
   if (has_changed_type && log)
     LLDB_LOGF(log, "[%s %p] has a new dynamic type %s", GetName().GetCString(),
               static_cast<void *>(this), GetTypeName().GetCString());
 
   if (m_address.IsValid() && m_dynamic_type_info) {
     // The variable value is in the Scalar value inside the m_value. We can
     // point our m_data right to it.
-    m_error = m_value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+    m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
     if (m_error.Success()) {
       if (!CanProvideValue()) {
         // this value object represents an aggregate type whose children have
         // values, but this object does not. So we say we are changed if our
         // location has changed.
         SetValueDidChange(m_value.GetValueType() != old_value.GetValueType() ||
                           m_value.GetScalar() != old_value.GetScalar());
       }

lldb/source/Core/ValueObjectMemory.cpp

if (m_address.IsValid()) {
 
     switch (value_type) {
     default:
       llvm_unreachable("Unhandled expression result value kind...");
 
     case Value::eValueTypeScalar:
       // The variable value is in the Scalar value inside the m_value. We can
       // point our m_data right to it.
-      m_error = m_value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+      m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
       break;
 
     case Value::eValueTypeFileAddress:
     case Value::eValueTypeLoadAddress:
     case Value::eValueTypeHostAddress:
       // The DWARF expression result was an address in the inferior process. If
       // this variable is an aggregate type, we just need the address as the
       // main value as all child variable objects will rely upon this location
case Value::eValueTypeHostAddress:
         if (m_type_sp)
           value.SetContext(Value::eContextTypeLLDBType, m_type_sp.get());
         else {
           // value.SetContext(Value::eContextTypeClangType,
           // m_compiler_type.GetOpaqueQualType());
           value.SetCompilerType(m_compiler_type);
         }
 
-        m_error = value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+        m_error = value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
       }
       break;
     }
 
     SetValueIsValid(m_error.Success());
   }
   return m_error.Success();
 }
 
 bool ValueObjectMemory::IsInScope() {
   // FIXME: Maybe try to read the memory address, and if that works, then
   // we are in scope?
   return true;
 }
 
 lldb::ModuleSP ValueObjectMemory::GetModule() { return m_address.GetModule(); }

lldb/source/Core/ValueObjectSyntheticFilter.cpp

 
 lldb::ValueObjectSP ValueObjectSynthetic::GetNonSyntheticValue() {
   return m_parent->GetSP();
 }
 
 void ValueObjectSynthetic::CopyValueData(ValueObject *source) {
   m_value = (source->UpdateValueIfNeeded(), source->GetValue());
   ExecutionContext exe_ctx(GetExecutionContextRef());
-  m_error = m_value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+  m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
 }
 
 bool ValueObjectSynthetic::CanProvideValue() {
   if (!UpdateValueIfNeeded())
     return false;
   if (m_provides_value == eLazyBoolYes)
     return true;
   return m_parent->CanProvideValue();

lldb/source/Core/ValueObjectVariable.cpp

if (expr.Evaluate(&exe_ctx, nullptr, loclist_base_load_addr, nullptr,
 
       switch (value_type) {
       case Value::eValueTypeVector:
       // fall through
       case Value::eValueTypeScalar:
         // The variable value is in the Scalar value inside the m_value. We can
         // point our m_data right to it.
         m_error =
-            m_value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+            m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
         break;
 
       case Value::eValueTypeFileAddress:
       case Value::eValueTypeLoadAddress:
       case Value::eValueTypeHostAddress:
         // The DWARF expression result was an address in the inferior process.
         // If this variable is an aggregate type, we just need the address as
         // the main value as all child variable objects will rely upon this
if (expr.Evaluate(&exe_ctx, nullptr, loclist_base_load_addr, nullptr,
           SetValueDidChange(value_type != old_value.GetValueType() ||
                             m_value.GetScalar() != old_value.GetScalar());
         } else {
           // Copy the Value and set the context to use our Variable so it can
           // extract read its value into m_data appropriately
           Value value(m_value);
           value.SetContext(Value::eContextTypeVariable, variable);
           m_error =
-              value.GetValueAsData(&exe_ctx, m_data, 0, GetModule().get());
+              value.GetValueAsData(&exe_ctx, m_data, GetModule().get());
 
           SetValueDidChange(value_type != old_value.GetValueType() ||
                             m_value.GetScalar() != old_value.GetScalar());
         }
         break;
       }
 
       SetValueIsValid(m_error.Success());

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

bool success =
         abi->GetArgumentValues(exe_ctx.GetThreadRef(), argument_values);
     if (!success)
       return false;
 
     // Now get a pointer value from the zeroth argument.
     Status error;
     DataExtractor data;
     error = argument_values.GetValueAtIndex(0)->GetValueAsData(&exe_ctx, data,
-                                                               0, nullptr);
+                                                               nullptr);
     lldb::offset_t offset = 0;
     lldb::addr_t region_addr = data.GetPointer(&offset);
 
     if (region_addr != 0)
       vtable_handler->ReadRegions(region_addr);
   }
   return false;
 }