https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96983
>From 6bd566504355e8d50b9c922df9ebce18e07a726f Mon Sep 17 00:00:00 2001 From: Jason Molenda <jmole...@apple.com> Date: Thu, 27 Jun 2024 15:34:48 -0700 Subject: [PATCH 1/2] [lldb] [ObjectFileMachO] BSS segments are loadable segments ObjectFileMachO::SetLoadAddress sets the address of each segment in a binary in a Target, but it ignores segments that are not loaded in the virtual address space. It was marking segments that were purely BSS -- having no content in the file, but in zero-initialized memory when running in the virtual address space -- as not-loadable, unless they were named "DATA". This works pretty well for typical userland binaries, but in less Darwin environments, there may be BSS segments with other names, that ARE loadable. I looked at the origin of SectionIsLoadable's check for this, and it was a cleanup by Greg in 2018 where we had three different implementations of the idea in ObjectFileMachO and one of them skipped zero-file-size segments (BSS), which made it into the centralized SectionIsLoadable method. Also add some logging to the DynamicLoader log channel when loading a binary - it's the first place I look when debugging segment address setting bugs, and it wasn't emitting anything. rdar://129870649 --- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 2979bf69bf762..164c4409747e0 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6159,10 +6159,6 @@ Section *ObjectFileMachO::GetMachHeaderSection() { bool ObjectFileMachO::SectionIsLoadable(const Section *section) { if (!section) return false; - const bool is_dsym = (m_header.filetype == MH_DSYM); - if (section->GetFileSize() == 0 && !is_dsym && - section->GetName() != GetSegmentNameDATA()) - return false; if (section->IsThreadSpecific()) return false; if (GetModule().get() != section->GetModule().get()) @@ -6202,6 +6198,7 @@ lldb::addr_t ObjectFileMachO::CalculateSectionLoadAddressForMemoryImage( bool ObjectFileMachO::SetLoadAddress(Target &target, lldb::addr_t value, bool value_is_offset) { + Log *log(GetLog(LLDBLog::DynamicLoader)); ModuleSP module_sp = GetModule(); if (!module_sp) return false; @@ -6217,17 +6214,37 @@ bool ObjectFileMachO::SetLoadAddress(Target &target, lldb::addr_t value, // malformed. const bool warn_multiple = true; + if (log) { + std::string binary_description; + if (GetFileSpec()) { + binary_description += "path='"; + binary_description += GetFileSpec().GetPath(); + binary_description += "' "; + } + if (GetUUID()) { + binary_description += "uuid="; + binary_description += GetUUID().GetAsString(); + } + LLDB_LOGF(log, "ObjectFileMachO::SetLoadAddress %s", + binary_description.c_str()); + } if (value_is_offset) { // "value" is an offset to apply to each top level segment for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx) { // Iterate through the object file sections to find all of the // sections that size on disk (to avoid __PAGEZERO) and load them SectionSP section_sp(section_list->GetSectionAtIndex(sect_idx)); - if (SectionIsLoadable(section_sp.get())) + if (SectionIsLoadable(section_sp.get())) { + LLDB_LOGF(log, + "ObjectFileMachO::SetLoadAddress segment '%s' load addr is " + "0x%" PRIx64, + section_sp->GetName().AsCString(), + section_sp->GetFileAddress() + value); if (target.GetSectionLoadList().SetSectionLoadAddress( section_sp, section_sp->GetFileAddress() + value, warn_multiple)) ++num_loaded_sections; + } } } else { // "value" is the new base address of the mach_header, adjust each @@ -6242,6 +6259,10 @@ bool ObjectFileMachO::SetLoadAddress(Target &target, lldb::addr_t value, CalculateSectionLoadAddressForMemoryImage( value, mach_header_section, section_sp.get()); if (section_load_addr != LLDB_INVALID_ADDRESS) { + LLDB_LOGF(log, + "ObjectFileMachO::SetLoadAddress segment '%s' load addr is " + "0x%" PRIx64, + section_sp->GetName().AsCString(), section_load_addr); if (target.GetSectionLoadList().SetSectionLoadAddress( section_sp, section_load_addr, warn_multiple)) ++num_loaded_sections; >From 2a7b154cbdffb75d763d3f8587266c9a7c0a0eb0 Mon Sep 17 00:00:00 2001 From: Jason Molenda <ja...@molenda.com> Date: Mon, 1 Jul 2024 17:29:41 -0700 Subject: [PATCH 2/2] Build up the log message in a StreamString which is a little more compact. --- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 164c4409747e0..0dcb1bed23548 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6215,18 +6215,14 @@ bool ObjectFileMachO::SetLoadAddress(Target &target, lldb::addr_t value, const bool warn_multiple = true; if (log) { - std::string binary_description; - if (GetFileSpec()) { - binary_description += "path='"; - binary_description += GetFileSpec().GetPath(); - binary_description += "' "; - } + StreamString logmsg; + logmsg << "ObjectFileMachO::SetLoadAddress "; + if (GetFileSpec()) + logmsg << "path='" << GetFileSpec().GetPath() << "' "; if (GetUUID()) { - binary_description += "uuid="; - binary_description += GetUUID().GetAsString(); + logmsg << "uuid=" << GetUUID().GetAsString(); } - LLDB_LOGF(log, "ObjectFileMachO::SetLoadAddress %s", - binary_description.c_str()); + LLDB_LOGF(log, "%s", logmsg.GetData()); } if (value_is_offset) { // "value" is an offset to apply to each top level segment _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits