dwarflint: Fix attribute and form validation in .debug_abbrev and .debug_info

- and a test case that used to SIGSEGV

diff --git a/dwarflint/check_debug_abbrev.cc b/dwarflint/check_debug_abbrev.cc
index 0d1786d7e637b1b085ccb0e935113c4edf059687..39b5f316de25fd4e9a90b2e0a3e62053ec2aadc8 100644 (file)
--- a/dwarflint/check_debug_abbrev.cc
+++ b/dwarflint/check_debug_abbrev.cc
@@ -27,6 +27,13 @@
 # include <config.h>
 #endif
 
+#include <dwarf.h>
+#include <sstream>
+#include <cassert>
+#include <algorithm>
+
+#include "../libdw/c++/dwarf"
+
 #include "check_debug_info.hh"
 #include "check_debug_abbrev.hh"
 #include "pri.hh"
@@ -36,11 +43,6 @@
 #include "messages.hh"
 #include "misc.hh"
 
-#include <dwarf.h>
-#include <sstream>
-#include <cassert>
-#include <algorithm>
-
 checkdescriptor const *
 check_debug_abbrev::descriptor ()
 {
@@ -112,13 +114,14 @@ namespace
 
   void
   complain (where const *where,
-           attribute const *attribute, form const *form,
+           int attrib_name, int form_name,
            bool indirect, char const *qualifier)
   {
     wr_error (*where)
-      << "attribute " << *attribute << " with " << qualifier
-      << (indirect ? " indirect" : "") << " form "
-      << *form << '.' << std::endl;
+      << "attribute " << elfutils::dwarf::attributes::name (attrib_name)
+      << " with " << qualifier << (indirect ? " indirect" : "")
+      << " form " << elfutils::dwarf::forms::name (form_name)
+      << '.' << std::endl;
   }
 
   bool
@@ -369,7 +372,6 @@ namespace
 
            /* Now if both are zero, this was the last attribute.  */
            null_attrib = attrib_name == 0 && attrib_form == 0;
-           attribute const *attribute = NULL;
 
            REALLOC (cur, attribs);
 
@@ -392,17 +394,25 @@ namespace
                continue;
              }
 
-           attribute = ver->get_attribute (attrib_name);
+           attribute const *attribute = ver->get_attribute (attrib_name);
            if (attribute == NULL)
              {
                wr_error (where)
                  << "invalid or unknown name " << pri::hex (attrib_name)
                  << '.' << std::endl;
                // libdw should handle unknown attribute, as long as
-               // the form is kosher.
-               continue;
+               // the form is kosher, so don't fail the check.
              }
 
+           form const *form = check_debug_abbrev::check_form
+             (ver, attribute, attrib_form, &where, false);
+           if (form == NULL)
+             // Error message has been emitted in check_form.
+             failed = true;
+
+           if (form == NULL || attribute == NULL)
+             continue;
+
            std::pair<std::map<unsigned, uint64_t>::iterator, bool> inserted
              = seen.insert (std::make_pair (attrib_name, attr_off));
            if (!inserted.second)
@@ -418,15 +428,6 @@ namespace
                  failed = true;
              }
 
-           form const *form = check_debug_abbrev::check_form
-             (ver, attrib_form, attribute, &where, false);
-           if (form == NULL)
-             {
-               // Error message is emitted in check_form.
-               failed = true;
-               continue;
-             }
-
            if (attrib_name == DW_AT_sibling)
              {
                if (!cur->has_children)
@@ -483,9 +484,10 @@ check_debug_abbrev::check_debug_abbrev (checkstack &stack, dwarflint &lint)
 }
 
 form const *
-check_debug_abbrev::check_form (dwarf_version const *ver, int form_name,
-                               attribute const *attribute, where const *where,
-                               bool indirect)
+check_debug_abbrev::check_form (dwarf_version const *ver,
+                               attribute const *attribute,
+                               int form_name,
+                               where const *where, bool indirect)
 {
   form const *form = ver->get_form (form_name);
   if (form == NULL)
@@ -496,14 +498,19 @@ check_debug_abbrev::check_form (dwarf_version const *ver, int form_name,
       return NULL;
     }
 
-  if (!ver->form_allowed (attribute->name (), form_name))
+  if (attribute != NULL)
     {
-      complain (where, attribute, form, indirect, "invalid");
-      return NULL;
+
+      int attrib_name = attribute->name ();
+      if (!ver->form_allowed (attribute, form))
+       {
+         complain (where, attrib_name, form_name, indirect, "invalid");
+         return NULL;
+       }
+      else if (attrib_name == DW_AT_sibling
+              && sibling_form_suitable (ver, form) == sfs_long)
+       complain (where, attrib_name, form_name, indirect, "unsuitable");
     }
-  else if (attribute->name () == DW_AT_sibling
-          && sibling_form_suitable (ver, form_name) == sfs_long)
-    complain (where, attribute, form, indirect, "unsuitable");
 
   return form;
 }

