Skip to content

[lldb][NFC] Replace GetLocalBufferSize() with GetLocalBuffer() #126333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 8, 2025

Conversation

augusto2112
Copy link
Contributor

GetLocalBuffer() makes more sense as an API.

GetLocalBuffer() makes more sense as an API.
@llvmbot
Copy link
Member

llvmbot commented Feb 8, 2025

@llvm/pr-subscribers-lldb

Author: Augusto Noronha (augusto2112)

Changes

GetLocalBuffer() makes more sense as an API.


Full diff: https://github.com/llvm/llvm-project/pull/126333.diff

4 Files Affected:

  • (modified) lldb/include/lldb/ValueObject/ValueObject.h (+9-8)
  • (modified) lldb/source/ValueObject/ValueObject.cpp (+6-6)
  • (modified) lldb/source/ValueObject/ValueObjectDynamicValue.cpp (+1-1)
  • (modified) lldb/unittests/ValueObject/DynamicValueObjectLocalBuffer.cpp (+1-4)
diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h
index c8d5c2723106d6d..a0f53d20327cdc5 100644
--- a/lldb/include/lldb/ValueObject/ValueObject.h
+++ b/lldb/include/lldb/ValueObject/ValueObject.h
@@ -865,17 +865,18 @@ class ValueObject {
 
   virtual void SetLanguageFlags(uint64_t flags) { m_language_flags = flags; }
 
-  /// Returns the size of the local buffer if it's available.
+  /// Returns the local buffer that this ValueObject points to if it's
+  /// available.
   /// \return
-  ///     The size of the local buffer if this value object's value points to a
-  ///     host address, and if that size can be determined. Otherwise, returns
-  ///     LLDB_INVALID_ADDRESS.
+  ///     The local buffer if this value object's value points to a
+  ///     host address, and if that buffer can be determined. Otherwise, returns
+  ///     an empty ArrayRef.
   ///
   /// TODO: Because a ValueObject's Value can point to any arbitrary memory
-  /// location, it is possible that the size of the local buffer can't be
-  /// determined at all. See the comment in Value::m_value for a more thorough
-  /// explanation of why that is.
-  uint64_t GetLocalBufferSize();
+  /// location, it is possible that we can't find what what buffer we're
+  /// pointing to, and thus also can't know its size. See the comment in
+  /// Value::m_value for a more thorough explanation of why that is.
+  llvm::ArrayRef<uint8_t> GetLocalBuffer() const;
 
 protected:
   typedef ClusterManager<ValueObject> ValueObjectManager;
diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index 551d882a48d40f6..9d98f62c0379b65 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -849,20 +849,20 @@ bool ValueObject::SetData(DataExtractor &data, Status &error) {
   return true;
 }
 
-uint64_t ValueObject::GetLocalBufferSize() {
+llvm::ArrayRef<uint8_t> ValueObject::GetLocalBuffer() const {
   if (m_value.GetValueType() != Value::ValueType::HostAddress)
-    return LLDB_INVALID_ADDRESS;
+    return {};
   auto start = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
   if (start == LLDB_INVALID_ADDRESS)
-    return LLDB_INVALID_ADDRESS;
+    return {};
   // Does our pointer point to this value object's m_data buffer?
   if ((uint64_t)m_data.GetDataStart() == start)
-    return m_data.GetByteSize();
+    return m_data.GetData();
   // Does our pointer point to the value's buffer?
   if ((uint64_t)m_value.GetBuffer().GetBytes() == start)
-    return m_value.GetBuffer().GetByteSize();
+    return m_value.GetBuffer().GetData();
   // Our pointer points to something else. We can't know what the size is.
-  return LLDB_INVALID_ADDRESS;
+  return {};
 }
 
 static bool CopyStringDataToBufferSP(const StreamString &source,
diff --git a/lldb/source/ValueObject/ValueObjectDynamicValue.cpp b/lldb/source/ValueObject/ValueObjectDynamicValue.cpp
index dddb0f0700b38a4..ecd663af68c2dbe 100644
--- a/lldb/source/ValueObject/ValueObjectDynamicValue.cpp
+++ b/lldb/source/ValueObject/ValueObjectDynamicValue.cpp
@@ -241,7 +241,7 @@ bool ValueObjectDynamicValue::UpdateValue() {
       SetValueDidChange(true);
 
     // If we found a host address, and the dynamic type fits in the local buffer
-    // that was found, point to thar buffer. Later on this function will copy
+    // that was found, point to that buffer. Later on this function will copy
     // the buffer over.
     if (value_type == Value::ValueType::HostAddress && !local_buffer.empty()) {
       auto *exe_scope = exe_ctx.GetBestExecutionContextScope();
diff --git a/lldb/unittests/ValueObject/DynamicValueObjectLocalBuffer.cpp b/lldb/unittests/ValueObject/DynamicValueObjectLocalBuffer.cpp
index e3cf0f8a87bd2a3..417708dd2dc226c 100644
--- a/lldb/unittests/ValueObject/DynamicValueObjectLocalBuffer.cpp
+++ b/lldb/unittests/ValueObject/DynamicValueObjectLocalBuffer.cpp
@@ -66,11 +66,8 @@ struct MockLanguageRuntime : public LanguageRuntime {
         *ast, "TypeWitInt", ast->GetBasicType(lldb::BasicType::eBasicTypeInt),
         "theIntField", LanguageType::eLanguageTypeC_plus_plus);
     class_type_or_name.SetCompilerType(int_type);
-    local_buffer = {(uint8_t *)in_value.GetValue().GetScalar().ULongLong(
-                        LLDB_INVALID_ADDRESS),
-                    static_cast<size_t>(in_value.GetLocalBufferSize())};
+    local_buffer = in_value.GetLocalBuffer();
     value_type = Value::ValueType::HostAddress;
-
     return true;
   }
 

@augusto2112 augusto2112 merged commit 9d5edc9 into llvm:main Feb 8, 2025
9 checks passed
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants