[Lldb-commits] [PATCH] D41169: ObjectFile: remove ReadSectionData/MemoryMapSectionData mutual recursion

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added a reviewer: clayborg.
Herald added subscribers: JDevlieghere, emaste.

These two functions were calling each other, while handling different
branches of the if(IsInMemory()). I am assuming this had a reason at
some point in the past, but right now it's just confusing.

I resolve this by removing the MemoryMapSectionData function and
inlining the !IsInMemory branch into ReadSectionData. There isn't
anything mmap-related in this function anyway, as the decision whether
to mmap is handled at a higher level.

This is a preparatory step to make ObjectFileELF be able to decompress
compressed sections (I want to make sure that all calls reading section
data are routed through a single piece of code).


https://reviews.llvm.org/D41169

Files:
  include/lldb/Symbol/ObjectFile.h
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Symbol/ObjectFile.cpp


Index: source/Symbol/ObjectFile.cpp
===================================================================
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -561,20 +561,6 @@
   } else {
     // The object file now contains a full mmap'ed copy of the object file 
data,
     // so just use this
-    return MemoryMapSectionData(section, section_data);
-  }
-}
-
-size_t ObjectFile::MemoryMapSectionData(Section *section,
-                                        DataExtractor &section_data) {
-  // If some other objectfile owns this data, pass this to them.
-  if (section->GetObjectFile() != this)
-    return section->GetObjectFile()->MemoryMapSectionData(section,
-                                                          section_data);
-
-  if (IsInMemory()) {
-    return ReadSectionData(section, section_data);
-  } else {
     if (!section->IsRelocated())
       RelocateSection(section);
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -444,10 +444,8 @@
     Section *section =
         section_list->FindSectionByName(GetDWARFMachOSegmentName()).get();
 
-    // Memory map the DWARF mach-o segment so we have everything mmap'ed
-    // to keep our heap memory usage down.
     if (section)
-      m_obj_file->MemoryMapSectionData(section, m_dwarf_data);
+      m_obj_file->ReadSectionData(section, m_dwarf_data);
   }
 
   get_apple_names_data();
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1830,8 +1830,6 @@
 }
 
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
-  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
-
   if (!m_sections_ap.get() && ParseSectionHeaders()) {
     m_sections_ap.reset(new SectionList());
 
Index: include/lldb/Symbol/ObjectFile.h
===================================================================
--- include/lldb/Symbol/ObjectFile.h
+++ include/lldb/Symbol/ObjectFile.h
@@ -805,9 +805,6 @@
   virtual size_t ReadSectionData(Section *section,
                                  DataExtractor &section_data);
 
-  size_t MemoryMapSectionData(Section *section,
-                              DataExtractor &section_data);
-
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.



Index: source/Symbol/ObjectFile.cpp
===================================================================
--- source/Symbol/ObjectFile.cpp
+++ source/Symbol/ObjectFile.cpp
@@ -561,20 +561,6 @@
   } else {
     // The object file now contains a full mmap'ed copy of the object file data,
     // so just use this
-    return MemoryMapSectionData(section, section_data);
-  }
-}
-
-size_t ObjectFile::MemoryMapSectionData(Section *section,
-                                        DataExtractor &section_data) {
-  // If some other objectfile owns this data, pass this to them.
-  if (section->GetObjectFile() != this)
-    return section->GetObjectFile()->MemoryMapSectionData(section,
-                                                          section_data);
-
-  if (IsInMemory()) {
-    return ReadSectionData(section, section_data);
-  } else {
     if (!section->IsRelocated())
       RelocateSection(section);
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -444,10 +444,8 @@
     Section *section =
         section_list->FindSectionByName(GetDWARFMachOSegmentName()).get();
 
-    // Memory map the DWARF mach-o segment so we have everything mmap'ed
-    // to keep our heap memory usage down.
     if (section)
-      m_obj_file->MemoryMapSectionData(section, m_dwarf_data);
+      m_obj_file->ReadSectionData(section, m_dwarf_data);
   }
 
   get_apple_names_data();
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1830,8 +1830,6 @@
 }
 
 void ObjectFileELF::CreateSections(SectionList &unified_section_list) {
-  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES);
-
   if (!m_sections_ap.get() && ParseSectionHeaders()) {
     m_sections_ap.reset(new SectionList());
 
Index: include/lldb/Symbol/ObjectFile.h
===================================================================
--- include/lldb/Symbol/ObjectFile.h
+++ include/lldb/Symbol/ObjectFile.h
@@ -805,9 +805,6 @@
   virtual size_t ReadSectionData(Section *section,
                                  DataExtractor &section_data);
 
-  size_t MemoryMapSectionData(Section *section,
-                              DataExtractor &section_data);
-
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits