[Lldb-commits] [PATCH] D12291: Add split dwarf support to SymbolFileDWARF

[Tamas Berghammer via lldb-commits]

tberghammer added a comment.

In the current version of the patch the compile units in the main object file hands out only the compile unit DIE with the information what is available in the main object file. I considered the other approach (hand out all DIEs by the DWARF compile unit in the main object file) but I dropped it for the following reasons:

- If we hand out DIEs from a dwo symbol file then each DIE have to store a pointer to the symbol file (or compile unit) it belongs to what is a significant memory overhead (we have more DIEs then Symbols) if we ever want to store all DIE in memory. Even worse is that we have to change all name to DIE index to contain a pointer (compile unit / dwo symbol file) and an offset to find the DIE belongs to (compared to just an offset now) what is more entry then the number DIEs.
- In an average debug session run from an IDE the user usually sets the breakpoints based on file name + line number, display some stack traces, some variables and do some stepping. If we can index each dwo file separately then we can handle all of these features without parsing the full debug info what can give us some significant speed benefits at debugger startup time.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:208-214
@@ -202,1 +207,9 @@
+    
+    SymbolFileDWARFDwo*
+    GetDwoSymbolFile()
+    {
+        // Extract the compile unit DIE as that one contains the dwo symbol file information
+        ExtractDIEsIfNeeded(true);
+        return m_dwo_symbol_file.get();
+    }
 
----------------
clayborg wrote:
> Make protected and hide this from anyone but DWARFCompileUnit and DWARFDebugInfoEntry.
I don't really like friend classes and have the opinion that inside a plugin we can let the visibility a bit more open, but don't feel to strongly about it. I can change it if zou prefer that way.

================
Comment at: source/Symbol/ObjectFile.cpp:604-625
@@ -603,15 +603,23 @@
 
 SectionList *
-ObjectFile::GetSectionList()
+ObjectFile::GetSectionList(bool update_module_section_list)
 {
     if (m_sections_ap.get() == nullptr)
     {
-        ModuleSP module_sp(GetModule());
-        if (module_sp)
+        if (update_module_section_list)
         {
-            lldb_private::Mutex::Locker locker(module_sp->GetMutex());
-            CreateSections(*module_sp->GetUnifiedSectionList());
+            ModuleSP module_sp(GetModule());
+            if (module_sp)
+            {
+                lldb_private::Mutex::Locker locker(module_sp->GetMutex());
+                CreateSections(*module_sp->GetUnifiedSectionList());
+            }
+        }
+        else
+        {
+            SectionList unified_section_list;
+            CreateSections(unified_section_list);
         }
     }
     return m_sections_ap.get();
 }
----------------
clayborg wrote:
> Can you explain why this is needed?
We create all of the dwo object files with the same module what we used for the executable object file (belongs to them). Because of it we have several section with the same section name (several dwo file + object file) what can't be stored inside a single module.

This check is to disable adding the sections in the dwo object file into the modules section list because they will be fetched by SymbolFileDWARFDwo::GetCachedSectionData what handles it correctly.


http://reviews.llvm.org/D12291