01/03: new SectionPart for Section subranges (for effective .debug_types concatenation)

[jankratochvil]

Summary

@clayborg requested more effective loading of concatenated sections. For compressed sections it means one has to measure decompressed size of both to be able to effectively allocate memory for both of them. But current LoadSectionData() can only load the data without knowing first how big it will be. Therefore D51578 introduces SectionReader but it needs to also handle subranges of DWP Section. Despite DWP Section is not supported to be compressed and compressed sections do not need to support subranges to unify the API I had to replace Section * by SectionPart for all the cases.

[labath]

This all still seems very messy to me. It also appears that we will still end up mmapping the object file, and then copying all of the debug info out of it into a heap buffer.

What's the reason we're trying so hard to concatenate things? IIRC, it was because it makes things appear DWARF5-like, but this is now creating a lot of infrastructure that will be completely unused in the dwarf5 case, so I think we're missing that goal. Can't we just admit that we are dealing with two sections here, and use one bit from the user_id_t (or whereever it's needed) to tell which one are we talking about?

[jankratochvil]

In D58330#1400812, @labath wrote:

It also appears that we will still end up mmapping the object file, and then copying all of the debug info out of it into a heap buffer.

In all usual cases at least on Linux LLDB should use just the mmap part, without any copying - see the block on line 691 of SymbolFileDWARF::get_debug_info_data(): https://reviews.llvm.org/D51578#C1361566NL682

The question is how hard to optimize the corner cases of IsInMemory(), SHF_COMPRESSED with .debug_types etc. Originally I did not optimize it but I think this is what @clayborg did not like: https://reviews.llvm.org/D51578#1383458

What's the reason we're trying so hard to concatenate things? IIRC, it was because it makes things appear DWARF5-like, but this is now creating a lot of infrastructure that will be completely unused in the dwarf5 case, so I think we're missing that goal.

In part yes, I agree. I cannot much agree with the "completely unused in the dwarf5 case" as for DWZ I need the same so if we do not implement it for .debug_types I need to implement it for DWZ anyway. Which is why I also do that - to justify some way such refactorization for DWZ. Red Hat does not use .debug_types and DWARF-5 does not have this problem anyway as you say.

Currently DWZ solves it on line 164 - DWZRedirect(): https://reviews.llvm.org/D40474#C1183882NL164
Although there the two sections to be merged (main .debug_info and .debug_info from DWZ common file - from /usr/lib/debug/.dwz/ directory) come from two different files. I haven't coded that yet.
Currently DWZ also needs D40473 full of GetMainDWARF() vs. original GetDWARF() and I hope I could drop this by also merging .debug_abbrev (and maybe some others). But I haven't yet tried to rebase the DWZ patchset on top of this merging patchset if there isn't some catch.

From the higher point of view: .debug_types is not needed thanks to DWARF-5 and Red Hat could replace DWZ with -fdebug-types-section or at least just not to use DWZ -m|--multifile option (the DWZ common files so we would stay at just one file). But then we live in a real world and people want to debug existing code built with .debug_types and RHELs deployed out there do use DWZ with -m|--multifile. And both do work in GDB now.

Can't we just admit that we are dealing with two sections here, and use one bit from the user_id_t (or whereever it's needed) to tell which one are we talking about?

Been there, done that - with offset_t (although by offseting it and not by one bit but that is similar) - the FORWARDER() dispatching of DWARFDataExtractor: https://reviews.llvm.org/D51578?id=170342#C1214469NL1
The problem is that it has some percents of performance impact which @clayborg did not like.
I think user_id_t is too high for this functionality as all the inter-DWARF references already use only offset_t inside the DWARF code.

[labath]

In D58330#1400825, @jankratochvil wrote:

In D58330#1400812, @labath wrote:

It also appears that we will still end up mmapping the object file, and then copying all of the debug info out of it into a heap buffer.

In all usual cases at least on Linux LLDB should use just the mmap part, without any copying - see the block on line 691 of SymbolFileDWARF::get_debug_info_data(): https://reviews.llvm.org/D51578#C1361566NL682

The question is how hard to optimize the corner cases of IsInMemory(), SHF_COMPRESSED with .debug_types etc. Originally I did not optimize it but I think this is what @clayborg did not like: https://reviews.llvm.org/D51578#1383458

Ok, I see what you mean. It's nice that we don't have this regression in the base case. Though I guess we will still do the copying as soon as debug_types sections appear. From one POV, that's fine, because it's a strict improvement over just not supporting debug_types at all. However, the fact that will have to do that in some cases anyway makes me think whether we have the right abstraction here.

What's the reason we're trying so hard to concatenate things? IIRC, it was because it makes things appear DWARF5-like, but this is now creating a lot of infrastructure that will be completely unused in the dwarf5 case, so I think we're missing that goal.

In part yes, I agree. I cannot much agree with the "completely unused in the dwarf5 case" as for DWZ I need the same so if we do not implement it for .debug_types I need to implement it for DWZ anyway. Which is why I also do that - to justify some way such refactorization for DWZ. Red Hat does not use .debug_types and DWARF-5 does not have this problem anyway as you say.

Currently DWZ solves it on line 164 - DWZRedirect(): https://reviews.llvm.org/D40474#C1183882NL164
Although there the two sections to be merged (main .debug_info and .debug_info from DWZ common file - from /usr/lib/debug/.dwz/ directory) come from two different files. I haven't coded that yet.
Currently DWZ also needs D40473 full of GetMainDWARF() vs. original GetDWARF() and I hope I could drop this by also merging .debug_abbrev (and maybe some others). But I haven't yet tried to rebase the DWZ patchset on top of this merging patchset if there isn't some catch.

From the higher point of view: .debug_types is not needed thanks to DWARF-5 and Red Hat could replace DWZ with -fdebug-types-section or at least just not to use DWZ -m|--multifile option (the DWZ common files so we would stay at just one file). But then we live in a real world and people want to debug existing code built with .debug_types and RHELs deployed out there do use DWZ with -m|--multifile. And both do work in GDB now.

I am not suggesting we shouldn't implement debug_types _at all_ because it is "solved" by DWARF5. However, i am questioning the "concatenation is the best vehicle to implement this support because it makes things similar to DWARF5" line of reasoning, because it seems to me that we will be greatly diverging from the DWARF5 code path anyway.

Can't we just admit that we are dealing with two sections here, and use one bit from the user_id_t (or whereever it's needed) to tell which one are we talking about?

Been there, done that - with offset_t (although by offseting it and not by one bit but that is similar) - the FORWARDER() dispatching of DWARFDataExtractor: https://reviews.llvm.org/D51578?id=170342#C1214469NL1
The problem is that it has some percents of performance impact which @clayborg did not like.

The forwarding implementation is the closest to what I have in mind, but it is not exactly that. I am imagining this to work at a slightly different level. Instead of changing the DataExtractor class at all, i'd have a new object, let's call it DataRegistry for the purposes of this explanation. Then when you want to look up some, which can potentially reside in another chunk of data (this should always be clear from the context, because DWARF is not meant to be consumed as concatenated) you do something like:

switch(form) {
case DW_FORM_which_refers_to_this_compile_unit:
  return {current_data_extractor, form_value};
case DW_FORM_which_refers_to_stuff_that_may_live_elsewhere:
  return {data_registry.lookup_the_data_extractor_for_the_potentially_external_entity(form_value), figure_out_the_offset_in_that_entity(form_value)};
}

The registry should contain the code necessary to produce the right section given the information known to the caller (I guess these would be things like the dwarf form, type unit signature, whatever info DWZs use to locate the external data, etc.)

The user_id_t comes into play when you need to "bookmark" a reference to a certain DIE, so that you come back to it later. In this case the value couldn't just be an offset into the debug_info section, but it would also have to contain some breadcrumbs so that you can track down the appropriate section too.

I think user_id_t is too high for this functionality as all the inter-DWARF references already use only offset_t inside the DWARF code.

So how do these offset_t cross-references work? (My apologies, I have successfully blocked out of my memory the inner workings of SymbolFileDWARF.) I assume there has to be a place where you take something which refers to a DIE in a different section, and then add the magic offset to account for the concatenation, because the magic offset will not be present in dwarf. These are the places I would hook into so that instead of a bare offset, they additionally pass a something which identifies the place where that data is supposed to be read from. I think this be the solution most understandable to people looking at the code in the future, as it work the way that dwarf was intended to be parsed, and would be at least rougly similar to the llvm implementation, which doesn't do any kind of concatenation. That alone would be worth a 5% performance hit to me, particularly as I think the hit could be reclaimed by fine-tuning how these translations are made.

I am sorry that we're driving you around like this. I understand it must be extremely frustrating to be working on this feature for so long. I just don't want to add to the mountain of technical debt that we already have when it comes to SymbolFileDWARF, without exploring all possibilities.

[jankratochvil]

In D58330#1400847, @labath wrote:

Ok, I see what you mean. It's nice that we don't have this regression in the base case. Though I guess we will still do the copying as soon as debug_types sections appear.

