[lldb] Section::GetName should return a StringRef#209926
Merged
Merged
Conversation
Section currently owns a ConstString to hold onto its name. I plan on changing this in the future, but first I want to remove ConstString from its API. This commit starts with `GetName`.
|
@llvm/pr-subscribers-lldb Author: Alex Langford (bulbazord) ChangesSection currently owns a ConstString to hold onto its name. I plan on changing this in the future, but first I want to remove ConstString from its API. This commit starts with Full diff: https://github.com/llvm/llvm-project/pull/209926.diff 9 Files Affected:
diff --git a/lldb/include/lldb/Core/Section.h b/lldb/include/lldb/Core/Section.h
index c29cc51fa899e..5395037e7c520 100644
--- a/lldb/include/lldb/Core/Section.h
+++ b/lldb/include/lldb/Core/Section.h
@@ -208,7 +208,7 @@ class Section : public std::enable_shared_from_this<Section>,
bool IsDescendant(const Section *section);
- ConstString GetName() const { return m_name; }
+ llvm::StringRef GetName() const { return m_name; }
bool Slide(lldb::addr_t slide_amount, bool slide_children);
diff --git a/lldb/source/API/SBSection.cpp b/lldb/source/API/SBSection.cpp
index ce2f5f1630749..bc00e32418a9f 100644
--- a/lldb/source/API/SBSection.cpp
+++ b/lldb/source/API/SBSection.cpp
@@ -58,7 +58,7 @@ const char *SBSection::GetName() {
SectionSP section_sp(GetSP());
if (section_sp)
- return section_sp->GetName().GetCString();
+ return ConstString(section_sp->GetName()).GetCString();
return nullptr;
}
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index e733c0f5d4a1a..112c47f0d7ff4 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1668,7 +1668,7 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
void DumpRegion(CommandReturnObject &result, Target &target,
const MemoryRegionInfo &range_info, lldb::addr_t load_addr) {
lldb_private::Address addr;
- ConstString section_name;
+ llvm::StringRef section_name;
if (target.ResolveLoadAddress(load_addr, addr)) {
SectionSP section_sp(addr.GetSection());
if (section_sp) {
@@ -1685,7 +1685,7 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
range_info.GetRange().GetRangeBase(),
range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
- name, section_name ? " " : "", section_name);
+ name, !section_name.empty() ? " " : "", section_name);
LazyBool memory_tagged = range_info.GetMemoryTagged();
if (memory_tagged == eLazyBoolYes)
result.AppendMessage("memory tagging: enabled");
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp b/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
index fbeb569106ba3..8c91c3075a913 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp
@@ -306,7 +306,7 @@ ELFSymbol::sectionIndexToString(elf_half shndx,
const lldb_private::Section *section =
section_list->GetSectionAtIndex(shndx).get();
if (section)
- return section->GetName().GetStringRef();
+ return section->GetName();
} break;
}
return "";
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 58accbeaf2d92..50975bddf7776 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2086,7 +2086,7 @@ static SectionSP FindMatchingSection(const SectionList §ion_list,
SectionSP sect_sp;
addr_t vm_addr = section->GetFileAddress();
- ConstString name = section->GetName();
+ llvm::StringRef name = section->GetName();
offset_t byte_size = section->GetByteSize();
bool thread_specific = section->IsThreadSpecific();
uint32_t permissions = section->GetPermissions();
@@ -2435,7 +2435,7 @@ ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
if (symbol_type == eSymbolTypeInvalid && symbol.getType() != STT_SECTION) {
if (symbol_section_sp) {
- ConstString sect_name = symbol_section_sp->GetName();
+ llvm::StringRef sect_name = symbol_section_sp->GetName();
if (sect_name == text_section_name || sect_name == init_section_name ||
sect_name == fini_section_name || sect_name == ctors_section_name ||
sect_name == dtors_section_name) {
@@ -3371,7 +3371,7 @@ void ObjectFileELF::ParseSymtab(Symtab &lldb_symtab) {
void ObjectFileELF::RelocateSection(lldb_private::Section *section)
{
- static const char *debug_prefix = ".debug";
+ static llvm::StringRef debug_prefix(".debug");
// Set relocated bit so we stop getting called, regardless of whether we
// actually relocate.
@@ -3381,24 +3381,24 @@ void ObjectFileELF::RelocateSection(lldb_private::Section *section)
if (CalculateType() != eTypeObjectFile)
return;
- const char *section_name = section->GetName().GetCString();
+ llvm::StringRef section_name = section->GetName();
// Can't relocate that which can't be named
- if (section_name == nullptr)
+ if (section_name.empty())
return;
// We don't relocate non-debug sections at the moment
- if (strncmp(section_name, debug_prefix, strlen(debug_prefix)))
+ if (!section_name.starts_with(debug_prefix))
return;
// Relocation section names to look for
- std::string needle = std::string(".rel") + section_name;
- std::string needlea = std::string(".rela") + section_name;
+ std::string needle = std::string(".rel") + section_name.str();
+ std::string needlea = std::string(".rela") + section_name.str();
for (SectionHeaderCollIter I = m_section_headers.begin();
I != m_section_headers.end(); ++I) {
if (I->sh_type == SHT_RELA || I->sh_type == SHT_REL) {
- const char *hay_name = I->section_name.GetCString();
- if (hay_name == nullptr)
+ llvm::StringRef hay_name = I->section_name.GetStringRef();
+ if (hay_name.empty())
continue;
if (needle == hay_name || needlea == hay_name) {
const ELFSectionHeader &reloc_header = *I;
@@ -4017,15 +4017,14 @@ size_t ObjectFileELF::ReadSectionData(Section *section,
return result;
auto Decompressor = llvm::object::Decompressor::create(
- section->GetName().GetStringRef(),
+ section->GetName(),
{reinterpret_cast<const char *>(section_data.GetDataStart()),
size_t(section_data.GetByteSize())},
GetByteOrder() == eByteOrderLittle, GetAddressByteSize() == 8);
if (!Decompressor) {
GetModule()->ReportWarning(
"unable to initialize decompressor for section '{0}': {1}",
- section->GetName().GetCString(),
- llvm::toString(Decompressor.takeError()).c_str());
+ section->GetName(), llvm::toString(Decompressor.takeError()).c_str());
section_data.Clear();
return 0;
}
@@ -4035,7 +4034,7 @@ size_t ObjectFileELF::ReadSectionData(Section *section,
if (auto error = Decompressor->decompress(
{buffer_sp->GetBytes(), size_t(buffer_sp->GetByteSize())})) {
GetModule()->ReportWarning("decompression of section '{0}' failed: {1}",
- section->GetName().GetCString(),
+ section->GetName(),
llvm::toString(std::move(error)).c_str());
section_data.Clear();
return 0;
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 420097d96a3ae..23a82735b6c99 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -2000,7 +2000,7 @@ static SymbolType GetSymbolType(const char *&symbol_name,
const SectionSP &symbol_section) {
SymbolType type = eSymbolTypeInvalid;
- const char *symbol_sect_name = symbol_section->GetName().AsCString(nullptr);
+ llvm::StringRef symbol_sect_name = symbol_section->GetName();
if (symbol_section->IsDescendant(text_section_sp.get())) {
if (symbol_section->IsClear(S_ATTR_PURE_INSTRUCTIONS |
S_ATTR_SELF_MODIFYING_CODE |
@@ -2011,8 +2011,7 @@ static SymbolType GetSymbolType(const char *&symbol_name,
} else if (symbol_section->IsDescendant(data_section_sp.get()) ||
symbol_section->IsDescendant(data_dirty_section_sp.get()) ||
symbol_section->IsDescendant(data_const_section_sp.get())) {
- if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__objc") == symbol_sect_name) {
+ if (symbol_sect_name.starts_with("__objc")) {
type = eSymbolTypeRuntime;
if (symbol_name) {
@@ -2037,15 +2036,12 @@ static SymbolType GetSymbolType(const char *&symbol_name,
}
}
}
- } else if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__gcc_except_tab") ==
- symbol_sect_name) {
+ } else if (symbol_sect_name.starts_with("__gcc_except_tab")) {
type = eSymbolTypeException;
} else {
type = eSymbolTypeData;
}
- } else if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__IMPORT") == symbol_sect_name) {
+ } else if (symbol_sect_name.starts_with("__IMPORT")) {
type = eSymbolTypeTrampoline;
}
return type;
@@ -3200,8 +3196,8 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
}
if (type == eSymbolTypeInvalid) {
- const char *symbol_sect_name =
- symbol_section->GetName().AsCString(nullptr);
+ llvm::StringRef symbol_sect_name =
+ symbol_section->GetName();
if (symbol_section->IsDescendant(
text_section_sp.get())) {
if (symbol_section->IsClear(
@@ -3217,26 +3213,19 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
data_dirty_section_sp.get()) ||
symbol_section->IsDescendant(
data_const_section_sp.get())) {
- if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__objc") ==
- symbol_sect_name) {
+ if (symbol_sect_name.starts_with("__objc")) {
type = eSymbolTypeRuntime;
if (TryParseV2ObjCMetadataSymbol(
symbol_name,
symbol_name_non_abi_mangled, type))
demangled_is_synthesized = true;
- } else if (symbol_sect_name &&
- ::strstr(symbol_sect_name,
- "__gcc_except_tab") ==
- symbol_sect_name) {
+ } else if (symbol_sect_name.starts_with("__gcc_except_tab")) {
type = eSymbolTypeException;
} else {
type = eSymbolTypeData;
}
- } else if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__IMPORT") ==
- symbol_sect_name) {
+ } else if (symbol_sect_name.starts_with("__IMPORT"))
type = eSymbolTypeTrampoline;
} else if (symbol_section->IsDescendant(
objc_section_sp.get())) {
@@ -3960,8 +3949,7 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
}
if (type == eSymbolTypeInvalid) {
- const char *symbol_sect_name =
- symbol_section->GetName().AsCString(nullptr);
+ llvm::StringRef symbol_sect_name = symbol_section->GetName();
if (symbol_section->IsDescendant(text_section_sp.get())) {
if (symbol_section->IsClear(S_ATTR_PURE_INSTRUCTIONS |
S_ATTR_SELF_MODIFYING_CODE |
@@ -3974,23 +3962,18 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
data_dirty_section_sp.get()) ||
symbol_section->IsDescendant(
data_const_section_sp.get())) {
- if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__objc") == symbol_sect_name) {
+ if (symbol_sect_name.starts_with("__objc")) {
type = eSymbolTypeRuntime;
if (TryParseV2ObjCMetadataSymbol(
symbol_name, symbol_name_non_abi_mangled, type))
demangled_is_synthesized = true;
- } else if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__gcc_except_tab") ==
- symbol_sect_name) {
+ } else if (symbol_sect_name.starts_with("__gcc_except_tab")) {
type = eSymbolTypeException;
} else {
type = eSymbolTypeData;
}
- } else if (symbol_sect_name &&
- ::strstr(symbol_sect_name, "__IMPORT") ==
- symbol_sect_name) {
+ } else if (symbol_sect_name.starts_with("__IMPORT")) {
type = eSymbolTypeTrampoline;
} else if (symbol_section->IsDescendant(objc_section_sp.get())) {
type = eSymbolTypeRuntime;
@@ -6078,7 +6061,7 @@ CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
addr_t vmaddr = section->GetLoadBaseAddress(&target);
if (vmaddr == LLDB_INVALID_ADDRESS)
continue;
- ConstString name = section->GetName();
+ llvm::StringRef name = section->GetName();
segment_vmaddr seg_vmaddr;
// This is the uncommon case where strncpy is exactly
// the right one, doesn't need to be nul terminated.
@@ -6086,8 +6069,8 @@ CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
// is not guaranteed to be nul-terminated if all 16 characters are
// used.
// coverity[buffer_size_warning]
- strncpy(seg_vmaddr.segname, name.AsCString(nullptr),
- sizeof(seg_vmaddr.segname));
+ strncpy(seg_vmaddr.segname, name.data(),
+ std::min(name.size(), sizeof(seg_vmaddr.segname)));
seg_vmaddr.vmaddr = vmaddr;
seg_vmaddr.unused = 0;
segment_vmaddrs.push_back(seg_vmaddr);
diff --git a/lldb/source/Target/SectionLoadList.cpp b/lldb/source/Target/SectionLoadList.cpp
index 9531d994bb3d0..b4d1161b45aa7 100644
--- a/lldb/source/Target/SectionLoadList.cpp
+++ b/lldb/source/Target/SectionLoadList.cpp
@@ -118,9 +118,8 @@ bool SectionLoadList::SetSectionLoadAddress(const lldb::SectionSP §ion,
"address {0:x16} maps to more than one section: {1}.{2} and "
"{3}.{4}",
load_addr, module_sp->GetFileSpec().GetFilename(),
- section->GetName().GetCString(),
- curr_module_sp->GetFileSpec().GetFilename(),
- ats_pos->second->GetName().GetCString());
+ section->GetName(), curr_module_sp->GetFileSpec().GetFilename(),
+ ats_pos->second->GetName());
}
}
ats_pos->second = section;
@@ -183,12 +182,11 @@ bool SectionLoadList::SetSectionUnloaded(const lldb::SectionSP §ion_sp,
const FileSpec &module_file_spec(section_sp->GetModule()->GetFileSpec());
module_name = module_file_spec.GetPath();
}
- LLDB_LOGF(
- log,
- "SectionLoadList::%s (section = %p (%s.%s), load_addr = 0x%16.16" PRIx64
- ")",
- __FUNCTION__, static_cast<void *>(section_sp.get()),
- module_name.c_str(), section_sp->GetName().AsCString(""), load_addr);
+ LLDB_LOG(log,
+ "SectionLoadList::{0} (section = {1:x} ({2}.{3}), load_addr = "
+ "0x{4,16:x})",
+ __FUNCTION__, static_cast<void *>(section_sp.get()),
+ module_name.c_str(), section_sp->GetName(), load_addr);
}
bool erased = false;
std::lock_guard<std::recursive_mutex> guard(m_mutex);
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e851495a81f38..253107da1551f 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2074,9 +2074,8 @@ size_t Target::ReadMemoryFromFileCache(const Address &addr, void *dst,
if (bytes_read > 0)
return bytes_read;
else
- error = Status::FromErrorStringWithFormat(
- "error reading data from section %s",
- section_sp->GetName().GetCString());
+ error = Status::FromErrorStringWithFormatv(
+ "error reading data from section {0}", section_sp->GetName());
} else
error = Status::FromErrorString("address isn't from a object file");
} else
diff --git a/lldb/tools/lldb-test/lldb-test.cpp b/lldb/tools/lldb-test/lldb-test.cpp
index 45cfb8000a1af..b74ca2fc25b70 100644
--- a/lldb/tools/lldb-test/lldb-test.cpp
+++ b/lldb/tools/lldb-test/lldb-test.cpp
@@ -1005,7 +1005,7 @@ static void dumpSectionList(LinePrinter &Printer, const SectionList &List, bool
AutoIndent Indent(Printer, 2);
Printer.formatLine("Index: {0}", I);
Printer.formatLine("ID: {0:x}", S->GetID());
- Printer.formatLine("Name: {0}", S->GetName().GetStringRef());
+ Printer.formatLine("Name: {0}", S->GetName());
Printer.formatLine("Type: {0}", S->GetTypeAsCString());
Printer.formatLine("Permissions: {0}", GetPermissionsAsCString(S->GetPermissions()));
Printer.formatLine("Thread specific: {0:y}", S->IsThreadSpecific());
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,cpp -- lldb/include/lldb/Core/Section.h lldb/source/API/SBSection.cpp lldb/source/Commands/CommandObjectMemory.cpp lldb/source/Plugins/ObjectFile/ELF/ELFHeader.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Target/SectionLoadList.cpp lldb/source/Target/Target.cpp lldb/tools/lldb-test/lldb-test.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 23a82735b..b05567b35 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3220,12 +3220,13 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
symbol_name,
symbol_name_non_abi_mangled, type))
demangled_is_synthesized = true;
- } else if (symbol_sect_name.starts_with("__gcc_except_tab")) {
+ } else if (symbol_sect_name.starts_with(
+ "__gcc_except_tab")) {
type = eSymbolTypeException;
} else {
type = eSymbolTypeData;
}
- } else if (symbol_sect_name.starts_with("__IMPORT"))
+ } else if (symbol_sect_name.starts_with("__IMPORT"))
type = eSymbolTypeTrampoline;
} else if (symbol_section->IsDescendant(
objc_section_sp.get())) {
|
JDevlieghere
approved these changes
Jul 16, 2026
JDevlieghere
left a comment
JDevlieghere
left a comment
Member
There was a problem hiding this comment.
By itself this slightly pessimizes things, but LGTM with the future direction in mind.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Section currently owns a ConstString to hold onto its name. I plan on changing this in the future, but first I want to remove ConstString from its API. This commit starts with
GetName.