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

[Greg Clayton]

Looks good, commit when ready. We have run into such cases as well, so this will be a good fix.

I have been working on our clang and llvm compiler engineers to have the debug builds of the compiler driver include a call to a DWARF verification binary (on MacOSX "dwarfdump --verify --debug-info --eh-frame <dwarf-file>" will verify the DWARF) and flag any .o file that is produced with bad DWARF. It is very common for lexical blocks to have incorrectly scoped hi and low PC values, arrays and other types to be missing a type, and other illegal DWARF.

We need to come up with a centralized place to emit errors and warnings. We should probably add a few static functions to lldb_private::Debugger so we can easily re-route the errors and warnings by changing one function instead of changing  many fprintf to stderr. I am guilty of adding the same kind of code lately, but it would be good to centralize the warning and error output at some point.

Greg Clayton

On May 19, 2011, at 8:59 PM, Stephen Wilson wrote:

> 
> 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();
> }