https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/207296
By having ConstString in this interface, we are forcing callers to create a ConstString in order to perform a lookup. In many cases, `FindSectionByName` is a best-effort interface, meaning we're creating ConstStrings speculatively. In other cases, we were creating a ConstString at the call-site instead of computing it once and re-using it. I audited all the callsites and changed all the callsites that created a ConstString for the purpose of calling `FindSectionByName`. Other callsites will take more time to change. >From 188e671b8daf2327b424c8e59e1a8f80afeb2e8d Mon Sep 17 00:00:00 2001 From: Alex Langford <[email protected]> Date: Thu, 2 Jul 2026 16:50:10 -0700 Subject: [PATCH] [lldb] Remove ConstString from SectionList::FindSectionByName By having ConstString in this interface, we are forcing callers to create a ConstString in order to perform a lookup. In many cases, `FindSectionByName` is a best-effort interface, meaning we're creating ConstStrings speculatively. In other cases, we were creating a ConstString at the call-site instead of computing it once and re-using it. I audited all the callsites and changed all the callsites that created a ConstString for the purpose of calling `FindSectionByName`. Other callsites will take more time to change. --- lldb/include/lldb/Core/Section.h | 2 +- lldb/source/API/SBModule.cpp | 3 +-- lldb/source/API/SBSection.cpp | 4 +--- lldb/source/Commands/CommandObjectTarget.cpp | 3 +-- lldb/source/Core/Section.cpp | 8 ++++---- .../DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp | 6 +++--- .../DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp | 5 +++-- .../ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp | 3 +-- .../ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp | 5 ++--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 8 +++----- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +- lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp | 3 +-- lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp | 2 +- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp | 2 +- lldb/unittests/Core/ModuleTest.cpp | 2 +- lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp | 6 +++--- lldb/unittests/Symbol/LineTableTest.cpp | 3 +-- 17 files changed, 29 insertions(+), 38 deletions(-) diff --git a/lldb/include/lldb/Core/Section.h b/lldb/include/lldb/Core/Section.h index 5823a1719b32f..c29cc51fa899e 100644 --- a/lldb/include/lldb/Core/Section.h +++ b/lldb/include/lldb/Core/Section.h @@ -61,7 +61,7 @@ class SectionList { void Dump(llvm::raw_ostream &s, unsigned indent, Target *target, bool show_header, uint32_t depth) const; - lldb::SectionSP FindSectionByName(ConstString section_dstr) const; + lldb::SectionSP FindSectionByName(llvm::StringRef section_name) const; lldb::SectionSP FindSectionByID(lldb::user_id_t sect_id) const; diff --git a/lldb/source/API/SBModule.cpp b/lldb/source/API/SBModule.cpp index 82634f9d95c8d..d9a427d7e5d0c 100644 --- a/lldb/source/API/SBModule.cpp +++ b/lldb/source/API/SBModule.cpp @@ -557,8 +557,7 @@ SBSection SBModule::FindSection(const char *sect_name) { module_sp->GetSymbolFile(); SectionList *section_list = module_sp->GetSectionList(); if (section_list) { - ConstString const_sect_name(sect_name); - SectionSP section_sp(section_list->FindSectionByName(const_sect_name)); + SectionSP section_sp(section_list->FindSectionByName(sect_name)); if (section_sp) { sb_section.SetSP(section_sp); } diff --git a/lldb/source/API/SBSection.cpp b/lldb/source/API/SBSection.cpp index 451d2052ff7ab..ce2f5f1630749 100644 --- a/lldb/source/API/SBSection.cpp +++ b/lldb/source/API/SBSection.cpp @@ -82,9 +82,7 @@ lldb::SBSection SBSection::FindSubSection(const char *sect_name) { if (sect_name) { SectionSP section_sp(GetSP()); if (section_sp) { - ConstString const_sect_name(sect_name); - sb_section.SetSP( - section_sp->GetChildren().FindSectionByName(const_sect_name)); + sb_section.SetSP(section_sp->GetChildren().FindSectionByName(sect_name)); } } return sb_section; diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp index 2c9e3435e0555..28065a47fd413 100644 --- a/lldb/source/Commands/CommandObjectTarget.cpp +++ b/lldb/source/Commands/CommandObjectTarget.cpp @@ -2991,11 +2991,10 @@ class CommandObjectTargetModulesLoad const char *sect_name = args.GetArgumentAtIndex(i); const char *load_addr_cstr = args.GetArgumentAtIndex(i + 1); if (sect_name && load_addr_cstr) { - ConstString const_sect_name(sect_name); addr_t load_addr; if (llvm::to_integer(load_addr_cstr, load_addr)) { SectionSP section_sp( - section_list->FindSectionByName(const_sect_name)); + section_list->FindSectionByName(sect_name)); if (section_sp) { if (section_sp->IsThreadSpecific()) { result.AppendErrorWithFormat( diff --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp index 8a90c812cd6ac..f3b01429f61ef 100644 --- a/lldb/source/Core/Section.cpp +++ b/lldb/source/Core/Section.cpp @@ -556,21 +556,21 @@ SectionSP SectionList::GetSectionAtIndex(size_t idx) const { return sect_sp; } -SectionSP SectionList::FindSectionByName(ConstString section_dstr) const { +SectionSP SectionList::FindSectionByName(llvm::StringRef section_name) const { SectionSP sect_sp; // Check if we have a valid section string - if (section_dstr && !m_sections.empty()) { + if (!section_name.empty() && !m_sections.empty()) { const_iterator sect_iter; const_iterator end = m_sections.end(); for (sect_iter = m_sections.begin(); sect_iter != end && sect_sp.get() == nullptr; ++sect_iter) { Section *child_section = sect_iter->get(); if (child_section) { - if (child_section->GetName() == section_dstr) { + if (child_section->GetName() == section_name) { sect_sp = *sect_iter; } else { sect_sp = - child_section->GetChildren().FindSectionByName(section_dstr); + child_section->GetChildren().FindSectionByName(section_name); } } } diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp index 14a54e29d3862..53f1ba4038cd4 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp @@ -763,13 +763,13 @@ bool DynamicLoaderDarwin::AddModulesUsingPreloadedModules( if (objfile) { SectionList *sections = objfile->GetSectionList(); if (sections) { - ConstString commpage_dbstr("__commpage"); + llvm::StringRef commpage_sect_name("__commpage"); Section *commpage_section = - sections->FindSectionByName(commpage_dbstr).get(); + sections->FindSectionByName(commpage_sect_name).get(); if (commpage_section) { ModuleSpec module_spec(objfile->GetFileSpec(), image_info.GetArchitecture()); - module_spec.GetObjectName() = commpage_dbstr; + module_spec.GetObjectName() = ConstString(commpage_sect_name); ModuleSP commpage_image_module_sp( target_images.FindFirstModule(module_spec)); if (!commpage_image_module_sp) { diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp index 3246cffaa9e9d..ea74f7729c8db 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp @@ -283,9 +283,10 @@ bool DynamicLoaderMacOSXDYLD::ReadDYLDInfoFromMemoryAndSetNotificationCallback( } if (m_dyld_all_image_infos_addr == LLDB_INVALID_ADDRESS) { - ConstString g_sect_name("__all_image_info"); + llvm::StringRef all_image_info_sect_name("__all_image_info"); SectionSP dyld_aii_section_sp = - dyld_module_sp->GetSectionList()->FindSectionByName(g_sect_name); + dyld_module_sp->GetSectionList()->FindSectionByName( + all_image_info_sect_name); if (dyld_aii_section_sp) { Address dyld_aii_addr(dyld_aii_section_sp, 0); m_dyld_all_image_infos_addr = dyld_aii_addr.GetLoadAddress(&target); diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp index 3029454c493e3..668853f136871 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp @@ -401,8 +401,7 @@ AppleObjCRuntime::GetObjCVersion(Process *process, ModuleSP &objc_module_sp) { SectionList *sections = module_sp->GetSectionList(); if (!sections) return ObjCRuntimeVersions::eObjC_VersionUnknown; - SectionSP v1_telltale_section_sp = - sections->FindSectionByName(ConstString("__OBJC")); + SectionSP v1_telltale_section_sp = sections->FindSectionByName("__OBJC"); if (v1_telltale_section_sp) { return ObjCRuntimeVersions::eAppleObjC_V1; } diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp index 647d8c0972cd3..f1f71f7d7a451 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -2626,13 +2626,12 @@ lldb::addr_t AppleObjCRuntimeV2::GetSharedCacheReadOnlyAddress() { SectionList *section_list = objc_module_sp->GetSectionList(); if (section_list) { - SectionSP text_segment_sp( - section_list->FindSectionByName(ConstString("__TEXT"))); + SectionSP text_segment_sp(section_list->FindSectionByName("__TEXT")); if (text_segment_sp) { SectionSP objc_opt_section_sp( text_segment_sp->GetChildren().FindSectionByName( - ConstString("__objc_opt_ro"))); + "__objc_opt_ro")); if (objc_opt_section_sp) { return objc_opt_section_sp->GetLoadBaseAddress( diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp index 1f8783b8f1fe3..80e8b689d7d4e 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -2197,8 +2197,7 @@ std::shared_ptr<ObjectFileELF> ObjectFileELF::GetGnuDebugDataObjectFile() { if (m_gnu_debug_data_object_file != nullptr) return m_gnu_debug_data_object_file; - SectionSP section = - GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata")); + SectionSP section = GetSectionList()->FindSectionByName(".gnu_debugdata"); if (!section) return nullptr; @@ -3247,8 +3246,7 @@ void ObjectFileELF::ParseSymtab(Symtab &lldb_symtab) { // section, nomatter if .symtab was already parsed or not. This is because // minidebuginfo normally removes the .symtab symbols which have their // matching .dynsym counterparts. - if (!symtab || - GetSectionList()->FindSectionByName(ConstString(".gnu_debugdata"))) { + if (!symtab || GetSectionList()->FindSectionByName(".gnu_debugdata")) { Section *dynsym = section_list->FindSectionByType(eSectionTypeELFDynamicSymbols, true) .get(); @@ -3920,7 +3918,7 @@ ObjectFile::Strata ObjectFileELF::CalculateStrata() { { SectionList *section_list = GetSectionList(); if (section_list) { - static ConstString loader_section_name(".interp"); + llvm::StringRef loader_section_name(".interp"); SectionSP loader_section = section_list->FindSectionByName(loader_section_name); if (loader_section) { diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 327c4dc431fb0..9907a59042a9c 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -5510,7 +5510,7 @@ ObjectFile::Strata ObjectFileMachO::CalculateStrata() { } else { SectionList *section_list = GetSectionList(); if (section_list) { - static ConstString g_kld_section_name("__KLD"); + llvm::StringRef g_kld_section_name("__KLD"); if (section_list->FindSectionByName(g_kld_section_name)) return eStrataKernel; } diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp index 6e8dfcb2648a3..ad58da05576e0 100644 --- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp +++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp @@ -376,8 +376,7 @@ Status PlatformAndroid::DownloadSymbolFile(const lldb::ModuleSP &module_sp, "Symbol file generation only supported on SDK 23+"); // If we already have symtab then we don't have to try and generate one - if (module_sp->GetSectionList()->FindSectionByName(ConstString(".symtab")) != - nullptr) + if (module_sp->GetSectionList()->FindSectionByName(".symtab") != nullptr) return Status::FromErrorString("Symtab already available in the module"); Status error; diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp index 26f8eeaf027d4..8efe164dd2901 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -971,7 +971,7 @@ StructuredData::ArraySP PlatformDarwin::ExtractCrashInfoAnnotations(Process &process) { Log *log = GetLog(LLDBLog::Process); - ConstString section_name("__crash_info"); + llvm::StringRef section_name("__crash_info"); Target &target = process.GetTarget(); StructuredData::ArraySP array_sp = std::make_shared<StructuredData::Array>(); diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 2f0bfa6f751ae..7b3c090286349 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -77,7 +77,7 @@ void HashElfTextSection(ModuleSP module_sp, std::vector<uint8_t> &breakpad_uuid, SectionList *sect_list = module_sp->GetSectionList(); if (sect_list == nullptr) return; - SectionSP sect_sp = sect_list->FindSectionByName(ConstString(".text")); + SectionSP sect_sp = sect_list->FindSectionByName(".text"); if (!sect_sp) return; constexpr size_t kMDGUIDSize = 16; diff --git a/lldb/unittests/Core/ModuleTest.cpp b/lldb/unittests/Core/ModuleTest.cpp index 62cc2c025c863..3a599603ba445 100644 --- a/lldb/unittests/Core/ModuleTest.cpp +++ b/lldb/unittests/Core/ModuleTest.cpp @@ -223,7 +223,7 @@ TEST(ModuleTest, GetSectionListConcurrent) { ... )"; - const ConstString text_name(".text"); + llvm::StringRef text_name(".text"); constexpr int kThreads = 8; // Each iteration uses a fresh module so the lazy build (and its race) is // re-triggered every time. diff --git a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp index 6da2663ff3ae4..957995a18881d 100644 --- a/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp +++ b/lldb/unittests/ObjectFile/ELF/TestObjectFileELF.cpp @@ -94,11 +94,11 @@ TEST_F(ObjectFileELFTest, SectionsResolveConsistently) { SectionList *list = module_sp->GetSectionList(); ASSERT_NE(nullptr, list); - auto bss_sp = list->FindSectionByName(ConstString(".bss")); + auto bss_sp = list->FindSectionByName(".bss"); ASSERT_NE(nullptr, bss_sp); - auto data_sp = list->FindSectionByName(ConstString(".data")); + auto data_sp = list->FindSectionByName(".data"); ASSERT_NE(nullptr, data_sp); - auto text_sp = list->FindSectionByName(ConstString(".text")); + auto text_sp = list->FindSectionByName(".text"); ASSERT_NE(nullptr, text_sp); const Symbol *X = module_sp->FindFirstSymbolWithNameAndType(ConstString("X"), diff --git a/lldb/unittests/Symbol/LineTableTest.cpp b/lldb/unittests/Symbol/LineTableTest.cpp index ca63c6ff51dad..80f2f219d0e81 100644 --- a/lldb/unittests/Symbol/LineTableTest.cpp +++ b/lldb/unittests/Symbol/LineTableTest.cpp @@ -171,8 +171,7 @@ CreateFakeModule(std::vector<LineTable::Sequence> line_sequences) { return file.takeError(); auto module_sp = std::make_shared<Module>(file->moduleSpec()); - SectionSP text_sp = - module_sp->GetSectionList()->FindSectionByName(ConstString(".text")); + SectionSP text_sp = module_sp->GetSectionList()->FindSectionByName(".text"); if (!text_sp) return createStringError("No .text"); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
