Questions about DWARFDebugInfoEntry::Extract and DWARFDebugInfoEntry::FastExtract

I hit an infinite loop in DWARFDebugInfoEntry::FastExtract when consuming some of our sym files containing DW_FORM_indirect attribute forms. In fixing the problem, I saw DWARFDebugInfoEntry::Extract which does pretty much the same thing (and did not have the bug). But looking at the call graph, it doesn’t look like Extract is called, at least not by any functions that are called themselves (see call graph at the bottom). It’s basically used by Dump and Verify and helpers for those methods.

So the question is, should I get rid of Extract and change everything to use FastExtract, or just fix the problem in FastExtract and leave everything else be?

Thanks,
Warren

DWARFDebugInfoEntry::Extract(SymbolFileDWARF *, const DWARFCompileUnit *, dw_offset_t *)
DWARFDebugInfo::Parse(SymbolFileDWARF *, Callback, void *)
DWARFDebugInfo::Dump(lldb_private::Stream *, SymbolFileDWARF *, unsigned int, unsigned int)
DWARFDebugInfo::Find(const char *, bool, std::vector<unsigned int,std::allocator> &)
DWARFDebugInfo::Find(lldb_private::RegularExpression &, std::vector<unsigned int,std::allocator> &)
DWARFDebugInfo::Verify(lldb_private::Stream *, SymbolFileDWARF *)
DWARFDebugInfoEntry::AppendTypeName(SymbolFileDWARF *, const DWARFCompileUnit *, unsigned int, lldb_private::Stream *)
DWARFDebugInfoEntry::AppendTypeName(SymbolFileDWARF *, const DWARFCompileUnit *, unsigned int, lldb_private::Stream *)
DWARFDebugInfoEntry::DumpAttribute(SymbolFileDWARF *, const DWARFCompileUnit *, const lldb_private::DataExtractor &, uint32_t *, lldb_private::Stream *, dw_attr_t, dw_form_t)
DWARFDebugInfoEntry::Dump(SymbolFileDWARF *, const DWARFCompileUnit *, lldb_private::Stream *, uint32_t)
DumpCallback(SymbolFileDWARF *, DWARFCompileUnitSP &, DWARFDebugInfoEntry *, unsigned int, unsigned int, void *) (4 matches)
DWARFDebugInfo::Dump(lldb_private::Stream *, SymbolFileDWARF *, unsigned int, unsigned int)
DWARFDebugInfo::Dump(lldb_private::Stream *, unsigned int, unsigned int)
DWARFDebugInfo::Dump(lldb_private::Stream *, unsigned int, unsigned int)
DWARFDebugInfoEntry::Dump(SymbolFileDWARF *, const DWARFCompileUnit *, lldb_private::Stream *, uint32_t)
DWARFDebugInfoEntry::DumpAncestry(SymbolFileDWARF *, const DWARFCompileUnit *, const DWARFDebugInfoEntry *, lldb_private::Stream *, uint32_t)
DWARFDebugInfoEntry::DumpAncestry(SymbolFileDWARF *, const DWARFCompileUnit *, const DWARFDebugInfoEntry *, lldb_private::Stream *, uint32_t)
DWARFDebugInfoEntry::GetName(SymbolFileDWARF *, const DWARFCompileUnit *, unsigned int, lldb_private::Stream *)
DWARFDebugInfoEntry::DumpAttribute(SymbolFileDWARF *, const DWARFCompileUnit *, const lldb_private::DataExtractor &, uint32_t *, lldb_private::Stream *, dw_attr_t, dw_form_t)
DWARFDebugInfoEntry::Dump(SymbolFileDWARF *, const DWARFCompileUnit *, lldb_private::Stream *, uint32_t)
DumpCallback(SymbolFileDWARF *, DWARFCompileUnitSP &, DWARFDebugInfoEntry *, unsigned int, unsigned int, void *) (4 matches)
DWARFDebugInfo::Dump(lldb_private::Stream *, SymbolFileDWARF *, unsigned int, unsigned int)
DWARFDebugInfo::Dump(lldb_private::Stream *, unsigned int, unsigned int)
DWARFDebugInfo::Dump(lldb_private::Stream *, unsigned int, unsigned int)
DWARFDebugInfoEntry::Dump(SymbolFileDWARF *, const DWARFCompileUnit *, lldb_private::Stream *, uint32_t)
DWARFDebugInfoEntry::DumpAncestry(SymbolFileDWARF *, const DWARFCompileUnit *, const DWARFDebugInfoEntry *, lldb_private::Stream *, uint32_t)
DWARFDebugInfoEntry::DumpAncestry(SymbolFileDWARF *, const DWARFCompileUnit *, const DWARFDebugInfoEntry *, lldb_private::Stream *, uint32_t)

The fast path currently ignores checking if offsets are valid while parsing a single DIE which can be bad if we have bad DWARF. So both should stay.

Send a patch to the list for one and I can fix it in both!

If you have an example file that contains DWARF with this issue, I wouldn't mind getting my hands on a few more external DWARF files so I can test my DWARF tools with them. I am not sure if you can spare any sample DWARF files if they don't contain anything proprietary, but if you can, please email me some directly.

Greg Clayton

Hi Greg,

Attached is patch that fixes a few issues:

1) The infinite loop in FastExtract. I fixed it by doing what Extract
does which uses a goto. I'm not a fan of goto's but the alternative of
setting "form_is_indirect = false" for everything other cases didn't seem
like a good solution either. Now Extract and FastExtract are almost
identical, so at some point we should probably consolidate the common code
into a shared function, or just make them one and deviate the differences
based on a parameter.

2) Fixed a crasher in SymbolFileDWARF. Where it was crashing would
probably never get executed with unix-style paths in the Dwarf, but does
with Windows-style paths. This is just a bandaid for now. The code
checks for absolute paths by checking if it starts with '/'. We'll need
to fix that up for Windows builds. I'd prefer not to have #ifdef's for
Windows in places like this so perhaps we can add a static function to
FileSpec or something that checks for absolute paths? I know there's been
some changes to FileSpec in the last couple of days but I haven't looked
closely at them yet. Another minor change is to fix the build when
ENABLE_DEBUG_PRINTF is defined.

3) Added some more Dwarf->Clang type mappings in ClangASTContext.

I've ran several of our sym files through the parser and it's working well
for compile units and function scopes. I'm still getting a lot of
crashers though when trying to parse types and variables. I'll send you a
couple in a separate email if you're interested in having a look.

Thanks,
Warren

patch.txt (7.95 KB)