That is not the case. If both .debug_info and .debug_types are uncompressed then the concatenated section just points to the start of .debug_info and ends at the end of .debug_types section, ignoring some "garbage" between those two sections (containing sections bettern .debug_info and .debug_types).

I will try to investigate your DW_FORM_* suggestion more thoroughly, thanks. It should have the same performance just the code may be more simple.

I assume there has to be a place where you take something which refers to a DIE in a different section, and then add the magic offset to account for the concatenation, because the magic offset will not be present in dwarf.

D40474 patches DWARFFormValue::Reference(). Common DW_FORM_ref* are CU-relative so they need no change. One needs the magic offset (dwz_debug_info_data.GetByteSize()) only for DW_FORM_ref_addr. And then also DWZ-specific DW_FORM_GNU_ref_alt.

[JDevlieghere]

lldb/include/lldb/Symbol/ObjectFile.h

58

Should this be a SectionSP? If not, can we use a reference?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

551

Inver the case and make this an early return?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

34

if (SectionSP section = section_list->FindSectionByType(sect_type, true)) 
  return SectionPart(section.get());

Would be a lot more readable imho.

lldb/source/Symbol/ObjectFile.cpp

753

Shouldn't these be lldbasserts?

[clayborg]

So I question the need for the SectionPart class. Seems like it would be easier to just add extra args to the ReadSectionData calls? Comments?

lldb/include/lldb/Symbol/ObjectFile.h

739

Do we need a class here? Why not just add defaulted arguments?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

58–60

If we keep this class add it to lldb-forward.h and remove this

lldb/include/lldb/Symbol/ObjectFile.h

virtual void PopulateSymtab(lldb_private::ObjectFile *obj_file,
                               lldb_private::Symtab &symtab) = 0;
 
   virtual void PopulateSectionList(lldb_private::ObjectFile *obj_file,
                                    lldb_private::SectionList &section_list) = 0;
 
   virtual ArchSpec GetArchitecture() = 0;
 };
 
+// Specify section optionally with its subrange.
+class SectionPart {
+public:
+  SectionPart();
+  SectionPart(Section *section);
+  SectionPart(Section *section, lldb::addr_t offset, lldb::addr_t length);
+  Section *GetSection() const;
+  lldb::addr_t GetOffsetInSection() const;
+  lldb::addr_t GetOffsetInFile() const;
+  lldb::addr_t GetOffsetInParentSection() const;
+  lldb::addr_t GetLength() const;
+  explicit operator bool() const;
+private:
+  Section *const m_section;
+  // It should match: llvm::DWARFUnitIndex::Entry::SectionContribution
+  // But we need wider lldb::addr_t for: ObjectFileJIT::ReadSectionData
+  const lldb::addr_t m_offset, m_length;
+};
+
 //----------------------------------------------------------------------
 /// @class ObjectFile ObjectFile.h "lldb/Symbol/ObjectFile.h"
 /// A plug-in interface definition class for object file parsers.
 ///
 /// Object files belong to Module objects and know how to extract information
 /// from executable, shared library, and object (.o) files used by operating
 /// system runtime. The symbol table and section list for an object file.
 ///
public:
   // compressed.
   virtual size_t ReadSectionData(Section *section,
                                  lldb::offset_t section_offset, void *dst,
                                  size_t dst_len);
 
   // This function will transparently decompress section data if the section if
   // compressed. Note that for compressed section the resulting data size may
   // be larger than what Section::GetFileSize reports.
