[Lldb-commits] [PATCH] lldb incorrectly parses DWARF information from Go binary

[Jason Molenda]

made two small comments.  Agree with adding clarifying comment about the behavior of Section::GetOffset to the header file.  Not sure I understand why we're changing off of the GetOffset method in SymbolFileDWARF -- it seems like we can use the GetOffset method like before and if this Section has a parent Section, add that parent Section's offset in.

================
Comment at: include/lldb/Core/Section.h:205
@@ -203,1 +204,3 @@
+    // there is no parent then this method returns the offset from
+    // the beginning of the program in memory.
     lldb::addr_t
----------------
Section.h uses the terminology "absolute file virtual address of this section if m_parent == NULL.  offset from parent file virtual address if m_parent != NULL".  I'd use similar wording here, maybe 

The absolute file virtual address of this section if it has no parent Section.  If there is a parent Section, this is the offset from the parent Section's file virtual address."

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:750
@@ +749,3 @@
+                    // Get the offset of this section from the beginning of the file.
+                    lldb::offset_t offset = section_sp->GetFileOffset();
+                    SectionSP parent_sp (section_sp->GetParent());
----------------
Wouldn't it make sense to continue to use the Section::GetOffset() method here?  I'm not clear on why we're changing to GetFileOffset().

================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:756
@@ +755,3 @@
+                    {
+                        offset = offset - parent_sp->GetFileOffset();
+                    }
----------------
If we use Section::GetOffset() above, then this would be

offset = offset + parent_sp-GetOffset();

right?

http://reviews.llvm.org/D5568