https://github.com/bulbazord updated https://github.com/llvm/llvm-project/pull/172565
>From c1366fcde8126460a6e54442c670b007704cc3bc Mon Sep 17 00:00:00 2001 From: Alex Langford <[email protected]> Date: Tue, 16 Dec 2025 13:14:56 -0800 Subject: [PATCH 1/4] [lldb] Change how Symbol stores symbol re-export info This change is motivated by a recent crash report I received related to re-exported symbols and ConstStrings. I was unable to reproduce the crash so this fix is speculative. Symbols of type eSymbolTypeReExported have metadata associated with them to resolve them to the correct place. The first is the re-exported name, in case the name differs between the library re-exporting it and the library defining it. The second is the library that actually defines the symbol being re-exported. LLDB currently stores this metadata in an unsafe fashion. To store the re-export name, it takes the address of a ConstString's underlying storage and puts it in the Symbol's AddressRange's base Address. The same technique is applied for the library path except it is placed in the AddressRange's size member. The intended way of preventing any potential memory corruption is to call `Symbol::ValueIsAddress` before accessing or modifying its AddressRange information. If done correctly, this allows you to save 2 pointer's worth of space per Symbol object. However, I do not believe that the saved space is worth the risk of getting this wrong. rdar://166452748 --- lldb/include/lldb/Symbol/Symbol.h | 15 ++++++-- lldb/source/Symbol/Symbol.cpp | 59 ++++++++----------------------- 2 files changed, 27 insertions(+), 47 deletions(-) diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index b994c34e46493..90a7303de78ff 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -13,6 +13,7 @@ #include "lldb/Core/Mangled.h" #include "lldb/Core/Section.h" #include "lldb/Symbol/SymbolContextScope.h" +#include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/UserID.h" #include "lldb/lldb-enumerations.h" @@ -154,9 +155,11 @@ class Symbol : public SymbolContextScope { return m_mangled; } - ConstString GetReExportedSymbolName() const; + ConstString GetReExportedSymbolName() const { return m_reexport_name; } - FileSpec GetReExportedSymbolSharedLibrary() const; + FileSpec GetReExportedSymbolSharedLibrary() const { + return m_reexport_library; + } void SetReExportedSymbolName(ConstString name); @@ -312,7 +315,7 @@ class Symbol : public SymbolContextScope { // modules we've already seen to make sure we don't get caught in a cycle. Symbol *ResolveReExportedSymbolInModuleSpec( - Target &target, ConstString &reexport_name, + Target &target, ConstString reexport_name, lldb_private::ModuleSpec &module_spec, lldb_private::ModuleList &seen_modules) const; @@ -347,6 +350,12 @@ class Symbol : public SymbolContextScope { AddressRange m_addr_range; // Contains the value, or the section offset // address when the value is an address in a // section, and the size (if any) + /// Stores the re-exported name if this symbol is of type + /// eSymbolTypeReExported. + ConstString m_reexport_name; + /// Stores the re-exported shared library if this symbol is of type + /// eSymbolTypeReExported. + FileSpec m_reexport_library; uint32_t m_flags = 0; // A copy of the flags from the original symbol table, // the ObjectFile plug-in can interpret these }; diff --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp index 40497dbccc5c3..1c64d413686dd 100644 --- a/lldb/source/Symbol/Symbol.cpp +++ b/lldb/source/Symbol/Symbol.cpp @@ -46,7 +46,8 @@ Symbol::Symbol(uint32_t symID, llvm::StringRef name, SymbolType type, m_demangled_is_synthesized(false), m_contains_linker_annotations(contains_linker_annotations), m_is_weak(false), m_type(type), m_mangled(name), - m_addr_range(section_sp, offset, size), m_flags(flags) {} + m_addr_range(section_sp, offset, size), m_reexport_name(), + m_reexport_library(), m_flags(flags) {} Symbol::Symbol(uint32_t symID, const Mangled &mangled, SymbolType type, bool external, bool is_debug, bool is_trampoline, @@ -61,7 +62,7 @@ Symbol::Symbol(uint32_t symID, const Mangled &mangled, SymbolType type, m_demangled_is_synthesized(false), m_contains_linker_annotations(contains_linker_annotations), m_is_weak(false), m_type(type), m_mangled(mangled), m_addr_range(range), - m_flags(flags) {} + m_reexport_name(), m_reexport_library(), m_flags(flags) {} Symbol::Symbol(const Symbol &rhs) : SymbolContextScope(rhs), m_uid(rhs.m_uid), m_type_data(rhs.m_type_data), @@ -73,7 +74,8 @@ Symbol::Symbol(const Symbol &rhs) m_demangled_is_synthesized(rhs.m_demangled_is_synthesized), m_contains_linker_annotations(rhs.m_contains_linker_annotations), m_is_weak(rhs.m_is_weak), m_type(rhs.m_type), m_mangled(rhs.m_mangled), - m_addr_range(rhs.m_addr_range), m_flags(rhs.m_flags) {} + m_addr_range(rhs.m_addr_range), m_reexport_name(rhs.m_reexport_name), + m_reexport_library(rhs.m_reexport_library), m_flags(rhs.m_flags) {} const Symbol &Symbol::operator=(const Symbol &rhs) { if (this != &rhs) { @@ -93,6 +95,8 @@ const Symbol &Symbol::operator=(const Symbol &rhs) { m_type = rhs.m_type; m_mangled = rhs.m_mangled; m_addr_range = rhs.m_addr_range; + m_reexport_name = rhs.m_reexport_name; + m_reexport_library = rhs.m_reexport_library; m_flags = rhs.m_flags; } return *this; @@ -170,45 +174,14 @@ ConstString Symbol::GetDisplayName() const { return GetMangled().GetDisplayDemangledName(); } -ConstString Symbol::GetReExportedSymbolName() const { - if (m_type == eSymbolTypeReExported) { - // For eSymbolTypeReExported, the "const char *" from a ConstString is used - // as the offset in the address range base address. We can then make this - // back into a string that is the re-exported name. - intptr_t str_ptr = m_addr_range.GetBaseAddress().GetOffset(); - if (str_ptr != 0) - return ConstString((const char *)str_ptr); - else - return GetName(); - } - return ConstString(); -} - -FileSpec Symbol::GetReExportedSymbolSharedLibrary() const { - if (m_type == eSymbolTypeReExported) { - // For eSymbolTypeReExported, the "const char *" from a ConstString is used - // as the offset in the address range base address. We can then make this - // back into a string that is the re-exported name. - intptr_t str_ptr = m_addr_range.GetByteSize(); - if (str_ptr != 0) - return FileSpec((const char *)str_ptr); - } - return FileSpec(); -} - void Symbol::SetReExportedSymbolName(ConstString name) { SetType(eSymbolTypeReExported); - // For eSymbolTypeReExported, the "const char *" from a ConstString is used - // as the offset in the address range base address. - m_addr_range.GetBaseAddress().SetOffset((uintptr_t)name.GetCString()); + m_reexport_name = name; } bool Symbol::SetReExportedSymbolSharedLibrary(const FileSpec &fspec) { if (m_type == eSymbolTypeReExported) { - // For eSymbolTypeReExported, the "const char *" from a ConstString is used - // as the offset in the address range base address. - m_addr_range.SetByteSize( - (uintptr_t)ConstString(fspec.GetPath().c_str()).GetCString()); + m_reexport_library = fspec; return true; } return false; @@ -292,12 +265,11 @@ void Symbol::Dump(Stream *s, Target *target, uint32_t index, " 0x%8.8x %s", m_flags, name.AsCString("")); - ConstString reexport_name = GetReExportedSymbolName(); intptr_t shlib = m_addr_range.GetByteSize(); if (shlib) - s->Printf(" -> %s`%s\n", (const char *)shlib, reexport_name.GetCString()); + s->Printf(" -> %s`%s\n", (const char *)shlib, m_reexport_name.GetCString()); else - s->Printf(" -> %s\n", reexport_name.GetCString()); + s->Printf(" -> %s\n", m_reexport_name.GetCString()); } else { const char *format = m_size_is_sibling @@ -431,7 +403,7 @@ void Symbol::DumpSymbolContext(Stream *s) { lldb::addr_t Symbol::GetByteSize() const { return m_addr_range.GetByteSize(); } Symbol *Symbol::ResolveReExportedSymbolInModuleSpec( - Target &target, ConstString &reexport_name, ModuleSpec &module_spec, + Target &target, ConstString reexport_name, ModuleSpec &module_spec, ModuleList &seen_modules) const { ModuleSP module_sp; if (module_spec.GetFileSpec()) { @@ -481,13 +453,12 @@ Symbol *Symbol::ResolveReExportedSymbolInModuleSpec( } Symbol *Symbol::ResolveReExportedSymbol(Target &target) const { - ConstString reexport_name(GetReExportedSymbolName()); - if (reexport_name) { + if (m_reexport_name) { ModuleSpec module_spec; ModuleList seen_modules; - module_spec.GetFileSpec() = GetReExportedSymbolSharedLibrary(); + module_spec.GetFileSpec() = m_reexport_library; if (module_spec.GetFileSpec()) { - return ResolveReExportedSymbolInModuleSpec(target, reexport_name, + return ResolveReExportedSymbolInModuleSpec(target, m_reexport_name, module_spec, seen_modules); } } >From 498a94bd48eba56231e9b334a5371ff526cb3f3e Mon Sep 17 00:00:00 2001 From: Alex Langford <[email protected]> Date: Tue, 16 Dec 2025 14:59:54 -0800 Subject: [PATCH 2/4] Format --- lldb/source/Symbol/Symbol.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp index 1c64d413686dd..526f49d91e15f 100644 --- a/lldb/source/Symbol/Symbol.cpp +++ b/lldb/source/Symbol/Symbol.cpp @@ -267,7 +267,8 @@ void Symbol::Dump(Stream *s, Target *target, uint32_t index, intptr_t shlib = m_addr_range.GetByteSize(); if (shlib) - s->Printf(" -> %s`%s\n", (const char *)shlib, m_reexport_name.GetCString()); + s->Printf(" -> %s`%s\n", (const char *)shlib, + m_reexport_name.GetCString()); else s->Printf(" -> %s\n", m_reexport_name.GetCString()); } else { >From f46f18662b839d589bc15eaaea202c531eeb5dd2 Mon Sep 17 00:00:00 2001 From: Alex Langford <[email protected]> Date: Wed, 17 Dec 2025 14:47:00 -0800 Subject: [PATCH 3/4] Use getters instead of direct member access --- lldb/include/lldb/Symbol/Symbol.h | 2 +- lldb/source/Symbol/Symbol.cpp | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index 90a7303de78ff..6dbd25aef1873 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -157,7 +157,7 @@ class Symbol : public SymbolContextScope { ConstString GetReExportedSymbolName() const { return m_reexport_name; } - FileSpec GetReExportedSymbolSharedLibrary() const { + const FileSpec &GetReExportedSymbolSharedLibrary() const { return m_reexport_library; } diff --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp index 526f49d91e15f..9014e55dff3c6 100644 --- a/lldb/source/Symbol/Symbol.cpp +++ b/lldb/source/Symbol/Symbol.cpp @@ -265,12 +265,12 @@ void Symbol::Dump(Stream *s, Target *target, uint32_t index, " 0x%8.8x %s", m_flags, name.AsCString("")); - intptr_t shlib = m_addr_range.GetByteSize(); + const FileSpec &shlib = GetReExportedSymbolSharedLibrary(); if (shlib) - s->Printf(" -> %s`%s\n", (const char *)shlib, - m_reexport_name.GetCString()); + s->Printf(" -> %s`%s\n", shlib.GetPath().c_str(), + GetReExportedSymbolName().GetCString()); else - s->Printf(" -> %s\n", m_reexport_name.GetCString()); + s->Printf(" -> %s\n", GetReExportedSymbolName().GetCString()); } else { const char *format = m_size_is_sibling @@ -454,12 +454,13 @@ Symbol *Symbol::ResolveReExportedSymbolInModuleSpec( } Symbol *Symbol::ResolveReExportedSymbol(Target &target) const { - if (m_reexport_name) { + ConstString reexport_name(GetReExportedSymbolName()); + if (reexport_name) { ModuleSpec module_spec; ModuleList seen_modules; - module_spec.GetFileSpec() = m_reexport_library; + module_spec.GetFileSpec() = GetReExportedSymbolSharedLibrary(); if (module_spec.GetFileSpec()) { - return ResolveReExportedSymbolInModuleSpec(target, m_reexport_name, + return ResolveReExportedSymbolInModuleSpec(target, reexport_name, module_spec, seen_modules); } } >From dcabfc2a98b239c98ee37fc5326fccc0bddd0478 Mon Sep 17 00:00:00 2001 From: Alex Langford <[email protected]> Date: Wed, 17 Dec 2025 15:41:38 -0800 Subject: [PATCH 4/4] Implement Jonas's suggestion --- lldb/include/lldb/Symbol/Symbol.h | 18 ++++++------ lldb/source/Symbol/Symbol.cpp | 47 ++++++++++++++++++++++--------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/lldb/include/lldb/Symbol/Symbol.h b/lldb/include/lldb/Symbol/Symbol.h index 6dbd25aef1873..0d84c7abd1814 100644 --- a/lldb/include/lldb/Symbol/Symbol.h +++ b/lldb/include/lldb/Symbol/Symbol.h @@ -155,11 +155,9 @@ class Symbol : public SymbolContextScope { return m_mangled; } - ConstString GetReExportedSymbolName() const { return m_reexport_name; } + ConstString GetReExportedSymbolName() const; - const FileSpec &GetReExportedSymbolSharedLibrary() const { - return m_reexport_library; - } + FileSpec GetReExportedSymbolSharedLibrary() const; void SetReExportedSymbolName(ConstString name); @@ -321,6 +319,11 @@ class Symbol : public SymbolContextScope { void SynthesizeNameIfNeeded() const; + struct ReExportInfo { + ConstString name; + FileSpec library; + }; + uint32_t m_uid = UINT32_MAX; // User ID (usually the original symbol table index) uint16_t m_type_data = 0; // data specific to m_type @@ -350,12 +353,9 @@ class Symbol : public SymbolContextScope { AddressRange m_addr_range; // Contains the value, or the section offset // address when the value is an address in a // section, and the size (if any) - /// Stores the re-exported name if this symbol is of type - /// eSymbolTypeReExported. - ConstString m_reexport_name; - /// Stores the re-exported shared library if this symbol is of type + /// Stores re-export information if this symbol is of type /// eSymbolTypeReExported. - FileSpec m_reexport_library; + std::unique_ptr<ReExportInfo> m_reexport_info; uint32_t m_flags = 0; // A copy of the flags from the original symbol table, // the ObjectFile plug-in can interpret these }; diff --git a/lldb/source/Symbol/Symbol.cpp b/lldb/source/Symbol/Symbol.cpp index 9014e55dff3c6..0b060eba36efb 100644 --- a/lldb/source/Symbol/Symbol.cpp +++ b/lldb/source/Symbol/Symbol.cpp @@ -46,8 +46,8 @@ Symbol::Symbol(uint32_t symID, llvm::StringRef name, SymbolType type, m_demangled_is_synthesized(false), m_contains_linker_annotations(contains_linker_annotations), m_is_weak(false), m_type(type), m_mangled(name), - m_addr_range(section_sp, offset, size), m_reexport_name(), - m_reexport_library(), m_flags(flags) {} + m_addr_range(section_sp, offset, size), m_reexport_info(), + m_flags(flags) {} Symbol::Symbol(uint32_t symID, const Mangled &mangled, SymbolType type, bool external, bool is_debug, bool is_trampoline, @@ -62,7 +62,7 @@ Symbol::Symbol(uint32_t symID, const Mangled &mangled, SymbolType type, m_demangled_is_synthesized(false), m_contains_linker_annotations(contains_linker_annotations), m_is_weak(false), m_type(type), m_mangled(mangled), m_addr_range(range), - m_reexport_name(), m_reexport_library(), m_flags(flags) {} + m_reexport_info(), m_flags(flags) {} Symbol::Symbol(const Symbol &rhs) : SymbolContextScope(rhs), m_uid(rhs.m_uid), m_type_data(rhs.m_type_data), @@ -74,8 +74,10 @@ Symbol::Symbol(const Symbol &rhs) m_demangled_is_synthesized(rhs.m_demangled_is_synthesized), m_contains_linker_annotations(rhs.m_contains_linker_annotations), m_is_weak(rhs.m_is_weak), m_type(rhs.m_type), m_mangled(rhs.m_mangled), - m_addr_range(rhs.m_addr_range), m_reexport_name(rhs.m_reexport_name), - m_reexport_library(rhs.m_reexport_library), m_flags(rhs.m_flags) {} + m_addr_range(rhs.m_addr_range), m_reexport_info(), m_flags(rhs.m_flags) { + if (rhs.m_reexport_info) + m_reexport_info = std::make_unique<ReExportInfo>(*rhs.m_reexport_info); +} const Symbol &Symbol::operator=(const Symbol &rhs) { if (this != &rhs) { @@ -95,8 +97,8 @@ const Symbol &Symbol::operator=(const Symbol &rhs) { m_type = rhs.m_type; m_mangled = rhs.m_mangled; m_addr_range = rhs.m_addr_range; - m_reexport_name = rhs.m_reexport_name; - m_reexport_library = rhs.m_reexport_library; + if (rhs.m_reexport_info) + m_reexport_info = std::make_unique<ReExportInfo>(*rhs.m_reexport_info); m_flags = rhs.m_flags; } return *this; @@ -174,17 +176,36 @@ ConstString Symbol::GetDisplayName() const { return GetMangled().GetDisplayDemangledName(); } +ConstString Symbol::GetReExportedSymbolName() const { + if (!m_reexport_info) + return ConstString(); + + return m_reexport_info->name; +} + +FileSpec Symbol::GetReExportedSymbolSharedLibrary() const { + if (!m_reexport_info) + return FileSpec(); + + return m_reexport_info->library; +} + void Symbol::SetReExportedSymbolName(ConstString name) { SetType(eSymbolTypeReExported); - m_reexport_name = name; + if (!m_reexport_info) + m_reexport_info = std::make_unique<ReExportInfo>(); + m_reexport_info->name = name; } bool Symbol::SetReExportedSymbolSharedLibrary(const FileSpec &fspec) { - if (m_type == eSymbolTypeReExported) { - m_reexport_library = fspec; - return true; - } - return false; + if (m_type != eSymbolTypeReExported) + return false; + + if (!m_reexport_info) + m_reexport_info = std::make_unique<ReExportInfo>(); + + m_reexport_info->library = fspec; + return true; } uint32_t Symbol::GetSiblingIndex() const { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
