Author: Jason Molenda Date: 2024-04-03T16:40:34-07:00 New Revision: 622851a9059694487811a7f6078312fc2cce5486
URL: https://github.com/llvm/llvm-project/commit/622851a9059694487811a7f6078312fc2cce5486 DIFF: https://github.com/llvm/llvm-project/commit/622851a9059694487811a7f6078312fc2cce5486.diff LOG: [lldb] Set static Module's load addresses via ObjectFile (#87439) This is a followup to https://github.com/llvm/llvm-project/pull/86359 "[lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware memory (#86359)" where I treat LLVM_COV segments in a Mach-O binary as non-loadable. There is another codepath in `DynamicLoaderStatic::LoadAllImagesAtFileAddresses` which is called to set the load addresses for a Module to the file addresses. It has no logic to detect a segment that is not loaded in virtual memory (ObjectFileMachO::SectionIsLoadable), so it would set the load address for this LLVM_COV segment to the file address and shadow actual code, breaking lldb behavior. This method currently sets the load address for any section that doesn't have a load address set already. This presumes that a Module was added to the Target, some mechanism set the correct load address for SOME segments, and then this method is going to set the other segments to a no-slide value, assuming they were forgotten. ObjectFile base class doesn't, today, vend a SectionIsLoadable method, but we do have ObjectFile::SetLoadAddress and at a higher level, Module::SetLoadAddress, when we're setting the same slide to all segments. That's the behavior we want in this method. If any section has a load address, we don't touch this Module. Otherwise we set all sections to have a load address that is the same as the file address. I also audited the other parts of lldb that are calling SectionList::SectionLoadAddress and looked if they should be more correctly using Module::SetLoadAddress for the entire binary. But in most cases, we have the potential for different slides for different sections so this section-by-section approach must be taken. rdar://125800290 Added: Modified: lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp b/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp index a39aa2280ab86d..545998123dda1b 100644 --- a/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp +++ b/lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp @@ -84,51 +84,43 @@ void DynamicLoaderStatic::LoadAllImagesAtFileAddresses() { // Disable JIT for static dynamic loader targets m_process->SetCanJIT(false); + Target &target = m_process->GetTarget(); for (ModuleSP module_sp : module_list.Modules()) { if (module_sp) { bool changed = false; + bool no_load_addresses = true; + // If this module has a section with a load address set in + // the target, assume all necessary work is already done. There + // may be sections without a load address set intentionally + // and we don't want to mutate that. + // For a module with no load addresses set, set the load addresses + // to slide == 0, the same as the file addresses, in the target. ObjectFile *image_object_file = module_sp->GetObjectFile(); if (image_object_file) { SectionList *section_list = image_object_file->GetSectionList(); if (section_list) { - // All sections listed in the dyld image info structure will all - // either be fixed up already, or they will all be off by a single - // slide amount that is determined by finding the first segment that - // is at file offset zero which also has bytes (a file size that is - // greater than zero) in the object file. - - // Determine the slide amount (if any) const size_t num_sections = section_list->GetSize(); - size_t sect_idx = 0; - for (sect_idx = 0; sect_idx < num_sections; ++sect_idx) { - // Iterate through the object file sections to find the first - // section that starts of file offset zero and that has bytes in - // the file... + for (size_t sect_idx = 0; sect_idx < num_sections; ++sect_idx) { SectionSP section_sp(section_list->GetSectionAtIndex(sect_idx)); if (section_sp) { - // If this section already has a load address set in the target, - // don't re-set it to the file address. Something may have - // set it to a more correct value already. - if (m_process->GetTarget() - .GetSectionLoadList() - .GetSectionLoadAddress(section_sp) != - LLDB_INVALID_ADDRESS) { - continue; + if (target.GetSectionLoadList().GetSectionLoadAddress( + section_sp) != LLDB_INVALID_ADDRESS) { + no_load_addresses = false; + break; } - if (m_process->GetTarget().SetSectionLoadAddress( - section_sp, section_sp->GetFileAddress())) - changed = true; } } } } + if (no_load_addresses) + module_sp->SetLoadAddress(target, 0, true /*value_is_offset*/, changed); if (changed) loaded_module_list.AppendIfNeeded(module_sp); } } - m_process->GetTarget().ModulesDidLoad(loaded_module_list); + target.ModulesDidLoad(loaded_module_list); } ThreadPlanSP _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