-  virtual size_t ReadSectionData(Section *section,
+  virtual size_t ReadSectionData(const SectionPart &section_part,
                                  DataExtractor &section_data);
 
   bool IsInMemory() const { return m_memory_addr != LLDB_INVALID_ADDRESS; }
 
   // Strip linker annotations (such as @@VERSION) from symbol names.
   virtual llvm::StringRef
   StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const {
     return symbol_name;

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

public:
   ObjectFile::Type CalculateType() override;
 
   ObjectFile::Strata CalculateStrata() override;
 
   size_t ReadSectionData(lldb_private::Section *section,
                          lldb::offset_t section_offset, void *dst,
                          size_t dst_len) override;
 
-  size_t ReadSectionData(lldb_private::Section *section,
+  size_t ReadSectionData(const lldb_private::SectionPart &section_part,
                          lldb_private::DataExtractor &section_data) override;
 
   llvm::ArrayRef<elf::ELFProgramHeader> ProgramHeaders();
   lldb_private::DataExtractor GetSegmentData(const elf::ELFProgramHeader &H);
 
   llvm::StringRef
   StripLinkerSymbolAnnotations(llvm::StringRef symbol_name) const override;
 

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

size_t ObjectFileELF::ReadSectionData(Section *section,
 
   // For compressed sections we need to read to full data to be able to
   // decompress.
   DataExtractor data;
   ReadSectionData(section, data);
   return data.CopyData(section_offset, dst_len, dst);
 }
 
-size_t ObjectFileELF::ReadSectionData(Section *section,
+size_t ObjectFileELF::ReadSectionData(const SectionPart &section_part,
                                       DataExtractor &section_data) {
+  Section *section = section_part.GetSection();
   // If some other objectfile owns this data, pass this to them.
   if (section->GetObjectFile() != this)
-    return section->GetObjectFile()->ReadSectionData(section, section_data);
+    return section->GetObjectFile()->ReadSectionData(
+        section_part, section_data);
 
   size_t result = ObjectFile::ReadSectionData(section, section_data);
   if (result == 0 || !section->Test(SHF_COMPRESSED))
     return result;
 
+  if (section_part.GetOffsetInSection() != 0
+      || section_part.GetLength() != section->GetFileSize()) {
+    GetModule()->ReportWarning(
+        "Unable to read only part %" PRIu64 "+%" PRIu64
+        " of compressed section '%s'",
+        section_part.GetOffsetInSection(), section_part.GetLength(),
+        section->GetName().GetCString());
+    section_data.Clear();
+    return 0;
+  }
+
   auto Decompressor = llvm::object::Decompressor::create(
       section->GetName().GetStringRef(),
       {reinterpret_cast<const char *>(section_data.GetDataStart()),
        size_t(section_data.GetByteSize())},
       GetByteOrder() == eByteOrderLittle, GetAddressByteSize() == 8);
   if (!Decompressor) {
     GetModule()->ReportWarning(
         "Unable to initialize decompressor for section '%s': %s",

lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.h

public:
 
   uint32_t GetDependentModules(lldb_private::FileSpecList &files) override;
 
   size_t ReadSectionData(lldb_private::Section *section,
                          lldb::offset_t section_offset, void *dst,
                          size_t dst_len) override;
 
   size_t
-  ReadSectionData(lldb_private::Section *section,
+  ReadSectionData(const lldb_private::SectionPart &section_part,
                   lldb_private::DataExtractor &section_data) override;
 
   lldb_private::Address GetEntryPointAddress() override;
 
   lldb_private::Address GetBaseAddress() override;
 
   ObjectFile::Type CalculateType() override;
 

lldb/source/Plugins/ObjectFile/JIT/ObjectFileJIT.cpp

if (section_offset < file_size) {
 
     memcpy(dst, src, src_len);
     return src_len;
   }
   return 0;
 }
 
 size_t ObjectFileJIT::ReadSectionData(
-    lldb_private::Section *section,
+    const lldb_private::SectionPart &section_part,
     lldb_private::DataExtractor &section_data) {
-  if (section->GetFileSize()) {
-    const void *src = (void *)(uintptr_t)section->GetFileOffset();
+  if (section_part.GetLength()) {
+    const void *src = (void *)(uintptr_t)section_part.GetOffsetInFile();
 
     DataBufferSP data_sp(
-        new lldb_private::DataBufferHeap(src, section->GetFileSize()));
+        new lldb_private::DataBufferHeap(src, section_part.GetLength()));
     if (data_sp) {
       section_data.SetData(data_sp, 0, data_sp->GetByteSize());
       section_data.SetByteOrder(GetByteOrder());
       section_data.SetAddressByteSize(GetAddressByteSize());
       return section_data.GetByteSize();
     }
   }
   section_data.Clear();
   return 0;
 }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

 class DWARFDebugLine;
 class DWARFDebugRangesBase;
 class DWARFDeclContext;
 class DWARFDIECollection;
 class DWARFFormValue;
 class SymbolFileDWARFDebugMap;
 class SymbolFileDWARFDwo;
 class SymbolFileDWARFDwp;
+namespace lldb_private {
+  class SectionPart;
+}
 
 #define DIE_IS_BEING_PARSED ((lldb_private::Type *)1)
 
 class SymbolFileDWARF : public lldb_private::SymbolFile,
                         public lldb_private::UserID {
 public:
   friend class SymbolFileDWARFDebugMap;
   friend class SymbolFileDWARFDwo;
protected:
   };
 
   DISALLOW_COPY_AND_ASSIGN(SymbolFileDWARF);
 
   const lldb_private::DWARFDataExtractor &
   GetCachedSectionData(lldb::SectionType sect_type,
                        DWARFDataSegment &data_segment);
 
-  virtual void LoadSectionData(lldb::SectionType sect_type,
-                               lldb_private::DWARFDataExtractor &data);
+  virtual lldb_private::SectionPart GetSectionPart(
+      lldb::SectionType sect_type) const;
+
+  void LoadSectionData(lldb::SectionType sect_type,
+                       lldb_private::DWARFDataExtractor &data);
+  void LoadSectionData(const lldb_private::SectionPart &section_part,
 
   bool DeclContextMatchesThisSymbolFile(
       const lldb_private::CompilerDeclContext *decl_ctx);
 
   virtual DWARFUnit *
   GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit);
 
   DWARFUnit *GetNextUnparsedDWARFCompileUnit(DWARFUnit *prev_cu);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

uint32_t SymbolFileDWARF::CalculateAbilities() {
   }
   return abilities;
 }
 
 const DWARFDataExtractor &
 SymbolFileDWARF::GetCachedSectionData(lldb::SectionType sect_type,
                                       DWARFDataSegment &data_segment) {
   llvm::call_once(data_segment.m_flag, [this, sect_type, &data_segment] {
-    this->LoadSectionData(sect_type, std::ref(data_segment.m_data));
+    LoadSectionData(GetSectionPart(sect_type), data_segment.m_data);
   });
   return data_segment.m_data;
 }
 
-void SymbolFileDWARF::LoadSectionData(lldb::SectionType sect_type,
-                                      DWARFDataExtractor &data) {
+SectionPart SymbolFileDWARF::GetSectionPart(lldb::SectionType sect_type) const {
   ModuleSP module_sp(m_obj_file->GetModule());
   const SectionList *section_list = module_sp->GetSectionList();
   if (section_list) {
-    SectionSP section_sp(section_list->FindSectionByType(sect_type, true));
-    if (section_sp) {
-      // See if we memory mapped the DWARF segment?
-      if (m_dwarf_data.GetByteSize()) {
-        data.SetData(m_dwarf_data, section_sp->GetOffset(),
-                     section_sp->GetFileSize());
-      } else {
-        if (m_obj_file->ReadSectionData(section_sp.get(), data) == 0)
-          data.Clear();
-      }
-    }
-  }
-}
+    Section *section = section_list->FindSectionByType(sect_type, true).get();
+    if (section)
+      return section;
+  }
+  return SectionPart();
+}
+
+void SymbolFileDWARF::LoadSectionData(lldb::SectionType sect_type,
+                                      DWARFDataExtractor &data) {
+  LoadSectionData(GetSectionPart(sect_type), data);
+}
+
+void SymbolFileDWARF::LoadSectionData(const SectionPart &section_part,
+                                      DWARFDataExtractor &data) {
+  if (!section_part)
+    return;
+
+    data.SetData(m_dwarf_data,
+        section_part.GetOffsetInParentSection(), section_part.GetLength());
+    if (m_obj_file->ReadSectionData(section_part, data) == 0)
 
 const DWARFDataExtractor &SymbolFileDWARF::get_debug_abbrev_data() {
   return GetCachedSectionData(eSectionTypeDWARFDebugAbbrev,
                               m_data_debug_abbrev);
 }
 
 const DWARFDataExtractor &SymbolFileDWARF::get_debug_addr_data() {
   return GetCachedSectionData(eSectionTypeDWARFDebugAddr, m_data_debug_addr);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

public:
 
   const lldb_private::DWARFDataExtractor &get_debug_abbrev_data() override;
   const lldb_private::DWARFDataExtractor &get_debug_addr_data() override;
   const lldb_private::DWARFDataExtractor &get_debug_info_data() override;
   const lldb_private::DWARFDataExtractor &get_debug_str_data() override;
   const lldb_private::DWARFDataExtractor &get_debug_str_offsets_data() override;
 
 protected:
-  void LoadSectionData(lldb::SectionType sect_type,
-                       lldb_private::DWARFDataExtractor &data) override;
+  lldb_private::SectionPart GetSectionPart(
+      lldb::SectionType sect_type) const override;
 
   DIEToTypePtr &GetDIEToType() override;
 
   DIEToVariableSP &GetDIEToVariable() override;
 
   DIEToClangType &GetForwardDeclDieToClangType() override;
 
   ClangTypeToDIE &GetForwardDeclClangTypeToDie() override;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

 
 SymbolFileDWARFDwo::SymbolFileDWARFDwo(ObjectFileSP objfile,
                                        DWARFUnit *dwarf_cu)
     : SymbolFileDWARF(objfile.get()), m_obj_file_sp(objfile),
       m_base_dwarf_cu(dwarf_cu) {
   SetID(((lldb::user_id_t)dwarf_cu->GetOffset()) << 32);
 }
 
-void SymbolFileDWARFDwo::LoadSectionData(lldb::SectionType sect_type,
-                                         DWARFDataExtractor &data) {
+SectionPart
+SymbolFileDWARFDwo::GetSectionPart(lldb::SectionType sect_type) const {
   const SectionList *section_list =
       m_obj_file->GetSectionList(false /* update_module_section_list */);
   if (section_list) {
-    SectionSP section_sp(section_list->FindSectionByType(sect_type, true));
-    if (section_sp) {
-      // See if we memory mapped the DWARF segment?
-      if (m_dwarf_data.GetByteSize()) {
-        data.SetData(m_dwarf_data, section_sp->GetOffset(),
-                     section_sp->GetFileSize());
-        return;
-      }
+    Section *section = section_list->FindSectionByType(sect_type, true).get();
+    if (section)
+      return section;
 
-      if (m_obj_file->ReadSectionData(section_sp.get(), data) != 0)
-        return;
-
-      data.Clear();
-    }
-  }
-
-  SymbolFileDWARF::LoadSectionData(sect_type, data);
+  return SymbolFileDWARF::GetSectionPart(sect_type);
 }
 
 lldb::CompUnitSP
 SymbolFileDWARFDwo::ParseCompileUnit(DWARFUnit *dwarf_cu,
                                      uint32_t cu_idx) {
   assert(GetCompileUnit() == dwarf_cu && "SymbolFileDWARFDwo::ParseCompileUnit "
                                          "called with incompatible compile "
                                          "unit");

 
 const DWARFDataExtractor &SymbolFileDWARFDwo::get_debug_addr_data() {
   // For single file split dwarf case (when we have .dwo sections in a .o),
   // we do not want to use the .debug_addr section from .o file,
   // but want to get one from the final executable.
   // For regular split debug case, .dwo file does not contain the
   // .debug_addr, so we would always fall back to such lookup anyways.
   llvm::call_once(m_data_debug_addr.m_flag, [this] {
-    SymbolFileDWARF::LoadSectionData(eSectionTypeDWARFDebugAddr,
-                                     std::ref(m_data_debug_addr.m_data));
+    LoadSectionData(SymbolFileDWARF::GetSectionPart(eSectionTypeDWARFDebugAddr),
   });
   return m_data_debug_addr.m_data;
 }
 
 const DWARFDataExtractor &SymbolFileDWARFDwo::get_debug_info_data() {
   return GetCachedSectionData(eSectionTypeDWARFDebugInfoDwo, m_data_debug_info);
 }
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.h

 
 class SymbolFileDWARFDwoDwp : public SymbolFileDWARFDwo {
 public:
   SymbolFileDWARFDwoDwp(SymbolFileDWARFDwp *dwp_symfile,
                         lldb::ObjectFileSP objfile, DWARFUnit *dwarf_cu,
                         uint64_t dwo_id);
 
 protected:
-  void LoadSectionData(lldb::SectionType sect_type,
-                       lldb_private::DWARFDataExtractor &data) override;
+  lldb_private::SectionPart GetSectionPart(
+      lldb::SectionType sect_type) const override;
 
   SymbolFileDWARFDwp *m_dwp_symfile;
   uint64_t m_dwo_id;
 };
 
 #endif // SymbolFileDWARFDwoDwp_SymbolFileDWARFDwoDwp_h_

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwoDwp.cpp

 
 SymbolFileDWARFDwoDwp::SymbolFileDWARFDwoDwp(SymbolFileDWARFDwp *dwp_symfile,
                                              ObjectFileSP objfile,
                                              DWARFUnit *dwarf_cu,
                                              uint64_t dwo_id)
     : SymbolFileDWARFDwo(objfile, dwarf_cu), m_dwp_symfile(dwp_symfile),
       m_dwo_id(dwo_id) {}
 
-void SymbolFileDWARFDwoDwp::LoadSectionData(lldb::SectionType sect_type,
-                                            DWARFDataExtractor &data) {
-  if (m_dwp_symfile->LoadSectionData(m_dwo_id, sect_type, data))
-    return;
+SectionPart
+SymbolFileDWARFDwoDwp::GetSectionPart(lldb::SectionType sect_type) const {
+  SectionPart section_part = m_dwp_symfile->GetSectionPart(m_dwo_id, sect_type);
+  if (section_part)
+    return section_part;
 
-  SymbolFileDWARF::LoadSectionData(sect_type, data);
+  return SymbolFileDWARF::GetSectionPart(sect_type);
 }

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.h

 class SymbolFileDWARFDwp {
 public:
   static std::unique_ptr<SymbolFileDWARFDwp>
   Create(lldb::ModuleSP module_sp, const lldb_private::FileSpec &file_spec);
 
   std::unique_ptr<SymbolFileDWARFDwo>
   GetSymbolFileForDwoId(DWARFUnit *dwarf_cu, uint64_t dwo_id);
 
-  bool LoadSectionData(uint64_t dwo_id, lldb::SectionType sect_type,
-                       lldb_private::DWARFDataExtractor &data);
+  lldb_private::SectionPart GetSectionPart(
+      uint64_t dwo_id, lldb::SectionType sect_type) const;
 
 private:
   explicit SymbolFileDWARFDwp(lldb::ModuleSP module_sp,
                               lldb::ObjectFileSP obj_file);
 
+  lldb_private::Section *GetRawSection(lldb::SectionType sect_type) const;
+
+  lldb_private::Section *GetRawSectionHaveLock(
+      lldb::SectionType sect_type) const;
+
   bool LoadRawSectionData(lldb::SectionType sect_type,
                           lldb_private::DWARFDataExtractor &data);
   
   void InitDebugCUIndexMap();
 
   lldb::ObjectFileSP m_obj_file;
 
-  std::mutex m_sections_mutex;
+  mutable std::mutex m_sections_mutex;
   std::map<lldb::SectionType, lldb_private::DWARFDataExtractor> m_sections;
 
   llvm::DWARFUnitIndex m_debug_cu_index;
   std::map<uint64_t, const llvm::DWARFUnitIndex::Entry *> m_debug_cu_index_map;
 };
 
 #endif // SymbolFileDWARFDwp_SymbolFileDWARFDwp_h_

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwp.cpp

 
 std::unique_ptr<SymbolFileDWARFDwo>
 SymbolFileDWARFDwp::GetSymbolFileForDwoId(DWARFUnit *dwarf_cu,
                                           uint64_t dwo_id) {
   return std::unique_ptr<SymbolFileDWARFDwo>(
       new SymbolFileDWARFDwoDwp(this, m_obj_file, dwarf_cu, dwo_id));
 }
 
-bool SymbolFileDWARFDwp::LoadSectionData(
-    uint64_t dwo_id, lldb::SectionType sect_type,
-    lldb_private::DWARFDataExtractor &data) {
-  lldb_private::DWARFDataExtractor section_data;
-  if (!LoadRawSectionData(sect_type, section_data))
-    return false;
+lldb_private::SectionPart SymbolFileDWARFDwp::GetSectionPart(
+    uint64_t dwo_id, lldb::SectionType sect_type) const {
+  lldb_private::Section *section = GetRawSection(sect_type);
+  if (!section)
+    return lldb_private::SectionPart();
 
   auto it = m_debug_cu_index_map.find(dwo_id);
   if (it == m_debug_cu_index_map.end())
-    return false;
+    return lldb_private::SectionPart();
 
   auto *offsets =
       it->second->getOffset(lldbSectTypeToLlvmSectionKind(sect_type));
-  if (offsets) {
-    data.SetData(section_data, offsets->Offset, offsets->Length);
-  } else {
-    data.SetData(section_data, 0, section_data.GetByteSize());
-  }
-  return true;
+  if (offsets)
+    return lldb_private::SectionPart(section,
+        offsets->Offset, offsets->Length);
+  return lldb_private::SectionPart(section);
+
+lldb_private::Section *SymbolFileDWARFDwp::GetRawSection(
+    lldb::SectionType sect_type) const {
+  std::lock_guard<std::mutex> lock(m_sections_mutex);
+  return GetRawSectionHaveLock(sect_type);
+}
+
+lldb_private::Section *
+SymbolFileDWARFDwp::GetRawSectionHaveLock(lldb::SectionType sect_type) const {
+  const lldb_private::SectionList *section_list =
+      m_obj_file->GetSectionList(false /* update_module_section_list */);
+  if (!section_list)
+    return nullptr;
+  return section_list->FindSectionByType(sect_type, true).get();
 }
 
 bool SymbolFileDWARFDwp::LoadRawSectionData(
     lldb::SectionType sect_type, lldb_private::DWARFDataExtractor &data) {
   std::lock_guard<std::mutex> lock(m_sections_mutex);
 
   auto it = m_sections.find(sect_type);
   if (it != m_sections.end()) {
     if (it->second.GetByteSize() == 0)
       return false;
 
     data = it->second;
     return true;
   }
 
-  const lldb_private::SectionList *section_list =
-      m_obj_file->GetSectionList(false /* update_module_section_list */);
-  if (section_list) {
-    lldb::SectionSP section_sp(
-        section_list->FindSectionByType(sect_type, true));
-    if (section_sp) {
-      if (m_obj_file->ReadSectionData(section_sp.get(), data) != 0) {
-        m_sections[sect_type] = data;
-        return true;
-      }
-    }
-  }
+  lldb_private::Section *section = GetRawSectionHaveLock(sect_type);
+  if (section && m_obj_file->ReadSectionData(section, data) != 0) {
   m_sections[sect_type].Clear();
   return false;
 }

lldb/source/Symbol/ObjectFile.cpp

if (IsInMemory()) {
     }
   }
   return 0;
 }
 
 //----------------------------------------------------------------------
 // Get the section data the file on disk
 //----------------------------------------------------------------------
-size_t ObjectFile::ReadSectionData(Section *section,
-                                   DataExtractor &section_data) {
+size_t ObjectFile::ReadSectionData(
+    const SectionPart &section_part, DataExtractor &section_data) {
+  Section *section = section_part.GetSection();
   // If some other objectfile owns this data, pass this to them.
   if (section->GetObjectFile() != this)
-    return section->GetObjectFile()->ReadSectionData(section, section_data);
+    return section->GetObjectFile()->ReadSectionData(
+        section_part, section_data);
 
   if (IsInMemory()) {
     ProcessSP process_sp(m_process_wp.lock());
     if (process_sp) {
       const addr_t base_load_addr =
           section->GetLoadBaseAddress(&process_sp->GetTarget());
       if (base_load_addr != LLDB_INVALID_ADDRESS) {
         DataBufferSP data_sp(
-            ReadMemory(process_sp, base_load_addr, section->GetByteSize()));
+            ReadMemory(process_sp,
+                base_load_addr + section_part.GetOffsetInSection(),
+                section_part.GetLength()));
         if (data_sp) {
           section_data.SetData(data_sp, 0, data_sp->GetByteSize());
           section_data.SetByteOrder(process_sp->GetByteOrder());
           section_data.SetAddressByteSize(process_sp->GetAddressByteSize());
           return section_data.GetByteSize();
         }
       }
     }
-    return GetData(section->GetFileOffset(), section->GetFileSize(),
-                   section_data);
   } else {
     // The object file now contains a full mmap'ed copy of the object file
     // data, so just use this
     if (!section->IsRelocated())
       RelocateSection(section);
-
-    return GetData(section->GetFileOffset(), section->GetFileSize(),
-                   section_data);
   }
+  return GetData(section_part.GetOffsetInFile(), section_part.GetLength(),
+                 section_data);
 }
 
 bool ObjectFile::SplitArchivePathWithObject(const char *path_with_object,
                                             FileSpec &archive_file,
                                             ConstString &archive_object,
                                             bool must_exist) {
   RegularExpression g_object_regex(llvm::StringRef("(.*)\\(([^\\)]+)\\)$"));
   RegularExpression::Match regex_match(2);
void llvm::format_provider<ObjectFile::Strata>::format(
   case ObjectFile::eStrataRawImage:
     OS << "raw image";
     break;
   case ObjectFile::eStrataJIT:
     OS << "jit";
     break;
   }
 }
+
+SectionPart::SectionPart() : m_section(nullptr),
+    m_offset(LLDB_INVALID_ADDRESS), m_length(LLDB_INVALID_ADDRESS) {}
+
+SectionPart::SectionPart(Section *section)
+    : m_section(section), m_offset(0), m_length(section->GetFileSize()) {
+  assert(m_section);
+}
+
+SectionPart::SectionPart(Section *section, lldb::addr_t offset, lldb::addr_t length)
+    : m_section(section), m_offset(offset), m_length(length) {
+  assert(m_section);
+  lldbassert(m_offset + m_length <= m_section->GetFileSize());
+}
+
+Section *SectionPart::GetSection() const {
+  assert(m_section);
+  return m_section;
+}
+
+lldb::addr_t SectionPart::GetOffsetInSection() const {
+  assert(m_section);
+  return m_offset;
+}
+
+lldb::addr_t SectionPart::GetOffsetInFile() const {
+  assert(m_section);
+  return m_section->GetFileOffset() + GetOffsetInSection();
+}
+
+lldb::addr_t SectionPart::GetOffsetInParentSection() const {
+  assert(m_section);
+  return m_section->GetOffset() + GetOffsetInSection();
+}
+
+lldb::addr_t SectionPart::GetLength() const {
+  assert(m_section);
+  return m_length;
+}
+
+SectionPart::operator bool() const {
+  return m_section != nullptr;
+}