Author: Pavel Labath Date: 2025-06-02T09:39:56+02:00 New Revision: e9fad0e91c49ca0f2669989dbad95664cbc9cbf3
URL: https://github.com/llvm/llvm-project/commit/e9fad0e91c49ca0f2669989dbad95664cbc9cbf3 DIFF: https://github.com/llvm/llvm-project/commit/e9fad0e91c49ca0f2669989dbad95664cbc9cbf3.diff LOG: [lldb] Refactor away UB in SBValue::GetLoadAddress (#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. Added: Modified: lldb/include/lldb/ValueObject/ValueObject.h lldb/include/lldb/ValueObject/ValueObjectConstResult.h lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h lldb/source/API/SBValue.cpp lldb/source/Commands/CommandObjectWatchpoint.cpp lldb/source/DataFormatters/CXXFunctionPointer.cpp lldb/source/DataFormatters/FormattersHelpers.cpp lldb/source/DataFormatters/TypeFormat.cpp lldb/source/DataFormatters/ValueObjectPrinter.cpp lldb/source/Expression/Materializer.cpp lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.cpp lldb/source/ValueObject/ValueObject.cpp lldb/source/ValueObject/ValueObjectChild.cpp lldb/source/ValueObject/ValueObjectConstResult.cpp lldb/source/ValueObject/ValueObjectConstResultChild.cpp lldb/source/ValueObject/ValueObjectConstResultImpl.cpp lldb/source/ValueObject/ValueObjectVTable.cpp lldb/test/API/python_api/value/TestValueAPI.py Removed: ################################################################################ diff --git a/lldb/include/lldb/ValueObject/ValueObject.h b/lldb/include/lldb/ValueObject/ValueObject.h index 0add8ebeccdc8..3c62f3c17619e 100644 --- a/lldb/include/lldb/ValueObject/ValueObject.h +++ b/lldb/include/lldb/ValueObject/ValueObject.h @@ -573,10 +573,14 @@ 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); + struct AddrAndType { + lldb::addr_t address = LLDB_INVALID_ADDRESS; + AddressType type = eAddressTypeInvalid; + }; + + virtual AddrAndType GetAddressOf(bool scalar_is_load_address = true); - lldb::addr_t GetPointerValue(AddressType *address_type = nullptr); + AddrAndType 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..1e4b81c4dc7f1 100644 --- a/lldb/include/lldb/ValueObject/ValueObjectConstResult.h +++ b/lldb/include/lldb/ValueObject/ValueObjectConstResult.h @@ -86,8 +86,7 @@ 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; + AddrAndType 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..c502ec41dfe90 100644 --- a/lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h +++ b/lldb/include/lldb/ValueObject/ValueObjectConstResultChild.h @@ -48,8 +48,7 @@ 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; + AddrAndType 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..0d4c976b193db 100644 --- a/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h +++ b/lldb/include/lldb/ValueObject/ValueObjectConstResultImpl.h @@ -10,6 +10,7 @@ #define LLDB_VALUEOBJECT_VALUEOBJECTCONSTRESULTIMPL_H #include "lldb/Utility/ConstString.h" +#include "lldb/ValueObject/ValueObject.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" @@ -21,7 +22,6 @@ namespace lldb_private { class CompilerType; class DataExtractor; class Status; -class ValueObject; } // namespace lldb_private namespace lldb_private { @@ -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 ValueObject::AddrAndType + 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..d878eb427a18d 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 [value, addr_type] = + 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..e79c3b8939fa6 100644 --- a/lldb/source/Commands/CommandObjectWatchpoint.cpp +++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp @@ -822,7 +822,6 @@ corresponding to the byte size of the data type."); // We passed the sanity check for the command. Proceed to set the // watchpoint now. - lldb::addr_t addr = 0; size_t size = 0; VariableSP var_sp; @@ -861,19 +860,7 @@ corresponding to the byte size of the data type."); CompilerType compiler_type; - if (valobj_sp) { - AddressType addr_type; - addr = valobj_sp->GetAddressOf(false, &addr_type); - if (addr_type == eAddressTypeLoad) { - // We're in business. - // Find out the size of this variable. - size = - m_option_watchpoint.watch_size.GetCurrentValue() == 0 - ? llvm::expectedToOptional(valobj_sp->GetByteSize()).value_or(0) - : m_option_watchpoint.watch_size.GetCurrentValue(); - } - compiler_type = valobj_sp->GetCompilerType(); - } else { + if (!valobj_sp) { const char *error_cstr = error.AsCString(nullptr); if (error_cstr) result.AppendError(error_cstr); @@ -883,6 +870,16 @@ corresponding to the byte size of the data type."); command.GetArgumentAtIndex(0)); return; } + auto [addr, addr_type] = valobj_sp->GetAddressOf(false); + if (addr_type == eAddressTypeLoad) { + // We're in business. + // Find out the size of this variable. + size = + m_option_watchpoint.watch_size.GetCurrentValue() == 0 + ? llvm::expectedToOptional(valobj_sp->GetByteSize()).value_or(0) + : m_option_watchpoint.watch_size.GetCurrentValue(); + } + compiler_type = valobj_sp->GetCompilerType(); // Now it's time to create the watchpoint. uint32_t watch_type = 0; diff --git a/lldb/source/DataFormatters/CXXFunctionPointer.cpp b/lldb/source/DataFormatters/CXXFunctionPointer.cpp index d79fb1d678ebd..ef0d432a32f79 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, func_ptr_address_type] = 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..d7b058d91c4a3 100644 --- a/lldb/source/DataFormatters/FormattersHelpers.cpp +++ b/lldb/source/DataFormatters/FormattersHelpers.cpp @@ -113,15 +113,16 @@ 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; + ValueObject::AddrAndType data_addr; if (valobj.IsPointerType()) - data_addr = valobj.GetPointerValue(&type); + data_addr = valobj.GetPointerValue(); else if (valobj.IsArrayType()) - data_addr = valobj.GetAddressOf(/*scalar_is_load_address=*/true, &type); - if (data_addr != LLDB_INVALID_ADDRESS && type == eAddressTypeFile) - return Address(data_addr, valobj.GetModule()->GetSectionList()); + data_addr = valobj.GetAddressOf(/*scalar_is_load_address=*/true); - return data_addr; + if (data_addr.address != LLDB_INVALID_ADDRESS && + data_addr.type == eAddressTypeFile) + return Address(data_addr.address, valobj.GetModule()->GetSectionList()); + + return data_addr.address; } diff --git a/lldb/source/DataFormatters/TypeFormat.cpp b/lldb/source/DataFormatters/TypeFormat.cpp index f4cb8b46d272a..f563ab076ae7f 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().address); 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..40493df8aec37 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().address == 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..79c804c6c4214 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).address; 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..6b743e29e21f6 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -897,12 +897,13 @@ bool ClangUserExpression::AddArguments(ExecutionContext &exe_ctx, Status object_ptr_error; if (m_ctx_obj) { - AddressType address_type; - object_ptr = m_ctx_obj->GetAddressOf(false, &address_type); - if (object_ptr == LLDB_INVALID_ADDRESS || - address_type != eAddressTypeLoad) + ValueObject::AddrAndType address = m_ctx_obj->GetAddressOf(false); + if (address.address == LLDB_INVALID_ADDRESS || + address.type != eAddressTypeLoad) object_ptr_error = Status::FromErrorString("Can't get context object's " "debuggee address"); + else + object_ptr = address.address; } else { if (m_in_cplusplus_method) { object_ptr = diff --git a/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp b/lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp index d3cdb231fbb01..87c37e576fad0 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 [frame_ptr_addr, addr_type] = 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..826e6ab090e10 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().address; 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 08cfcd4a26b8e..77e0755607a06 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_of_string, addr_type] = + 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_of_string, addr_type] = 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..75b00518aac53 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().address, + 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().address); 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().address, + 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().address, + 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().address, + 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().address, 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 [original_ptr, address_type] = + 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,8 @@ 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().address + 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..24a73717266a4 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().address; 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..e459cb281793a 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().address; 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().address; // 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..c33760eccd127 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().address; 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 29ddeba3df18e..accfd08b30205 100644 --- a/lldb/source/ValueObject/ValueObject.cpp +++ b/lldb/source/ValueObject/ValueObject.cpp @@ -656,9 +656,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().address; return (cstr_address != LLDB_INVALID_ADDRESS); } @@ -707,9 +705,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, addr_type] = + is_pointer_type ? GetPointerValue() : GetAddressOf(true); switch (addr_type) { case eAddressTypeFile: { @@ -901,8 +898,7 @@ ValueObject::ReadPointedString(lldb::WritableDataBufferSP &buffer_sp, const Flags type_flags(GetTypeInfo(&elem_or_pointee_compiler_type)); if (type_flags.AnySet(eTypeIsArray | eTypeIsPointer) && elem_or_pointee_compiler_type.IsCharType()) { - addr_t cstr_address = LLDB_INVALID_ADDRESS; - AddressType cstr_address_type = eAddressTypeInvalid; + AddrAndType cstr_address; size_t cstr_len = 0; bool capped_data = false; @@ -917,14 +913,15 @@ ValueObject::ReadPointedString(lldb::WritableDataBufferSP &buffer_sp, cstr_len = max_length; } } - cstr_address = GetAddressOf(true, &cstr_address_type); + cstr_address = GetAddressOf(true); } else { // We have a pointer - cstr_address = GetPointerValue(&cstr_address_type); + cstr_address = GetPointerValue(); } - if (cstr_address == 0 || cstr_address == LLDB_INVALID_ADDRESS) { - if (cstr_address_type == eAddressTypeHost && is_array) { + if (cstr_address.address == 0 || + cstr_address.address == LLDB_INVALID_ADDRESS) { + if (cstr_address.type == eAddressTypeHost && is_array) { const char *cstr = GetDataExtractor().PeekCStr(0); if (cstr == nullptr) { s << "<invalid address>"; @@ -943,7 +940,7 @@ ValueObject::ReadPointedString(lldb::WritableDataBufferSP &buffer_sp, } } - Address cstr_so_addr(cstr_address); + Address cstr_so_addr(cstr_address.address); DataExtractor data; if (cstr_len > 0 && honor_array) { // I am using GetPointeeData() here to abstract the fact that some @@ -1198,7 +1195,7 @@ llvm::Expected<bool> ValueObject::GetValueAsBool() { return value_or_err->isNonZero(); } if (val_type.IsArrayType()) - return GetAddressOf() != 0; + return GetAddressOf().address != 0; return llvm::make_error<llvm::StringError>("type cannot be converted to bool", llvm::inconvertibleErrorCode()); @@ -1599,70 +1596,55 @@ bool ValueObject::DumpPrintableRepresentation( return var_success; } -addr_t ValueObject::GetAddressOf(bool scalar_is_load_address, - AddressType *address_type) { +ValueObject::AddrAndType +ValueObject::GetAddressOf(bool scalar_is_load_address) { // Can't take address of a bitfield if (IsBitfield()) - return LLDB_INVALID_ADDRESS; + return {}; if (!UpdateValueIfNeeded(false)) - return LLDB_INVALID_ADDRESS; + return {}; switch (m_value.GetValueType()) { case Value::ValueType::Invalid: - return LLDB_INVALID_ADDRESS; + return {}; case Value::ValueType::Scalar: if (scalar_is_load_address) { - if (address_type) - *address_type = eAddressTypeLoad; - return m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS); + return {m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS), + eAddressTypeLoad}; } - break; + return {}; 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.GetScalar().ULongLong(LLDB_INVALID_ADDRESS), + m_value.GetValueAddressType()}; + case Value::ValueType::HostAddress: + return {LLDB_INVALID_ADDRESS, m_value.GetValueAddressType()}; } - 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; - +ValueObject::AddrAndType ValueObject::GetPointerValue() { if (!UpdateValueIfNeeded(false)) - return address; + return {}; switch (m_value.GetValueType()) { case Value::ValueType::Invalid: - return LLDB_INVALID_ADDRESS; + return {}; case Value::ValueType::Scalar: - address = m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS); - break; + return {m_value.GetScalar().ULongLong(LLDB_INVALID_ADDRESS), + GetAddressTypeOfChildren()}; 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 {m_data.GetAddress(&data_offset), GetAddressTypeOfChildren()}; + } } - if (address_type) - *address_type = GetAddressTypeOfChildren(); - - return address; + llvm_unreachable("Unhandled value type!"); } static const char *ConvertBoolean(lldb::LanguageType language_type, @@ -2749,7 +2731,7 @@ ValueObjectSP ValueObject::CreateConstantValue(ConstString name) { valobj_sp = ValueObjectConstResult::Create( exe_ctx.GetBestExecutionContextScope(), GetCompilerType(), name, data, - GetAddressOf()); + GetAddressOf().address); } if (!valobj_sp) { @@ -2875,9 +2857,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 [addr, address_type] = GetAddressOf(/*scalar_is_load_address=*/false); error.Clear(); if (addr != LLDB_INVALID_ADDRESS && address_type != eAddressTypeHost) { switch (address_type) { @@ -2964,8 +2944,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().address; if (ptr_value != LLDB_INVALID_ADDRESS) { Address ptr_addr(ptr_value); @@ -2978,8 +2957,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().address; if (ptr_value != LLDB_INVALID_ADDRESS) { Address ptr_addr(ptr_value); @@ -2991,11 +2969,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_value, addr_type] = GetAddressOf(scalar_is_load_address); if (addr_type == eAddressTypeFile) { lldb::ModuleSP module_sp(GetModule()); if (!module_sp) @@ -3008,8 +2984,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..e29634c2f3322 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().address; switch (parent->GetAddressTypeOfChildren()) { case eAddressTypeFile: { diff --git a/lldb/source/ValueObject/ValueObjectConstResult.cpp b/lldb/source/ValueObject/ValueObjectConstResult.cpp index 01f870529a14d..774749620e0d0 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); +ValueObject::AddrAndType +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..768878984e169 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); +ValueObject::AddrAndType +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..874628774d47e 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) { +ValueObject::AddrAndType +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 {0, eAddressTypeInvalid}; - 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, m_live_address_type}; } size_t ValueObjectConstResultImpl::GetPointeeData(DataExtractor &data, diff --git a/lldb/source/ValueObject/ValueObjectVTable.cpp b/lldb/source/ValueObject/ValueObjectVTable.cpp index 92bd086d88ee4..6f6ee4579cee5 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().address; 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..0da57346212d0 100644 --- a/lldb/test/API/python_api/value/TestValueAPI.py +++ b/lldb/test/API/python_api/value/TestValueAPI.py @@ -269,7 +269,14 @@ def test(self): frame0.FindVariable("another_fixed_int_ptr").GetValue(), "0xaa", ) + 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( - frame0.FindVariable("a_null_int_ptr").GetValue(), - "0x0", + a_null_int_ptr.Dereference().GetLoadAddress(), lldb.LLDB_INVALID_ADDRESS ) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
