Fix LLDB crash accessing DWZ (Fedora) debug info

[jankratochvil]

Summary

Currently:

(lldb) l main
=================================================================
==26703==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000ffc8 at pc 0x7f6aaaaffea0 bp 0x7ffda6302f30 sp 0x7ffda6302f20
READ of size 8 at 0x60200000ffc8 thread T0
    #0 in DWARFDebugInfoEntry::BuildAddressRangeTable(SymbolFileDWARF*, DWARFCompileUnit const*, DWARFDebugAranges*) const tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1336
    #1 in DWARFDebugInfoEntry::BuildAddressRangeTable(SymbolFileDWARF*, DWARFCompileUnit const*, DWARFDebugAranges*) const tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp:1350
    #2 in DWARFCompileUnit::BuildAddressRangeTable(SymbolFileDWARF*, DWARFDebugAranges*) tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:438

0x60200000ffc8 is located 8 bytes to the right of 16-byte region [0x60200000ffb0,0x60200000ffc0)
allocated by thread T2 here:
    #0 in operator new(unsigned long) (/lib64/libasan.so.3+0xc7ea0)
    #1 in __gnu_cxx::new_allocator<DWARFDebugInfoEntry>::allocate(unsigned long, void const*) /usr/include/c++/6.3.1/ext/new_allocator.h:104
    #2 in std::allocator_traits<std::allocator<DWARFDebugInfoEntry> >::allocate(std::allocator<DWARFDebugInfoEntry>&, unsigned long) /usr/include/c++/6.3.1/bits/alloc_traits.h:416
    #3 in std::__cxx1998::_Vector_base<DWARFDebugInfoEntry, std::allocator<DWARFDebugInfoEntry> >::_M_allocate(unsigned long) /usr/include/c++/6.3.1/bits/stl_vector.h:170
    #4 in void std::__cxx1998::vector<DWARFDebugInfoEntry, std::allocator<DWARFDebugInfoEntry> >::_M_emplace_back_aux<DWARFDebugInfoEntry const&>(DWARFDebugInfoEntry const&) /usr/include/c++/6.3.1/bits/vector.tcc:412
    #5 in std::__cxx1998::vector<DWARFDebugInfoEntry, std::allocator<DWARFDebugInfoEntry> >::push_back(DWARFDebugInfoEntry const&) /usr/include/c++/6.3.1/bits/stl_vector.h:924
    #6 in std::__debug::vector<DWARFDebugInfoEntry, std::allocator<DWARFDebugInfoEntry> >::push_back(DWARFDebugInfoEntry const&) /usr/include/c++/6.3.1/debug/vector:465
    #7 in DWARFCompileUnit::AddDIE(DWARFDebugInfoEntry&) (/quad/home/jkratoch/redhat/llvm-git-build/bin/../lib/liblldb.so.5+0x3ff19fc)
    #8 in DWARFCompileUnit::AddCompileUnitDIE(DWARFDebugInfoEntry&) tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:283
    #9 in DWARFCompileUnit::ExtractDIEsIfNeeded(bool) tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:186

Fixed:

(lldb) target create "true.all"
Current executable set to 'true.all' (x86_64).
(lldb) l main
warning: (x86_64) true.all 0x000004ff: compile unit 19 failed to create a valid lldb_private::CompileUnit class.
error: Could not find function named: "main".

This patch does not really implement DWZ support but in general LLDB should not crash on DWARF tags it does not know.

The warning there locks up, I do not know why yet, it is some unrelated bug. There was already an existing other ReportWarning() which would apparently also lock up now. I haven't try to test old versions and possibly bisect it yet.

Attached true.all is from Fedora 25 x86_64:

eu-unstrip -o true.all /usr/bin/true /usr/lib/debug/bin/true.debug

[jankratochvil]

true.all120 KBDownload

[labath]

I definitely agree we shouldn't crash if we see things we don't recognize, but I have no idea if it should be done this way. Greg should review this, as he's more familiar with the code.

