[Lldb-commits] [PATCH] Do not parse DIE's outside a compilation units range

[Stephen Wilson]

Recently I hit a case where a lookup into the abbrev table failed.  Consider
the following code in DWARFDebugInfoEntry::FastExtract:

    if (abbrCode)
    {
        uint32_t offset = *offset_ptr;

        m_abbrevDecl = cu->GetAbbreviations()->GetAbbreviationDeclaration(abbrCode);
        
        // Skip all data in the .debug_info for the attributes
        const uint32_t numAttributes = m_abbrevDecl->NumAttributes();

The lookup fails, m_abbrevDecl is NULL, SIGSEGV...

OK, we should probably check that the lookup is good to protect against the
case of corrupt DWARF, but I do not think this lookup should fail in the
normal case.


In DWARFCompileUnit::ExtractDIEsIfNeeded we are relying on a compilation units
DIEs to be terminated by a null entry.  I think the standard is fairly clear
that all sibling chains are to be terminated by null, but at least gcc 4.5.2
disagrees -- the top level chain drops the final entry.  This results in us
interpreting the next compilation unit header as a DIE, thus generating the
bogus abbrCode.

Regardless of whether gcc is right or wrong, we should not overstep a
compilation units extent.  The following patch ensures that we do not attempt
to extract a DIE beyond the length specified for a given DWARFCompileUnit by
ensuring our current offset is strictly less than the start of the next CU.



diff --git a/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp b/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
index 966d28f..1e0431a 100644
--- a/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
+++ b/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
@@ -154,8 +154,11 @@ DWARFCompileUnit::ExtractDIEsIfNeeded (bool cu_die_only)
                         m_offset,
                         cu_die_only);
 
-    // Set the offset to that of the first DIE
+    // Set the offset to that of the first DIE and calculate the start of the
+    // next compilation unit header.
     uint32_t offset = GetFirstDIEOffset();
+    uint32_t next_cu_offset = GetNextCompileUnitOffset();
+
     DWARFDebugInfoEntry die;
         // Keep a flat array of the DIE for binary lookup by DIE offset
 //    Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
@@ -173,7 +176,8 @@ DWARFCompileUnit::ExtractDIEsIfNeeded (bool cu_die_only)
     const DataExtractor& debug_info_data = m_dwarf2Data->get_debug_info_data();
 
     const uint8_t *fixed_form_sizes = DWARFFormValue::GetFixedFormSizesForAddressSize (GetAddressByteSize());
-    while (die.FastExtract (debug_info_data, this, fixed_form_sizes, &offset))
+    while (offset < next_cu_offset &&
+           die.FastExtract (debug_info_data, this, fixed_form_sizes, &offset))
     {
 //        if (log)
 //            log->Printf("0x%8.8x: %*.*s%s%s",
@@ -220,18 +224,22 @@ DWARFCompileUnit::ExtractDIEsIfNeeded (bool cu_die_only)
                 break;  // We are done with this compile unit!
         }
 
-        if (offset > GetNextCompileUnitOffset())
+    }
+
+    // Give a little bit of info if we encounter corrupt DWARF (our offset
+    // should always terminate at or before the start of the next compilation
+    // unit header).
+    if (offset > next_cu_offset)
+    {
+        char path[PATH_MAX];
+        ObjectFile *objfile = m_dwarf2Data->GetObjectFile();
+        if (objfile)
         {
-            char path[PATH_MAX];
-            ObjectFile *objfile = m_dwarf2Data->GetObjectFile();
-            if (objfile)
-            {
-                objfile->GetFileSpec().GetPath(path, sizeof(path));
-            }
-            fprintf (stderr, "warning: DWARF compile unit extends beyond its bounds cu 0x%8.8x at 0x%8.8x in '%s'\n", GetOffset(), offset, path);
-            break;
+            objfile->GetFileSpec().GetPath(path, sizeof(path));
         }
+        fprintf (stderr, "warning: DWARF compile unit extends beyond its bounds cu 0x%8.8x at 0x%8.8x in '%s'\n", GetOffset(), offset, path);
     }
+
     SetDIERelations();
     return m_die_array.size();
 }