From a76df78470d7994f73df0353225cbddc463cce63 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Thu, 29 Apr 2021 19:13:21 +0200 Subject: [PATCH] [lldb] Make the NSSet formatter faster and less prone to infinite recursion Right now to get the 'NSSet *` pointer value we first derefence it and then take the address of the result. Beside being inefficient this potentially can cause an infinite recursion if the `pointer` value we get is a pointer of a type that the TypeSystem can't derefence. If the pointer is for example some form of `void *` that the dynamic type resolution can't resolve to an actual type, then the `Derefence` call goes back to asking the formatters how to reference it. If the NSSet formatter then checks if it's an NSSet variation under the hood then we just end infinitely often recursion. In practice this seems to happen with some form of Builtin.RawPointer we get from a NSDictionary in Swift. FWIW, no other formatter is doing the same deref->addressOf as here and there doesn't seem to be any specific reason to do so in the git history (it's just part of the initial formatter commit) Reviewed By: JDevlieghere Differential Revision: https://reviews.llvm.org/D101537 --- lldb/source/Plugins/Language/ObjC/NSSet.cpp | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/lldb/source/Plugins/Language/ObjC/NSSet.cpp b/lldb/source/Plugins/Language/ObjC/NSSet.cpp index 4dbbe6fbddff..43ef7b694bbe 100644 --- a/lldb/source/Plugins/Language/ObjC/NSSet.cpp +++ b/lldb/source/Plugins/Language/ObjC/NSSet.cpp @@ -444,18 +444,12 @@ bool lldb_private::formatters::NSSetISyntheticFrontEnd::Update() { if (!valobj_sp) return false; m_exe_ctx_ref = valobj_sp->GetExecutionContextRef(); - Status error; - if (valobj_sp->IsPointerType()) { - valobj_sp = valobj_sp->Dereference(error); - if (error.Fail() || !valobj_sp) - return false; - } - error.Clear(); lldb::ProcessSP process_sp(valobj_sp->GetProcessSP()); if (!process_sp) return false; m_ptr_size = process_sp->GetAddressByteSize(); - uint64_t data_location = valobj_sp->GetAddressOf() + m_ptr_size; + uint64_t data_location = valobj_sp->GetValueAsUnsigned(0) + m_ptr_size; + Status error; if (m_ptr_size == 4) { m_data_32 = new DataDescriptor_32(); process_sp->ReadMemory(data_location, m_data_32, sizeof(DataDescriptor_32), @@ -728,18 +722,12 @@ lldb_private::formatters:: if (!valobj_sp) return false; m_exe_ctx_ref = valobj_sp->GetExecutionContextRef(); - Status error; - if (valobj_sp->IsPointerType()) { - valobj_sp = valobj_sp->Dereference(error); - if (error.Fail() || !valobj_sp) - return false; - } - error.Clear(); lldb::ProcessSP process_sp(valobj_sp->GetProcessSP()); if (!process_sp) return false; m_ptr_size = process_sp->GetAddressByteSize(); - uint64_t data_location = valobj_sp->GetAddressOf() + m_ptr_size; + uint64_t data_location = valobj_sp->GetValueAsUnsigned(0) + m_ptr_size; + Status error; if (m_ptr_size == 4) { m_data_32 = new D32(); process_sp->ReadMemory(data_location, m_data_32, sizeof(D32),