[Lldb-commits] [lldb] [lldb] Refactor away UB in SBValue::GetLoadAddress (PR #141799)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed May 28 09:34:42 PDT 2025


https://github.com/labath created https://github.com/llvm/llvm-project/pull/141799

The problem was in calling GetLoadAddress on a value in the error state, where `ValueObject::GetLoadAddress` could end up accessing the uninitialized "address type" by-ref return value from `GetAddressOf`. This probably happened because each function expected the other to initialize it.

We can guarantee initialization by turning this into a proper return value.

I've added a test, but it only (reliably) crashes if lldb is built with ubsan.

>From d8f16549f06b14e18ca90d6756ed2416b732c813 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Wed, 28 May 2025 18:25:46 +0200
Subject: [PATCH] [lldb] Refactor away UB in SBValue::GetLoadAddress

The problem was in calling GetLoadAddress on a value in the error state,
where `ValueObject::GetLoadAddress` could end up accessing the
uninitialized "address type" by-ref return value from `GetAddressOf`.
This probably happened because each function expected the other to
initialize it.

We can guarantee initialization by turning this into a proper return
value.

I've added a test, but it only (reliably) crashes if lldb is built with
ubsan.
---
 lldb/include/lldb/ValueObject/ValueObject.h   |  6 +-
 .../lldb/ValueObject/ValueObjectConstResult.h |  4 +-
 .../ValueObject/ValueObjectConstResultChild.h |  4 +-
 .../ValueObject/ValueObjectConstResultImpl.h  |  4 +-
 lldb/source/API/SBValue.cpp                   |  6 +-
 .../Commands/CommandObjectWatchpoint.cpp      |  2 +-
 .../DataFormatters/CXXFunctionPointer.cpp     |  3 +-
 .../DataFormatters/FormattersHelpers.cpp      |  8 +-
 lldb/source/DataFormatters/TypeFormat.cpp     |  2 +-
 .../DataFormatters/ValueObjectPrinter.cpp     |  3 +-
 lldb/source/Expression/Materializer.cpp       |  4 +-
 .../Clang/ClangUserExpression.cpp             |  2 +-
 .../Plugins/Language/CPlusPlus/Coroutines.cpp |  3 +-
 .../Plugins/Language/CPlusPlus/LibCxxList.cpp |  2 +-
 .../Plugins/Language/CPlusPlus/LibStdcpp.cpp  | 18 +---
 .../ItaniumABI/ItaniumABILanguageRuntime.cpp  | 66 ++++++-------
 .../AppleObjCRuntime/AppleObjCRuntimeV1.cpp   |  2 +-
 .../AppleObjCRuntime/AppleObjCRuntimeV2.cpp   |  4 +-
 .../ObjC/ObjCLanguageRuntime.cpp              |  2 +-
 lldb/source/ValueObject/ValueObject.cpp       | 93 +++++++------------
 lldb/source/ValueObject/ValueObjectChild.cpp  |  2 +-
 .../ValueObject/ValueObjectConstResult.cpp    |  6 +-
 .../ValueObjectConstResultChild.cpp           |  7 +-
 .../ValueObjectConstResultImpl.cpp            | 18 ++--
 lldb/source/ValueObject/ValueObjectVTable.cpp |  2 +-
 .../test/API/python_api/value/TestValueAPI.py | 13 ++-
 26 files changed, 121 insertions(+), 165 deletions(-)

diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h
index 0add8ebeccdc8..5b78e57cb4996 100644
--- a/lldb/include/lldb/ValueObject/ValueObject.h
+++ b/lldb/include/lldb/ValueObject/ValueObject.h
@@ -573,10 +573,10 @@ class ValueObject {
   /// child as well.
   void SetName(ConstString name) { m_name = name; }
 
-  virtual lldb::addr_t GetAddressOf(bool scalar_is_load_address = true,
-                                    AddressType *address_type = nullptr);
+  virtual std::pair<AddressType, lldb::addr_t>
+  GetAddressOf(bool scalar_is_load_address = true);
 
-  lldb::addr_t GetPointerValue(AddressType *address_type = nullptr);
+  std::pair<AddressType, lldb::addr_t> GetPointerValue();
 
   lldb::ValueObjectSP GetSyntheticChild(ConstString key) const;
 
diff --git a/lldb/include/lldb/ValueObject/ValueObjectConstResult.h b/lldb/include/lldb/ValueObject/ValueObjectConstResult.h
index 2ee531f5858e1..3a11ab360649a 100644
--- a/lldb/include/lldb/ValueObject/ValueObjectConstResult.h
+++ b/lldb/include/lldb/ValueObject/ValueObjectConstResult.h
@@ -86,8 +86,8 @@ class ValueObjectConstResult : public ValueObject {
 
   lldb::ValueObjectSP AddressOf(Status &error) override;
 
-  lldb::addr_t GetAddressOf(bool scalar_is_load_address = true,
-                            AddressType *address_type = nullptr) override;
+  std::pair<AddressType, lldb::addr_t>
+  GetAddressOf(bool scalar_is_load_address = true) override;
 
   size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
                         uint32_t item_count = 1) override;
diff --git a/lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h b/lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h
index ad97b885684ee..09d62253e1bb0 100644
--- a/lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h
+++ b/lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h
@@ -48,8 +48,8 @@ class ValueObjectConstResultChild : public ValueObjectChild {
 
   lldb::ValueObjectSP AddressOf(Status &error) override;
 
-  lldb::addr_t GetAddressOf(bool scalar_is_load_address = true,
-                            AddressType *address_type = nullptr) override;
+  std::pair<AddressType, lldb::addr_t>
+  GetAddressOf(bool scalar_is_load_address = true) override;
 
   size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
                         uint32_t item_count = 1) override;
diff --git a/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h b/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
index 5509886a8965d..3fc882ac782ec 100644
--- a/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
+++ b/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h
@@ -58,8 +58,8 @@ class ValueObjectConstResultImpl {
     m_live_address_type = address_type;
   }
 
-  virtual lldb::addr_t GetAddressOf(bool scalar_is_load_address = true,
-                                    AddressType *address_type = nullptr);
+  virtual std::pair<AddressType, lldb::addr_t>
+  GetAddressOf(bool scalar_is_load_address = true);
 
   virtual size_t GetPointeeData(DataExtractor &data, uint32_t item_idx = 0,
                                 uint32_t item_count = 1);
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 88c86a5482910..4b7aae6a54a04 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -1336,10 +1336,8 @@ lldb::SBAddress SBValue::GetAddress() {
   if (value_sp) {
     TargetSP target_sp(value_sp->GetTargetSP());
     if (target_sp) {
-      lldb::addr_t value = LLDB_INVALID_ADDRESS;
-      const bool scalar_is_load_address = true;
-      AddressType addr_type;
-      value = value_sp->GetAddressOf(scalar_is_load_address, &addr_type);
+      auto [addr_type, value] =
+          value_sp->GetAddressOf(/*scalar_is_load_address=*/true);
       if (addr_type == eAddressTypeFile) {
         ModuleSP module_sp(value_sp->GetModule());
         if (module_sp)
diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 20f4b91f15340..e36c6a33de7df 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -863,7 +863,7 @@ corresponding to the byte size of the data type.");
 
     if (valobj_sp) {
       AddressType addr_type;
-      addr = valobj_sp->GetAddressOf(false, &addr_type);
+      std::tie(addr_type, addr) = valobj_sp->GetAddressOf(false);
       if (addr_type == eAddressTypeLoad) {
         // We're in business.
         // Find out the size of this variable.
diff --git a/lldb/source/DataFormatters/CXXFunctionPointer.cpp b/lldb/source/DataFormatters/CXXFunctionPointer.cpp
index d79fb1d678ebd..7e73086ee6a35 100644
--- a/lldb/source/DataFormatters/CXXFunctionPointer.cpp
+++ b/lldb/source/DataFormatters/CXXFunctionPointer.cpp
@@ -24,8 +24,7 @@ using namespace lldb_private::formatters;
 bool lldb_private::formatters::CXXFunctionPointerSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   StreamString sstr;
-  AddressType func_ptr_address_type = eAddressTypeInvalid;
-  addr_t func_ptr_address = valobj.GetPointerValue(&func_ptr_address_type);
+  auto [func_ptr_address_type, func_ptr_address] = valobj.GetPointerValue();
   if (func_ptr_address != 0 && func_ptr_address != LLDB_INVALID_ADDRESS) {
     switch (func_ptr_address_type) {
     case eAddressTypeInvalid:
diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp
index 5f5541c352623..38cee43a64471 100644
--- a/lldb/source/DataFormatters/FormattersHelpers.cpp
+++ b/lldb/source/DataFormatters/FormattersHelpers.cpp
@@ -114,12 +114,14 @@ lldb_private::formatters::ExtractIndexFromString(const char *item_name) {
 Address
 lldb_private::formatters::GetArrayAddressOrPointerValue(ValueObject &valobj) {
   lldb::addr_t data_addr = LLDB_INVALID_ADDRESS;
-  AddressType type;
+  AddressType type = eAddressTypeInvalid;
 
   if (valobj.IsPointerType())
-    data_addr = valobj.GetPointerValue(&type);
+    std::tie(type, data_addr) = valobj.GetPointerValue();
   else if (valobj.IsArrayType())
-    data_addr = valobj.GetAddressOf(/*scalar_is_load_address=*/true, &type);
+    std::tie(type, data_addr) =
+        valobj.GetAddressOf(/*scalar_is_load_address=*/true);
+
   if (data_addr != LLDB_INVALID_ADDRESS && type == eAddressTypeFile)
     return Address(data_addr, valobj.GetModule()->GetSectionList());
 
diff --git a/lldb/source/DataFormatters/TypeFormat.cpp b/lldb/source/DataFormatters/TypeFormat.cpp
index f4cb8b46d272a..6b91a76f9d9c5 100644
--- a/lldb/source/DataFormatters/TypeFormat.cpp
+++ b/lldb/source/DataFormatters/TypeFormat.cpp
@@ -80,7 +80,7 @@ bool TypeFormatImpl_Format::FormatObject(ValueObject *valobj,
               Status error;
               WritableDataBufferSP buffer_sp(
                   new DataBufferHeap(max_len + 1, 0));
-              Address address(valobj->GetPointerValue());
+              Address address(valobj->GetPointerValue().second);
               target_sp->ReadCStringFromMemory(
                   address, (char *)buffer_sp->GetBytes(), max_len, error);
               if (error.Success())
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index 5e04a621bbda8..0d6af406012a3 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -542,8 +542,7 @@ bool ValueObjectPrinter::ShouldPrintChildren(
   if (is_ptr || is_ref) {
     // We have a pointer or reference whose value is an address. Make sure
     // that address is not NULL
-    AddressType ptr_address_type;
-    if (valobj.GetPointerValue(&ptr_address_type) == 0)
+    if (valobj.GetPointerValue().second == 0)
       return false;
 
     const bool is_root_level = m_curr_depth == 0;
diff --git a/lldb/source/Expression/Materializer.cpp b/lldb/source/Expression/Materializer.cpp
index 8d48b5e50041c..634b929ce19a1 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -508,10 +508,8 @@ class EntityVariableBase : public Materializer::Entity {
         return;
       }
     } else {
-      AddressType address_type = eAddressTypeInvalid;
-      const bool scalar_is_load_address = false;
       lldb::addr_t addr_of_valobj =
-          valobj_sp->GetAddressOf(scalar_is_load_address, &address_type);
+          valobj_sp->GetAddressOf(/*scalar_is_load_address=*/false).second;
       if (addr_of_valobj != LLDB_INVALID_ADDRESS) {
         Status write_error;
         map.WritePointerToMemory(load_addr, addr_of_valobj, write_error);
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index ae34a983612f7..fad21d5b1275e 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -898,7 +898,7 @@ bool ClangUserExpression::AddArguments(ExecutionContext &exe_ctx,
 
     if (m_ctx_obj) {
       AddressType address_type;
-      object_ptr = m_ctx_obj->GetAddressOf(false, &address_type);
+      std::tie(address_type, object_ptr) = m_ctx_obj->GetAddressOf(false);
       if (object_ptr == LLDB_INVALID_ADDRESS ||
           address_type != eAddressTypeLoad)
         object_ptr_error = Status::FromErrorString("Can't get context object's "
diff --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
index d3cdb231fbb01..8e01878eec6b0 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp
@@ -32,8 +32,7 @@ static lldb::addr_t GetCoroFramePtrFromHandle(ValueObjectSP valobj_sp) {
   if (!ptr_sp->GetCompilerType().IsPointerType())
     return LLDB_INVALID_ADDRESS;
 
-  AddressType addr_type;
-  lldb::addr_t frame_ptr_addr = ptr_sp->GetPointerValue(&addr_type);
+  auto [addr_type, frame_ptr_addr] = ptr_sp->GetPointerValue();
   if (!frame_ptr_addr || frame_ptr_addr == LLDB_INVALID_ADDRESS)
     return LLDB_INVALID_ADDRESS;
   lldbassert(addr_type == AddressType::eAddressTypeLoad);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
index 30db5f15c388f..851fe821408c5 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
@@ -388,7 +388,7 @@ lldb::ValueObjectSP ListFrontEnd::GetChildAtIndex(uint32_t idx) {
       return lldb::ValueObjectSP();
 
     // if we grabbed the __next_ pointer, then the child is one pointer deep-er
-    lldb::addr_t addr = current_sp->GetParent()->GetPointerValue();
+    lldb::addr_t addr = current_sp->GetParent()->GetPointerValue().second;
     addr = addr + 2 * process_sp->GetAddressByteSize();
     ExecutionContext exe_ctx(process_sp);
     current_sp =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 02113baf64b8c..b00b7cc1c2681 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -240,16 +240,10 @@ VectorIteratorSyntheticFrontEnd::GetIndexOfChildWithName(ConstString name) {
 bool lldb_private::formatters::LibStdcppStringSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   const bool scalar_is_load_addr = true;
-  AddressType addr_type;
-  lldb::addr_t addr_of_string = LLDB_INVALID_ADDRESS;
-  if (valobj.IsPointerOrReferenceType()) {
-    Status error;
-    ValueObjectSP pointee_sp = valobj.Dereference(error);
-    if (pointee_sp && error.Success())
-      addr_of_string = pointee_sp->GetAddressOf(scalar_is_load_addr, &addr_type);
-  } else
-    addr_of_string =
-        valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
+  auto [addr_type, addr_of_string] =
+      valobj.IsPointerOrReferenceType()
+          ? valobj.GetPointerValue()
+          : valobj.GetAddressOf(scalar_is_load_addr);
   if (addr_of_string != LLDB_INVALID_ADDRESS) {
     switch (addr_type) {
     case eAddressTypeLoad: {
@@ -296,9 +290,7 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
 bool lldb_private::formatters::LibStdcppWStringSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   const bool scalar_is_load_addr = true;
-  AddressType addr_type;
-  lldb::addr_t addr_of_string =
-      valobj.GetAddressOf(scalar_is_load_addr, &addr_type);
+  auto [addr_type, addr_of_string] = valobj.GetAddressOf(scalar_is_load_addr);
   if (addr_of_string != LLDB_INVALID_ADDRESS) {
     switch (addr_type) {
     case eAddressTypeLoad: {
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
index 0d068ed5950d5..2ee0d0dff824f 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
@@ -69,9 +69,8 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
       LLDB_LOGF(log,
                 "0x%16.16" PRIx64
                 ": static-type = '%s' has vtable symbol '%s'\n",
-                in_value.GetPointerValue(),
-                in_value.GetTypeName().GetCString(),
-                symbol_name.str().c_str());
+                in_value.GetPointerValue().second,
+                in_value.GetTypeName().GetCString(), symbol_name.str().c_str());
       // We are a C++ class, that's good.  Get the class name and look it
       // up:
       llvm::StringRef class_name = symbol_name;
@@ -111,7 +110,7 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
       lldb::TypeSP type_sp;
       if (class_types.Empty()) {
         LLDB_LOGF(log, "0x%16.16" PRIx64 ": is not dynamic\n",
-                  in_value.GetPointerValue());
+                  in_value.GetPointerValue().second);
         return TypeAndOrName();
       }
       if (class_types.GetSize() == 1) {
@@ -119,13 +118,13 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
         if (type_sp) {
           if (TypeSystemClang::IsCXXClassType(
                   type_sp->GetForwardCompilerType())) {
-            LLDB_LOGF(
-                log,
-                "0x%16.16" PRIx64
-                ": static-type = '%s' has dynamic type: uid={0x%" PRIx64
-                "}, type-name='%s'\n",
-                in_value.GetPointerValue(), in_value.GetTypeName().AsCString(),
-                type_sp->GetID(), type_sp->GetName().GetCString());
+            LLDB_LOGF(log,
+                      "0x%16.16" PRIx64
+                      ": static-type = '%s' has dynamic type: uid={0x%" PRIx64
+                      "}, type-name='%s'\n",
+                      in_value.GetPointerValue().second,
+                      in_value.GetTypeName().AsCString(), type_sp->GetID(),
+                      type_sp->GetName().GetCString());
             type_info.SetTypeSP(type_sp);
           }
         }
@@ -135,14 +134,13 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
           for (i = 0; i < class_types.GetSize(); i++) {
             type_sp = class_types.GetTypeAtIndex(i);
             if (type_sp) {
-              LLDB_LOGF(
-                  log,
-                  "0x%16.16" PRIx64
-                  ": static-type = '%s' has multiple matching dynamic "
-                  "types: uid={0x%" PRIx64 "}, type-name='%s'\n",
-                  in_value.GetPointerValue(),
-                  in_value.GetTypeName().AsCString(),
-                  type_sp->GetID(), type_sp->GetName().GetCString());
+              LLDB_LOGF(log,
+                        "0x%16.16" PRIx64
+                        ": static-type = '%s' has multiple matching dynamic "
+                        "types: uid={0x%" PRIx64 "}, type-name='%s'\n",
+                        in_value.GetPointerValue().second,
+                        in_value.GetTypeName().AsCString(), type_sp->GetID(),
+                        type_sp->GetName().GetCString());
             }
           }
         }
@@ -152,14 +150,13 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
           if (type_sp) {
             if (TypeSystemClang::IsCXXClassType(
                     type_sp->GetForwardCompilerType())) {
-              LLDB_LOGF(
-                  log,
-                  "0x%16.16" PRIx64 ": static-type = '%s' has multiple "
-                  "matching dynamic types, picking "
-                  "this one: uid={0x%" PRIx64 "}, type-name='%s'\n",
-                  in_value.GetPointerValue(),
-                  in_value.GetTypeName().AsCString(),
-                  type_sp->GetID(), type_sp->GetName().GetCString());
+              LLDB_LOGF(log,
+                        "0x%16.16" PRIx64 ": static-type = '%s' has multiple "
+                        "matching dynamic types, picking "
+                        "this one: uid={0x%" PRIx64 "}, type-name='%s'\n",
+                        in_value.GetPointerValue().second,
+                        in_value.GetTypeName().AsCString(), type_sp->GetID(),
+                        type_sp->GetName().GetCString());
               type_info.SetTypeSP(type_sp);
             }
           }
@@ -170,7 +167,7 @@ TypeAndOrName ItaniumABILanguageRuntime::GetTypeInfo(
                     "0x%16.16" PRIx64
                     ": static-type = '%s' has multiple matching dynamic "
                     "types, didn't find a C++ match\n",
-                    in_value.GetPointerValue(),
+                    in_value.GetPointerValue().second,
                     in_value.GetTypeName().AsCString());
         }
       }
@@ -230,13 +227,10 @@ llvm::Expected<LanguageRuntime::VTableInfo>
     return llvm::createStringError(std::errc::invalid_argument,
                                    "invalid process");
 
-  AddressType address_type;
-  lldb::addr_t original_ptr = LLDB_INVALID_ADDRESS;
-  if (type.IsPointerOrReferenceType())
-    original_ptr = in_value.GetPointerValue(&address_type);
-  else
-    original_ptr = in_value.GetAddressOf(/*scalar_is_load_address=*/true,
-                                         &address_type);
+  auto [address_type, original_ptr] =
+      type.IsPointerOrReferenceType()
+          ? in_value.GetPointerValue()
+          : in_value.GetAddressOf(/*scalar_is_load_address=*/true);
   if (original_ptr == LLDB_INVALID_ADDRESS || address_type != eAddressTypeLoad)
     return llvm::createStringError(std::errc::invalid_argument,
                                    "failed to get the address of the value");
@@ -357,7 +351,7 @@ bool ItaniumABILanguageRuntime::GetDynamicTypeAndAddress(
     return false;
   // So the dynamic type is a value that starts at offset_to_top above
   // the original address.
-  lldb::addr_t dynamic_addr = in_value.GetPointerValue() + offset_to_top;
+  lldb::addr_t dynamic_addr = in_value.GetPointerValue().second + offset_to_top;
   if (!m_process->GetTarget().ResolveLoadAddress(
           dynamic_addr, dynamic_address)) {
     dynamic_address.SetRawAddress(dynamic_addr);
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
index db1317d70d060..2e31a3c645a78 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp
@@ -55,7 +55,7 @@ bool AppleObjCRuntimeV1::GetDynamicTypeAndAddress(
     auto class_descriptor(GetClassDescriptor(in_value));
     if (class_descriptor && class_descriptor->IsValid() &&
         class_descriptor->GetClassName()) {
-      const addr_t object_ptr = in_value.GetPointerValue();
+      const addr_t object_ptr = in_value.GetPointerValue().second;
       address.SetRawAddress(object_ptr);
       class_type_or_name.SetName(class_descriptor->GetClassName());
     }
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index f458357d948da..cb337b317dc91 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -801,7 +801,7 @@ bool AppleObjCRuntimeV2::GetDynamicTypeAndAddress(
     // be the ISA pointer.
     ClassDescriptorSP objc_class_sp(GetNonKVOClassDescriptor(in_value));
     if (objc_class_sp) {
-      const addr_t object_ptr = in_value.GetPointerValue();
+      const addr_t object_ptr = in_value.GetPointerValue().second;
       address.SetRawAddress(object_ptr);
 
       ConstString class_name(objc_class_sp->GetClassName());
@@ -1521,7 +1521,7 @@ AppleObjCRuntimeV2::GetClassDescriptor(ValueObject &valobj) {
   // ObjC object)
   if (!valobj.GetCompilerType().IsValid())
     return objc_class_sp;
-  addr_t isa_pointer = valobj.GetPointerValue();
+  addr_t isa_pointer = valobj.GetPointerValue().second;
 
   // tagged pointer
   if (IsTaggedPointer(isa_pointer))
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
index cb745135b0249..f75ec177775ad 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp
@@ -270,7 +270,7 @@ ObjCLanguageRuntime::GetClassDescriptor(ValueObject &valobj) {
   // pointers returned by the expression parser, don't consider this a valid
   // ObjC object)
   if (valobj.GetCompilerType().IsValid()) {
-    addr_t isa_pointer = valobj.GetPointerValue();
+    addr_t isa_pointer = valobj.GetPointerValue().second;
     if (isa_pointer != LLDB_INVALID_ADDRESS) {
       ExecutionContext exe_ctx(valobj.GetExecutionContextRef());
 
diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index 487c95ca03a69..041cd386fb0e9 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -657,9 +657,7 @@ bool ValueObject::IsCStringContainer(bool check_pointer) {
     return true;
   if (type_flags.Test(eTypeIsArray))
     return true;
-  addr_t cstr_address = LLDB_INVALID_ADDRESS;
-  AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetPointerValue(&cstr_address_type);
+  addr_t cstr_address = GetPointerValue().second;
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 
@@ -708,9 +706,8 @@ size_t ValueObject::GetPointeeData(DataExtractor &data, uint32_t item_idx,
     lldb::DataBufferSP data_sp(heap_buf_ptr =
                                    new lldb_private::DataBufferHeap());
 
-    AddressType addr_type;
-    lldb::addr_t addr = is_pointer_type ? GetPointerValue(&addr_type)
-                                        : GetAddressOf(true, &addr_type);
+    auto [addr_type, addr] =
+        is_pointer_type ? GetPointerValue() : GetAddressOf(true);
 
     switch (addr_type) {
     case eAddressTypeFile: {
@@ -918,10 +915,10 @@ ValueObject::ReadPointedString(lldb::WritableDataBufferSP &buffer_sp,
           cstr_len = max_length;
         }
       }
-      cstr_address = GetAddressOf(true, &cstr_address_type);
+      std::tie(cstr_address_type, cstr_address) = GetAddressOf(true);
     } else {
       // We have a pointer
-      cstr_address = GetPointerValue(&cstr_address_type);
+      std::tie(cstr_address_type, cstr_address) = GetPointerValue();
     }
 
     if (cstr_address == 0 || cstr_address == LLDB_INVALID_ADDRESS) {
@@ -1199,7 +1196,7 @@ llvm::Expected<bool> ValueObject::GetValueAsBool() {
       return value_or_err->isNonZero();
   }
   if (val_type.IsArrayType())
-    return GetAddressOf() != 0;
+    return GetAddressOf().second != 0;
 
   return llvm::make_error<llvm::StringError>("type cannot be converted to bool",
                                              llvm::inconvertibleErrorCode());
@@ -1600,70 +1597,55 @@ bool ValueObject::DumpPrintableRepresentation(
   return var_success;
 }
 
-addr_t ValueObject::GetAddressOf(bool scalar_is_load_address,
-                                 AddressType *address_type) {
+std::pair<AddressType, addr_t>
+ValueObject::GetAddressOf(bool scalar_is_load_address) {
   // Can't take address of a bitfield
   if (IsBitfield())
-    return LLDB_INVALID_ADDRESS;
+    return {eAddressTypeInvalid, LLDB_INVALID_ADDRESS};
 
   if (!UpdateValueIfNeeded(false))
-    return LLDB_INVALID_ADDRESS;
+    return {eAddressTypeInvalid, LLDB_INVALID_ADDRESS};
 
   switch (m_value.GetValueType()) {
   case Value::ValueType::Invalid:
-    return LLDB_INVALID_ADDRESS;
+    return {eAddressTypeInvalid, LLDB_INVALID_ADDRESS};
   case Value::ValueType::Scalar:
     if (scalar_is_load_address) {
-      if (address_type)
-        *address_type = eAddressTypeLoad;
-      return m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
+      return {eAddressTypeLoad,
+              m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS)};
     }
-    break;
+    return {eAddressTypeInvalid, LLDB_INVALID_ADDRESS};
 
   case Value::ValueType::LoadAddress:
-  case Value::ValueType::FileAddress: {
-    if (address_type)
-      *address_type = m_value.GetValueAddressType();
-    return m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
-  } break;
-  case Value::ValueType::HostAddress: {
-    if (address_type)
-      *address_type = m_value.GetValueAddressType();
-    return LLDB_INVALID_ADDRESS;
-  } break;
+  case Value::ValueType::FileAddress:
+    return {m_value.GetValueAddressType(),
+            m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS)};
+  case Value::ValueType::HostAddress:
+    return {m_value.GetValueAddressType(), LLDB_INVALID_ADDRESS};
   }
-  if (address_type)
-    *address_type = eAddressTypeInvalid;
-  return LLDB_INVALID_ADDRESS;
+  llvm_unreachable("Unhandled value type!");
 }
 
-addr_t ValueObject::GetPointerValue(AddressType *address_type) {
-  addr_t address = LLDB_INVALID_ADDRESS;
-  if (address_type)
-    *address_type = eAddressTypeInvalid;
-
+std::pair<AddressType, addr_t> ValueObject::GetPointerValue() {
   if (!UpdateValueIfNeeded(false))
-    return address;
+    return {eAddressTypeInvalid, LLDB_INVALID_ADDRESS};
 
   switch (m_value.GetValueType()) {
   case Value::ValueType::Invalid:
-    return LLDB_INVALID_ADDRESS;
+    return {eAddressTypeInvalid, LLDB_INVALID_ADDRESS};
   case Value::ValueType::Scalar:
-    address = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
-    break;
+    return {GetAddressTypeOfChildren(),
+            m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS)};
 
   case Value::ValueType::HostAddress:
   case Value::ValueType::LoadAddress:
   case Value::ValueType::FileAddress: {
     lldb::offset_t data_offset = 0;
-    address = m_data.GetAddress(&data_offset);
-  } break;
+    return {GetAddressTypeOfChildren(), m_data.GetAddress(&data_offset)};
+  }
   }
 
-  if (address_type)
-    *address_type = GetAddressTypeOfChildren();
-
-  return address;
+  llvm_unreachable("Unhandled value type!");
 }
 
 static const char *ConvertBoolean(lldb::LanguageType language_type,
@@ -2750,7 +2732,7 @@ ValueObjectSP ValueObject::CreateConstantValue(ConstString name) {
 
     valobj_sp = ValueObjectConstResult::Create(
         exe_ctx.GetBestExecutionContextScope(), GetCompilerType(), name, data,
-        GetAddressOf());
+        GetAddressOf().second);
   }
 
   if (!valobj_sp) {
@@ -2876,9 +2858,7 @@ ValueObjectSP ValueObject::AddressOf(Status &error) {
   if (m_addr_of_valobj_sp)
     return m_addr_of_valobj_sp;
 
-  AddressType address_type = eAddressTypeInvalid;
-  const bool scalar_is_load_address = false;
-  addr_t addr = GetAddressOf(scalar_is_load_address, &address_type);
+  auto [address_type, addr] = GetAddressOf(/*scalar_is_load_address=*/false);
   error.Clear();
   if (addr != LLDB_INVALID_ADDRESS && address_type != eAddressTypeHost) {
     switch (address_type) {
@@ -2965,8 +2945,7 @@ lldb::ValueObjectSP ValueObject::Clone(ConstString new_name) {
 ValueObjectSP ValueObject::CastPointerType(const char *name,
                                            CompilerType &compiler_type) {
   ValueObjectSP valobj_sp;
-  AddressType address_type;
-  addr_t ptr_value = GetPointerValue(&address_type);
+  addr_t ptr_value = GetPointerValue().second;
 
   if (ptr_value != LLDB_INVALID_ADDRESS) {
     Address ptr_addr(ptr_value);
@@ -2979,8 +2958,7 @@ ValueObjectSP ValueObject::CastPointerType(const char *name,
 
 ValueObjectSP ValueObject::CastPointerType(const char *name, TypeSP &type_sp) {
   ValueObjectSP valobj_sp;
-  AddressType address_type;
-  addr_t ptr_value = GetPointerValue(&address_type);
+  addr_t ptr_value = GetPointerValue().second;
 
   if (ptr_value != LLDB_INVALID_ADDRESS) {
     Address ptr_addr(ptr_value);
@@ -2992,11 +2970,9 @@ ValueObjectSP ValueObject::CastPointerType(const char *name, TypeSP &type_sp) {
 }
 
 lldb::addr_t ValueObject::GetLoadAddress() {
-  lldb::addr_t addr_value = LLDB_INVALID_ADDRESS;
   if (auto target_sp = GetTargetSP()) {
     const bool scalar_is_load_address = true;
-    AddressType addr_type;
-    addr_value = GetAddressOf(scalar_is_load_address, &addr_type);
+    auto [addr_type, addr_value] = GetAddressOf(scalar_is_load_address);
     if (addr_type == eAddressTypeFile) {
       lldb::ModuleSP module_sp(GetModule());
       if (!module_sp)
@@ -3009,8 +2985,9 @@ lldb::addr_t ValueObject::GetLoadAddress() {
     } else if (addr_type == eAddressTypeHost ||
                addr_type == eAddressTypeInvalid)
       addr_value = LLDB_INVALID_ADDRESS;
+    return addr_value;
   }
-  return addr_value;
+  return LLDB_INVALID_ADDRESS;
 }
 
 llvm::Expected<lldb::ValueObjectSP> ValueObject::CastDerivedToBaseType(
diff --git a/lldb/source/ValueObject/ValueObjectChild.cpp b/lldb/source/ValueObject/ValueObjectChild.cpp
index d7f1ad08415e3..cbd4d4cf3c7cc 100644
--- a/lldb/source/ValueObject/ValueObjectChild.cpp
+++ b/lldb/source/ValueObject/ValueObjectChild.cpp
@@ -118,7 +118,7 @@ bool ValueObjectChild::UpdateValue() {
            (parent_type_flags.AnySet(lldb::eTypeInstanceIsPointer)));
 
       if (parent->GetCompilerType().ShouldTreatScalarValueAsAddress()) {
-        m_value.GetScalar() = parent->GetPointerValue();
+        m_value.GetScalar() = parent->GetPointerValue().second;
 
         switch (parent->GetAddressTypeOfChildren()) {
         case eAddressTypeFile: {
diff --git a/lldb/source/ValueObject/ValueObjectConstResult.cpp b/lldb/source/ValueObject/ValueObjectConstResult.cpp
index 01f870529a14d..0c7a53b0bd748 100644
--- a/lldb/source/ValueObject/ValueObjectConstResult.cpp
+++ b/lldb/source/ValueObject/ValueObjectConstResult.cpp
@@ -265,9 +265,9 @@ lldb::ValueObjectSP ValueObjectConstResult::AddressOf(Status &error) {
   return m_impl.AddressOf(error);
 }
 
-lldb::addr_t ValueObjectConstResult::GetAddressOf(bool scalar_is_load_address,
-                                                  AddressType *address_type) {
-  return m_impl.GetAddressOf(scalar_is_load_address, address_type);
+std::pair<AddressType, addr_t>
+ValueObjectConstResult::GetAddressOf(bool scalar_is_load_address) {
+  return m_impl.GetAddressOf(scalar_is_load_address);
 }
 
 size_t ValueObjectConstResult::GetPointeeData(DataExtractor &data,
diff --git a/lldb/source/ValueObject/ValueObjectConstResultChild.cpp b/lldb/source/ValueObject/ValueObjectConstResultChild.cpp
index b1c800dfe8c33..963d920e9cd1f 100644
--- a/lldb/source/ValueObject/ValueObjectConstResultChild.cpp
+++ b/lldb/source/ValueObject/ValueObjectConstResultChild.cpp
@@ -50,10 +50,9 @@ lldb::ValueObjectSP ValueObjectConstResultChild::AddressOf(Status &error) {
   return m_impl.AddressOf(error);
 }
 
-lldb::addr_t
-ValueObjectConstResultChild::GetAddressOf(bool scalar_is_load_address,
-                                          AddressType *address_type) {
-  return m_impl.GetAddressOf(scalar_is_load_address, address_type);
+std::pair<AddressType, lldb::addr_t>
+ValueObjectConstResultChild::GetAddressOf(bool scalar_is_load_address) {
+  return m_impl.GetAddressOf(scalar_is_load_address);
 }
 
 size_t ValueObjectConstResultChild::GetPointeeData(DataExtractor &data,
diff --git a/lldb/source/ValueObject/ValueObjectConstResultImpl.cpp b/lldb/source/ValueObject/ValueObjectConstResultImpl.cpp
index d79f655b015a3..b1149a9e37927 100644
--- a/lldb/source/ValueObject/ValueObjectConstResultImpl.cpp
+++ b/lldb/source/ValueObject/ValueObjectConstResultImpl.cpp
@@ -206,22 +206,16 @@ ValueObjectConstResultImpl::Cast(const CompilerType &compiler_type) {
   return result_cast->GetSP();
 }
 
-lldb::addr_t
-ValueObjectConstResultImpl::GetAddressOf(bool scalar_is_load_address,
-                                         AddressType *address_type) {
+std::pair<AddressType, addr_t>
+ValueObjectConstResultImpl::GetAddressOf(bool scalar_is_load_address) {
 
   if (m_impl_backend == nullptr)
-    return 0;
-
-  if (m_live_address == LLDB_INVALID_ADDRESS) {
-    return m_impl_backend->ValueObject::GetAddressOf(scalar_is_load_address,
-                                                     address_type);
-  }
+    return {eAddressTypeInvalid, 0};
 
-  if (address_type)
-    *address_type = m_live_address_type;
+  if (m_live_address == LLDB_INVALID_ADDRESS)
+    return m_impl_backend->ValueObject::GetAddressOf(scalar_is_load_address);
 
-  return m_live_address;
+  return {m_live_address_type, m_live_address};
 }
 
 size_t ValueObjectConstResultImpl::GetPointeeData(DataExtractor &data,
diff --git a/lldb/source/ValueObject/ValueObjectVTable.cpp b/lldb/source/ValueObject/ValueObjectVTable.cpp
index 92bd086d88ee4..e8962cc723f4a 100644
--- a/lldb/source/ValueObject/ValueObjectVTable.cpp
+++ b/lldb/source/ValueObject/ValueObjectVTable.cpp
@@ -252,7 +252,7 @@ bool ValueObjectVTable::UpdateValue() {
   m_num_vtable_entries = (vtable_end_addr - vtable_start_addr) / m_addr_size;
 
   m_value.SetValueType(Value::ValueType::LoadAddress);
-  m_value.GetScalar() = parent->GetAddressOf();
+  m_value.GetScalar() = parent->GetAddressOf().second;
   auto type_system_or_err =
       target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC_plus_plus);
   if (type_system_or_err) {
diff --git a/lldb/test/API/python_api/value/TestValueAPI.py b/lldb/test/API/python_api/value/TestValueAPI.py
index 9eaf2c994d846..08299343a7862 100644
--- a/lldb/test/API/python_api/value/TestValueAPI.py
+++ b/lldb/test/API/python_api/value/TestValueAPI.py
@@ -269,7 +269,12 @@ def test(self):
             frame0.FindVariable("another_fixed_int_ptr").GetValue(),
             "0xaa",
         )
-        self.assertEqual(
-            frame0.FindVariable("a_null_int_ptr").GetValue(),
-            "0x0",
-        )
+        a_null_int_ptr = frame0.FindVariable("a_null_int_ptr")
+        self.assertEqual( a_null_int_ptr.GetValue(), "0x0")
+
+        # Check that dereferencing a null pointer produces reasonable results
+        # (does not crash).
+        self.assertEqual(a_null_int_ptr.Dereference().GetError().GetCString(),
+                         "parent is NULL")
+        self.assertEqual(a_null_int_ptr.Dereference().GetLoadAddress(),
+                         lldb.LLDB_INVALID_ADDRESS)



More information about the lldb-commits mailing list