https://github.com/bulbazord created https://github.com/llvm/llvm-project/pull/210797
No need to be a ConstString, Mach-O segment names are always 16 bytes long. Because this struct is shared for both segment_command and segment_command_64, I opted to keep the definition instead of replacing it with one of llvm's MachO structs. I chose a 17-byte character array so that whatever goes into it can always be treated as a null-terminated C string. >From d6a580b967913a073f789654bd07c1208f773347 Mon Sep 17 00:00:00 2001 From: Alex Langford <[email protected]> Date: Thu, 16 Jul 2026 14:18:31 -0700 Subject: [PATCH] [lldb] Change DynamicLoaderDarwin::Segment::name type No need to be a ConstString, Mach-O segment names are always 16 bytes long. Because this struct is shared for both segment_command and segment_command_64, I opted to keep the definition instead of replacing it with one of llvm's MachO structs. I chose a 17-byte character array so that whatever goes into it can always be treated as a null-terminated C string. --- .../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 37 ++++++++++--------- .../MacOSX-DYLD/DynamicLoaderDarwin.h | 7 +++- .../MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp | 10 ++--- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp index 9d8de411adee4..f68c3edecc131 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp @@ -311,8 +311,7 @@ bool DynamicLoaderDarwin::UpdateImageLoadAddress(Module *module, // don't map it into lldb's target section load list. if (info.segments[i].vmsize == 0) { LLDB_LOG(log, "{0}: Omitting zero-size segment {1}", - info.file_spec.GetFilename(), - info.segments[i].name.AsCString("")); + info.file_spec.GetFilename(), info.segments[i].name); continue; } @@ -320,8 +319,7 @@ bool DynamicLoaderDarwin::UpdateImageLoadAddress(Module *module, LLDB_LOG(log, "{0}: In-memory segment size for {1} is {2:x}" " but file segment size is {3:x}", - info.file_spec.GetFilename(), - info.segments[i].name.AsCString(""), + info.file_spec.GetFilename(), info.segments[i].name, info.segments[i].vmsize, section_sp->GetByteSize()); changed = m_process->GetTarget().SetSectionLoadAddress( @@ -387,11 +385,11 @@ bool DynamicLoaderDarwin::UnloadModuleSections(Module *module, section_sp, old_section_load_addr)) changed = true; } else { - Debugger::ReportWarning( - llvm::formatv("unable to find and unload segment named " - "'{0}' in '{1}' in macosx dynamic loader plug-in", - info.segments[i].name.AsCString("<invalid>"), - image_object_file->GetFileSpec().GetPath())); + Debugger::ReportWarning(llvm::formatv( + "unable to find and unload segment named " + "'{0}' in '{1}' in macosx dynamic loader plug-in", + info.segments[i].name[0] ? info.segments[i].name : "<invalid>", + image_object_file->GetFileSpec().GetPath())); } } } @@ -522,8 +520,11 @@ bool DynamicLoaderDarwin::JSONImageInformationIntoImageInfo( Segment segment; StructuredData::Dictionary *seg = segments->GetItemAtIndex(j)->GetAsDictionary(); - segment.name = - ConstString(seg->GetValueForKey("name")->GetAsString()->GetValue()); + llvm::StringRef seg_name = + seg->GetValueForKey("name")->GetAsString()->GetValue(); + // segment.name is initialized to all 0s, so we don't need to set the 17th + // byte to 0 after strncpy. + strncpy(segment.name, seg_name.data(), std::min(seg_name.size(), 16ul)); segment.vmaddr = seg->GetValueForKey("vmaddr")->GetUnsignedIntegerValue(); segment.vmsize = seg->GetValueForKey("vmsize")->GetUnsignedIntegerValue(); segment.fileoff = @@ -572,7 +573,7 @@ bool DynamicLoaderDarwin::JSONImageInformationIntoImageInfo( // that starts of file offset zero and that has bytes in the file... if ((image_infos[i].segments[k].fileoff == 0 && image_infos[i].segments[k].filesize > 0) || - (image_infos[i].segments[k].name == "__TEXT")) { + (llvm::StringRef(image_infos[i].segments[k].name) == "__TEXT")) { image_infos[i].slide = image_infos[i].address - image_infos[i].segments[k].vmaddr; // We have found the slide amount, so we can exit this for loop. @@ -871,13 +872,13 @@ bool DynamicLoaderDarwin::AlwaysRelyOnEHUnwindInfo(SymbolContext &sym_ctx) { void DynamicLoaderDarwin::Segment::PutToLog(Log *log, lldb::addr_t slide) const { if (slide == 0) - LLDB_LOGF(log, "\t\t%16s [0x%16.16" PRIx64 " - 0x%16.16" PRIx64 ")", - name.AsCString(""), vmaddr + slide, vmaddr + slide + vmsize); + LLDB_LOGF(log, "\t\t%16s [0x%16.16" PRIx64 " - 0x%16.16" PRIx64 ")", name, + vmaddr + slide, vmaddr + slide + vmsize); else - LLDB_LOGF( - log, - "\t\t%16s [0x%16.16" PRIx64 " - 0x%16.16" PRIx64 ") slide = 0x%" PRIx64, - name.AsCString(""), vmaddr + slide, vmaddr + slide + vmsize, slide); + LLDB_LOGF(log, + "\t\t%16s [0x%16.16" PRIx64 " - 0x%16.16" PRIx64 + ") slide = 0x%" PRIx64, + name, vmaddr + slide, vmaddr + slide + vmsize, slide); } lldb_private::ArchSpec DynamicLoaderDarwin::ImageInfo::GetArchitecture() const { diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h index cf3bb1b718e3a..37130d9326ab9 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h @@ -79,7 +79,9 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader { public: Segment() : name() {} - lldb_private::ConstString name; + // Segment name is 16 characters long. An extra byte is added to guarantee + // null termination in case all 16 bytes are used. + char name[17]; lldb::addr_t vmaddr = LLDB_INVALID_ADDRESS; lldb::addr_t vmsize = 0; lldb::addr_t fileoff = 0; @@ -90,7 +92,8 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader { uint32_t flags = 0; bool operator==(const Segment &rhs) const { - return name == rhs.name && vmaddr == rhs.vmaddr && vmsize == rhs.vmsize; + return llvm::StringRef(name) == llvm::StringRef(rhs.name) && + vmaddr == rhs.vmaddr && vmsize == rhs.vmsize; } void PutToLog(lldb_private::Log *log, lldb::addr_t slide) const; diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp index 3d30df12ba558..9294bbc169683 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp @@ -867,8 +867,8 @@ uint32_t DynamicLoaderMacOSXDYLD::ParseLoadCommands(const DataExtractor &data, load_cmd.cmdsize = data.GetU32(&offset); switch (load_cmd.cmd) { case llvm::MachO::LC_SEGMENT: { - segment.name.SetTrimmedCStringWithLength( - (const char *)data.GetData(&offset, 16), 16); + data.CopyData(offset, 16, segment.name); + offset += 16; // We are putting 4 uint32_t values 4 uint64_t values so we have to use // multiple 32 bit gets below. segment.vmaddr = data.GetU32(&offset); @@ -881,8 +881,8 @@ uint32_t DynamicLoaderMacOSXDYLD::ParseLoadCommands(const DataExtractor &data, } break; case llvm::MachO::LC_SEGMENT_64: { - segment.name.SetTrimmedCStringWithLength( - (const char *)data.GetData(&offset, 16), 16); + data.CopyData(offset, 16, segment.name); + offset += 16; // Extract vmaddr, vmsize, fileoff, and filesize all at once data.GetU64(&offset, &segment.vmaddr, 4); // Extract maxprot, initprot, nsects and flags all at once @@ -924,7 +924,7 @@ uint32_t DynamicLoaderMacOSXDYLD::ParseLoadCommands(const DataExtractor &data, // starts of file offset zero and that has bytes in the file... if ((dylib_info.segments[i].fileoff == 0 && dylib_info.segments[i].filesize > 0) || - (dylib_info.segments[i].name == "__TEXT")) { + (llvm::StringRef(dylib_info.segments[i].name) == "__TEXT")) { dylib_info.slide = dylib_info.address - dylib_info.segments[i].vmaddr; // We have found the slide amount, so we can exit this for loop. break; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