diff --git a/dwarflint/check_debug_abbrev.hh b/dwarflint/check_debug_abbrev.hh
index bdc7f8c6ae1091ce2a81099d93ca85079fd42439..28231647bc4e484b2373711cc6a30039f7662491 100644 (file)
--- a/dwarflint/check_debug_abbrev.hh
+++ b/dwarflint/check_debug_abbrev.hh
@@ -85,8 +85,8 @@ public:
 
   check_debug_abbrev (checkstack &stack, dwarflint &lint);
   static form const *check_form (dwarf_version const *ver,
+                                attribute const *attr,
                                 int form_name,
-                                attribute const *attribute,
                                 where const *where,
                                 bool indirect);
 

diff --git a/dwarflint/check_debug_info.cc b/dwarflint/check_debug_info.cc
index 45af96b85ba9ee3c234fcff2b0060de544b207fe..b712daab2de2c4767a9a2d2011b9e06daaea2c07 100644 (file)
--- a/dwarflint/check_debug_info.cc
+++ b/dwarflint/check_debug_info.cc
@@ -652,9 +652,13 @@ namespace
             it->name != 0 || it->form != 0; ++it)
          {
            where.ref = &it->where;
+           int form_name = it->form;
 
+           // In following, attribute may be NULL, but form never
+           // should.  We always need to know the form to be able to
+           // read .debug_info, so we fail in check_debug_abbrev if
+           // it's invalid or unknown.
            attribute const *attribute = ver->get_attribute (it->name);
-           int form_name = it->form;
            form const *form = ver->get_form (form_name);
            if (attribute != NULL
                && ver->form_class (form, attribute) == cl_indirect)
@@ -665,10 +669,12 @@ namespace
                  return -1;
                form_name = value;
                form = check_debug_abbrev::check_form
-                 (ver, form_name, attribute, &where, true);
+                 (ver, attribute, form_name, &where, true);
+               // N.B. check_form emits appropriate error messages.
                if (form == NULL)
                  return -1;
              }
+           assert (form != NULL);
 
            dw_class cls = attribute != NULL
              ? ver->form_class (form, attribute)
@@ -715,7 +721,7 @@ namespace
              (cl_reference, cl_loclistptr, cl_lineptr, cl_macptr,
               cl_rangelistptr);
 
-           if (cls != max_dw_class && ref_classes.test (cls))
+           if (form != NULL && cls != max_dw_class && ref_classes.test (cls))
              {
                form_bitness_t bitness = form->bitness ();
                if ((bitness == fb_32 && cu->head->offset_size == 8)

diff --git a/dwarflint/dwarf_gnu.cc b/dwarflint/dwarf_gnu.cc
index df3e85b1cdc0ead1bfd995d441f1168c6ed1949e..6335abb46396202921c00028b0b5719945da83e9 100644 (file)
--- a/dwarflint/dwarf_gnu.cc
+++ b/dwarflint/dwarf_gnu.cc
@@ -80,15 +80,12 @@ namespace
     {}
 
     virtual bool
-    form_allowed (int attribute_name, int form_name) const
+    form_allowed (attribute const *attr, form const *form) const
     {
-      if (attribute_name == DW_AT_GNU_odr_signature)
-       {
-         form const *f = get_form (form_name);
-         return f->classes ()[cl_constant] && f->width (NULL) == fw_8;
-       }
+      if (attr->name () == DW_AT_GNU_odr_signature)
+       return form->classes ()[cl_constant] && form->width (NULL) == fw_8;
       else
-       return std_dwarf::form_allowed (attribute_name, form_name);
+       return std_dwarf::form_allowed (attr, form);
     }
   };
 }

