[Lldb-commits] [lldb] 622851a - [lldb] Set static Module's load addresses via ObjectFile (#87439)

[via lldb-commits]

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