If you can send me a minimal elf file reproducing this crash (e.g. containing just a single compile unit, with just one function, type or whatever is needed to blow up), i'll try to write a test for that.

(btw: the reason ReportWarning locks up now is that we are doing compile unit parsing on multiple threads. It looks like we don't handle this case properly, unfortunately)

[clayborg]

This will stop the crash, but I don't believe this is the right way to fix this. We should probably remove all DIEs from the CU and warn that there was a problem parsing debug info in the specific compile unit. We don't want LLDB to crash later one when it is trying to build stuff using the DWARF that is just abnormally truncated.

I am guessing you will be following up with a patch to fix the parsing for DWZ debug info?

[clayborg]

DWARF tags that we don't know should be just fine. If you can forward a DWARF file that exhibits this behavior I can take a look.

[labath]

In D35622#814717, @clayborg wrote:

If you can forward a DWARF file that exhibits this behavior I can take a look.

I think that's what the true.all file in the attachment is.

[clayborg]

Ah, that was hidden by "Older Comments"... Got the file. Will take a look

[clayborg]

"true.all" has custom DW_FORM_XXX values. There is no way to know what those are without adding parsing support to handle them. The right fix is to add support for this into LLDB. I will attach abbrev.txt which contains the .debub_abbrev dump.

[jankratochvil]

In D35622#814717, @clayborg wrote:

DWARF tags that we don't know should be just fine.

OK sorry - not tags but DWZ provides new forms - DW_FORM_GNU_ref_alt and DW_FORM_GNU_strp_alt.

In D35622#814714, @clayborg wrote:

I am guessing you will be following up with a patch to fix the parsing for DWZ debug info?

Yes, I have something in the works.

[jankratochvil]

In D35622#814746, @clayborg wrote:

"true.all" has custom DW_FORM_XXX values. There is no way to know what those are without adding parsing support to handle them. The right fix is to add support for this into LLDB. I will attach abbrev.txt which contains the .debub_abbrev dump.

Still LLDB should not crash on unknown DW_FORM_*. Therefore I expect I should rework this patch to drop whole CU in such case.

[clayborg]

Looks like we need to add support for:

// Alternate debug sections proposal (output of "dwz" tool).
#define DW_FORM_GNU_ref_alt 0x1f20
#define DW_FORM_GNU_strp_alt 0x1f21

I see this in:

[5]: DW_TAG_variable                       DW_CHILDREN_no
    DW_AT_name                             0x00001f21
    DW_AT_decl_file                        DW_FORM_data1
    DW_AT_decl_line                        DW_FORM_data2
    DW_AT_type                             0x00001f20

No debugger will be able to handle DW_FORMs unless specific code is added to parse these DW_FORM values in the DWARF parser. Custom DW_FORM values are tricky because nothing describes them in the DWARF to let a DWARF parser know how to parse them.

The right fix to stop LLDB from crashing is to have the DWARF parser parse all abbreviations and make sure it knows about all of the DW_FORM_XXX values in the abbreviations. If it doesn't it should claim it can't parse the DWARF at all. This would be done in SymbolFileDWARF::CalculateAbilities() right after the "section_list->FindSectionByType(eSectionTypeDWARFDebugAbbrev,...)" line. Something like:

DebugAbbrev *abbrev = DebugAbbrev();
if (!abbrev->SupportsAllForms())
  return 0;

DebugAbbrev::SupportsAllForms() would iterate through all DWARFAbbreviationDeclarationSet and check to ensure it knows about each DW_FORM_XXX:

bool DebugAbbrev::SupportsAllForms() {
 for (const auto &pair: m_abbrevCollMap)
   if (!pair.second.SupportsAllForms())
      return false;
  return true;
}

Then in DWARFAbbreviationDeclarationSet:

bool DWARFAbbreviationDeclarationSet::SupportsAllForms() {
 for (const auto &abbr_decl: m_decls) {
   const size_t num_attrs = abbr_decl.NumAttributes();
   for (size_t i=0; i<num_attrs; ++i) {
      if (!DWARFFormValue::FormIsSupported(abbr_decl.GetFormByIndex(i))
        return false;
   }
 }
 return true;
}

Then in DWARFFormValue.cpp:

bool DWARFFormValue::FormIsSupported(dw_form_t form) {
  switch (form) {
    case DW_FORM_addr:
    case DW_FORM_block2:
    case DW_FORM_block4:
    case DW_FORM_data2:
    case DW_FORM_data4:
    case DW_FORM_data8:
    case DW_FORM_string:
    case DW_FORM_block:
    case DW_FORM_block1:
    case DW_FORM_data1:
    case DW_FORM_flag:
    case DW_FORM_sdata:
    case DW_FORM_strp:
    case DW_FORM_udata:
    case DW_FORM_ref_addr:
    case DW_FORM_ref1:
    case DW_FORM_ref2:
    case DW_FORM_ref4:
    case DW_FORM_ref8:
    case DW_FORM_ref_udata:
    case DW_FORM_indirect:
    case DW_FORM_sec_offset:
    case DW_FORM_expr_loc:
    case DW_FORM_flag_present:
    case DW_FORM_ref_sig8:
    case DW_FORM_GNU_str_index:
    case DW_FORM_GNU_addr_index:
      return true;
    default:
      break;
  }
  return false;
}

You can add DW_FORM_GNU_ref_alt and DW_FORM_GNU_strp_alt to the DWARFFormValue::FormIsSupported() once you add support for them.

[clayborg]

We will need a warning to be emitted in SymbolFileDWARF::CalculateAbilities() stating we won't parse the DWARF due to "unsupported DW_FORM value of 0x1f20". Since this isn't done in a multi-threaded way, the warning should work without deadlocking.

[jankratochvil]

I would use something like (0==success)

dw_form_t CheckUnsupportedForm() const;

instead of

bool SupportsAllForms(dw_form_t &invalid_form) const;

But the patch tries to follow the advice.

[clayborg]

Good idea on passing back the unsupported form. Expand to all unsupported forms as suggested in inlined comments and this will be good to go.

source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp

103 ↗

(On Diff #107368)

void DWARFAbbreviationDeclarationSet::GetUnsupportedForms(std::set<dw_form_t> &invalid_forms);

109 ↗

(On Diff #107368)

invalid_forms.insert(form);

110 ↗

(On Diff #107368)

Remove return

114 ↗

(On Diff #107368)

remove

199 ↗

(On Diff #107368)

void DWARFDebugAbbrev::GetUnsupportedForms(std::set<dw_form_t> &invalid_forms);

source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h

44 ↗

(On Diff #107368)

It is a good idea to pass back the invalid forms. Seeing as there might be more than 1 unsupported DW_FORM, we pass in a "std::set<dw_form_t> &invalid_forms" to report all forms that were found and not supported and change the return value to void.

So this might be better as:

void GetUnsupportedForms(std::set<dw_form_t> &invalid_forms);

69 ↗

(On Diff #107368)

void GetUnsupportedForms(std::set<dw_form_t> &invalid_forms);

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

520–526 ↗

(On Diff #107368)

if (abbrev) {
  std::set<dw_form_t> invalid_forms;
  abbrev->GetUnsupportedForms(invalid_forms);
  if (!invalid_forms.empty()) {
    StreamString error;
    error.Printf("unsupported DW_FORM value%s:", invalid_forms.size() > 1 ? "s" : "");
    for (auto form: invalid_forms)
      error.Printf(" %#x", form);
    m_obj_file->GetModule()->ReportWarning("%s", error.GetString().c_str());
  }
}

[clayborg]

I forgot the "return 0;" in the "if (!invalid_forms.empty()) {"

[jankratochvil]

Thanks.

(lldb) l main
warning: (x86_64) /usr/bin/true unsupported DW_FORM values: 0x1f20 0x1f21
error: Could not find function named: "main".
(lldb) _

[clayborg]

Very nice.

[jankratochvil]

Unfortunately I do not have commit access, could you check it in? Thanks.

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h

public:
   void Clear();
   dw_offset_t GetOffset() const { return m_offset; }
   void Dump(lldb_private::Stream *s) const;
   bool Extract(const lldb_private::DWARFDataExtractor &data,
                lldb::offset_t *offset_ptr);
   // void Encode(BinaryStreamBuf& debug_abbrev_buf) const;
   dw_uleb128_t
   AppendAbbrevDeclSequential(const DWARFAbbreviationDeclaration &abbrevDecl);
+  void GetUnsupportedForms(std::set<dw_form_t> &invalid_forms) const;
 
   const DWARFAbbreviationDeclaration *
   GetAbbreviationDeclaration(dw_uleb128_t abbrCode) const;
 
 private:
   dw_offset_t m_offset;
   uint32_t m_idx_offset;
   std::vector<DWARFAbbreviationDeclaration> m_decls;
 };
 
 typedef std::map<dw_offset_t, DWARFAbbreviationDeclarationSet>
     DWARFAbbreviationDeclarationCollMap;
 typedef DWARFAbbreviationDeclarationCollMap::iterator
     DWARFAbbreviationDeclarationCollMapIter;
 typedef DWARFAbbreviationDeclarationCollMap::const_iterator
     DWARFAbbreviationDeclarationCollMapConstIter;
 
 class DWARFDebugAbbrev {
 public:
   DWARFDebugAbbrev();
   const DWARFAbbreviationDeclarationSet *
   GetAbbreviationDeclarationSet(dw_offset_t cu_abbr_offset) const;
   void Dump(lldb_private::Stream *s) const;
   void Parse(const lldb_private::DWARFDataExtractor &data);
+  void GetUnsupportedForms(std::set<dw_form_t> &invalid_forms) const;
 
 protected:
   DWARFAbbreviationDeclarationCollMap m_abbrevCollMap;
   mutable DWARFAbbreviationDeclarationCollMapConstIter m_prev_abbr_offset_pos;
 };
 
 #endif // SymbolFileDWARF_DWARFDebugAbbrev_h_

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp

dw_uleb128_t DWARFAbbreviationDeclarationSet::AppendAbbrevDeclSequential(
 
   // Update the code for this new declaration
   m_decls.back().SetCode(code);
 
   return code; // return the new abbreviation code!
 }
 
 //----------------------------------------------------------------------
+// DWARFAbbreviationDeclarationSet::GetUnsupportedForms()
+//----------------------------------------------------------------------
+void DWARFAbbreviationDeclarationSet::GetUnsupportedForms(
+    std::set<dw_form_t> &invalid_forms) const {
+  for (const auto &abbr_decl : m_decls) {
+    const size_t num_attrs = abbr_decl.NumAttributes();
+    for (size_t i=0; i<num_attrs; ++i) {
+      dw_form_t form = abbr_decl.GetFormByIndex(i);
+      if (!DWARFFormValue::FormIsSupported(form))
+        invalid_forms.insert(form);
+    }
+  }
+}
+
+//----------------------------------------------------------------------
 // Encode
 //
 // Encode the abbreviation table onto the end of the buffer provided
 // into a byte representation as would be found in a ".debug_abbrev"
 // debug information section.
 //----------------------------------------------------------------------
 // void
 // DWARFAbbreviationDeclarationSet::Encode(BinaryStreamBuf& debug_abbrev_buf)
else {
     pos = m_abbrevCollMap.find(cu_abbr_offset);
     m_prev_abbr_offset_pos = pos;
   }
 
   if (pos != m_abbrevCollMap.end())
     return &(pos->second);
   return NULL;
 }
+
+//----------------------------------------------------------------------
+// DWARFDebugAbbrev::GetUnsupportedForms()
+//----------------------------------------------------------------------
+void DWARFDebugAbbrev::GetUnsupportedForms(
+    std::set<dw_form_t> &invalid_forms) const {
+  for (const auto &pair : m_abbrevCollMap)
+    pair.second.GetUnsupportedForms(invalid_forms);
+}

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.h

static bool SkipValue(const dw_form_t form,
                         const lldb_private::DWARFDataExtractor &debug_info_data,
                         lldb::offset_t *offset_ptr, const DWARFCompileUnit *cu);
   static bool IsBlockForm(const dw_form_t form);
   static bool IsDataForm(const dw_form_t form);
   static FixedFormSizes GetFixedFormSizesForAddressSize(uint8_t addr_size,
                                                         bool is_dwarf64);
   static int Compare(const DWARFFormValue &a, const DWARFFormValue &b);
   void Clear();
+  static bool FormIsSupported(dw_form_t form);
 
 protected:
   const DWARFCompileUnit *m_cu; // Compile unit for this form
   dw_form_t m_form;             // Form for this value
   ValueType m_value;            // Contains all data for the form
 };
 
 #endif // SymbolFileDWARF_DWARFFormValue_h_

lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp

case DW_FORM_indirect:
     llvm_unreachable(
         "This shouldn't happen after the form has been extracted...");
 
   default:
     llvm_unreachable("Unhandled DW_FORM");
   }
   return -1;
 }
+
+bool DWARFFormValue::FormIsSupported(dw_form_t form) {
+  switch (form) {
+    case DW_FORM_addr:
+    case DW_FORM_block2:
+    case DW_FORM_block4:
+    case DW_FORM_data2:
+    case DW_FORM_data4:
+    case DW_FORM_data8:
+    case DW_FORM_string:
+    case DW_FORM_block:
+    case DW_FORM_block1:
+    case DW_FORM_data1:
+    case DW_FORM_flag:
+    case DW_FORM_sdata:
+    case DW_FORM_strp:
+    case DW_FORM_udata:
+    case DW_FORM_ref_addr:
+    case DW_FORM_ref1:
+    case DW_FORM_ref2:
+    case DW_FORM_ref4:
+    case DW_FORM_ref8:
+    case DW_FORM_ref_udata:
+    case DW_FORM_indirect:
+    case DW_FORM_sec_offset:
+    case DW_FORM_exprloc:
+    case DW_FORM_flag_present:
+    case DW_FORM_ref_sig8:
+    case DW_FORM_GNU_str_index:
+    case DW_FORM_GNU_addr_index:
+      return true;
+    default:
+      break;
+  }
+  return false;
+}

lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (section != NULL) {
       debug_info_file_size = section->GetFileSize();
 
       section =
           section_list->FindSectionByType(eSectionTypeDWARFDebugAbbrev, true)
               .get();
       if (section)
         debug_abbrev_file_size = section->GetFileSize();
 
+      DWARFDebugAbbrev *abbrev = DebugAbbrev();
+      if (abbrev) {
+        std::set<dw_form_t> invalid_forms;
+        abbrev->GetUnsupportedForms(invalid_forms);
+        if (!invalid_forms.empty()) {
+          StreamString error;
+          error.Printf("unsupported DW_FORM value%s:", invalid_forms.size() > 1 ? "s" : "");
+          for (auto form : invalid_forms)
+            error.Printf(" %#x", form);
+          m_obj_file->GetModule()->ReportWarning("%s", error.GetString().str().c_str());
+          return 0;
+        }
+      }
+
       section =
           section_list->FindSectionByType(eSectionTypeDWARFDebugLine, true)
               .get();
       if (section)
         debug_line_file_size = section->GetFileSize();
     } else {
       const char *symfile_dir_cstr =
           m_obj_file->GetFileSpec().GetDirectory().GetCString();