diff --git a/dwarflint/dwarf_version.cc b/dwarflint/dwarf_version.cc
index 14e7961a8d39281a80e6cd5f7017639b49de8332..fc4595a65ee46f2e7b7cf9113f680d66cfe7c7b9 100644 (file)
--- a/dwarflint/dwarf_version.cc
+++ b/dwarflint/dwarf_version.cc
@@ -150,25 +150,19 @@ dwarf_version::form_allowed (int form) const
 }
 
 bool
-dwarf_version::form_allowed (int attribute_name, int form_name) const
+dwarf_version::form_allowed (attribute const *attr, form const *form) const
 {
-  attribute const *attribute = this->get_attribute (attribute_name);
-  assert (attribute != NULL);
-  dw_class_set const &attr_classes = attribute->classes ();
-
-  form const *form = this->get_form (form_name);
-  assert (form != NULL);
+  dw_class_set const &attr_classes = attr->classes ();
   dw_class_set const &form_classes = form->classes ();
-
   return (attr_classes & form_classes).any ();
 }
 
 sibling_form_suitable_t
-sibling_form_suitable (dwarf_version const *ver, int form)
+sibling_form_suitable (dwarf_version const *ver, form const *form)
 {
-  if (!ver->form_allowed (DW_AT_sibling, form))
+  if (!ver->form_allowed (ver->get_attribute (DW_AT_sibling), form))
     return sfs_invalid;
-  else if (form == DW_FORM_ref_addr)
+  else if (form->name () == DW_FORM_ref_addr)
     return sfs_long;
   else
     return sfs_ok;

diff --git a/dwarflint/dwarf_version.hh b/dwarflint/dwarf_version.hh
index f7a1df5d7532207e477dcb00685cf70ddd59ebe6..43df29e964691eeca09ba0d5426b0469394cfb9c 100644 (file)
--- a/dwarflint/dwarf_version.hh
+++ b/dwarflint/dwarf_version.hh
@@ -202,7 +202,8 @@ public:
 
   /// Figure out whether, in given DWARF version, given attribute is
   /// allowed to have given form.
-  virtual bool form_allowed (int attribute_name, int form_name) const;
+  virtual bool form_allowed (attribute const *attr, form const *form) const
+    __attribute__ ((nonnull (1, 2)));
 
   /// Answer a class of FORM given ATTRIBUTE as a context.  If there's
   /// exactly one candidate class, that's the one answered.  If
@@ -238,6 +239,7 @@ enum sibling_form_suitable_t
     sfs_invalid, ///< This form isn't allowed at DW_AT_sibling
   };
 sibling_form_suitable_t sibling_form_suitable (dwarf_version const *ver,
-                                              int form);
+                                              form const *form)
+  __attribute__ ((nonnull (1, 2)));
 
 #endif//DWARFLINT_DWARF_VERSION_HH

diff --git a/dwarflint/tests/garbage-7.bz2 b/dwarflint/tests/garbage-7.bz2
new file mode 100644 (file)
index 0000000..41963b6
Binary files /dev/null and b/dwarflint/tests/garbage-7.bz2 differ

diff --git a/dwarflint/tests/run-bad.sh b/dwarflint/tests/run-bad.sh
index 108f527bbc0a2698d63d7018560df643e45c251b..423e1f4183bd56952c54a0948eaf1a417fc6f1d7 100755 (executable)
--- a/dwarflint/tests/run-bad.sh
+++ b/dwarflint/tests/run-bad.sh
@@ -28,7 +28,7 @@
 srcdir=$srcdir/tests
 
 testfiles hello.bad-1 hello.bad-3 garbage-1 garbage-2 garbage-3 garbage-4 \
-    garbage-5 garbage-6
+    garbage-5 garbage-6 garbage-7
 
 testrun_compare ./dwarflint hello.bad-1 <<EOF
 error: .debug_info: DIE 0x83: abbrev section at 0x0 doesn't contain code 83.
@@ -72,3 +72,9 @@ error: .debug_abbrev: abbr. attribute 0xc: attribute stmt_list with invalid form
 error: .debug_abbrev: abbr. attribute 0x23: attribute frame_base with invalid form block1.
 error: .debug_abbrev: abbr. attribute 0x34: attribute location with invalid form block1.
 EOF
+
+testrun_compare ./dwarflint garbage-7 <<EOF
+error: .debug_abbrev: abbr. attribute 0x7e: invalid or unknown name 0x703.
+error: .debug_abbrev: abbr. attribute 0x7e: invalid form 0x0.
+error: .debug_abbrev: abbreviation 122: missing zero to mark end-of-table.
+EOF