Re: [dwarflint] asserts with check_debug_info

[Mark Wielaard <mjw at redhat dot com>]

On Mon, 2011-04-11 at 13:02 +0200, Petr Machata wrote:
> The original theory must have been that form_class won't be called
> unless you know the form/attr combo makes sense.  But it will be easier
> your way, max_dw_class is checked for anyway.  So go ahead and merge
> this.
> 
> If you have an example binary, please add it to a test suite.

I added an example to the testsuite. It asserts/crashes without the
patch, and succeeds silently with it.

Cheers,

Mark

commit b054bb07802775f0b2810fb9a05c72ca289cb169
Author: Mark Wielaard <mjw@redhat.com>
Date:   Mon Apr 11 13:55:58 2011 +0200

    dwarflint: Don't assert in check_debug_info can trigger an assert.
    
    Instead of asserting a form must exist, let form_class () return
    max_dw_class, like ambiguous_class () does. max_dw_class is already
    used as sentinel in read_die_chain () in case the attribute is unknown.
    This lets things work out for the low level check_debug_info check and
    then makes the higher-level checks complain about the unexpected form
    instead. Added an example binary and test dwarflint/tests/run-upper.sh.

diff --git a/dwarflint/Makefile.am b/dwarflint/Makefile.am
index 56e2611..f8d03c3 100644
--- a/dwarflint/Makefile.am
+++ b/dwarflint/Makefile.am
@@ -117,7 +117,8 @@ EXTRA_TESTS = tests/run-debug_abbrev-duplicate-attribute.sh \
 	tests/run-check_self_referential_die.sh \
 	tests/run-DW_AT_high_pc-relative.sh \
 	tests/run-DW_AT_high_pc-below.sh \
-	tests/run-DW_AT-later-version.sh
+	tests/run-DW_AT-later-version.sh \
+	tests/run-upper.sh
 
 TESTS = $(EXTRA_TESTS) \
 	tests/test-coverage \
@@ -150,7 +151,8 @@ EXTRA_DIST = $(EXTRA_TESTS) \
 	tests/check_self_referential_die.bz2 \
 	tests/DW_AT_high_pc-relative.bz2 \
 	tests/DW_AT_high_pc-below.bz2 \
-	tests/DW_AT-later-version.bz2
+	tests/DW_AT-later-version.bz2 \
+	tests/upper.bz2
 
 installed_TESTS_ENVIRONMENT = libdir=$(DESTDIR)$(libdir) \
 			      bindir=$(DESTDIR)$(bindir) \
diff --git a/dwarflint/check_debug_info.cc b/dwarflint/check_debug_info.cc
index 46d4ec5..c880fd9 100644
--- a/dwarflint/check_debug_info.cc
+++ b/dwarflint/check_debug_info.cc
@@ -796,8 +796,11 @@ namespace
 		valuep = &high_pc;
 		if (cls == cl_constant)
 		  high_pc_relative = true;
-		else
-		  assert (cls == cl_address);
+		else if (cls != cl_address)
+		  {
+		    wr_error (&where, ": DW_AT_high_pc in unknown form.\n");
+		    retval = -2;
+		  }
 		break;
 
 	      case DW_AT_decl_file:
diff --git a/dwarflint/dwarf_version.cc b/dwarflint/dwarf_version.cc
index 8a34ef0..28404a2 100644
--- a/dwarflint/dwarf_version.cc
+++ b/dwarflint/dwarf_version.cc
@@ -87,7 +87,6 @@ dwarf_version::form_class (form const *form, attribute const *attribute) const
   assert (attribute != NULL);
   dw_class_set result = form->classes ();
   result &= attribute->classes ();
-  assert (result.any ());
   if (result.count () > 1)
     {
       dw_class ret = this->ambiguous_class (form, attribute, result);
@@ -95,8 +94,10 @@ dwarf_version::form_class (form const *form, attribute const *attribute) const
       assert (result[ret]);
       return ret;
     }
-  else
+  else if (result.count () == 1)
     return static_cast<dw_class> (ffsl (result.to_ulong ()) - 1);
+  else
+    return max_dw_class;
 }
 
 form_width_t
diff --git a/dwarflint/tests/run-upper.sh b/dwarflint/tests/run-upper.sh
new file mode 100755
index 0000000..31f8829
--- /dev/null
+++ b/dwarflint/tests/run-upper.sh
@@ -0,0 +1,56 @@
+#! /bin/sh
+# Copyright (C) 2011 Red Hat, Inc.
+# This file is part of Red Hat elfutils.
+#
+# Red Hat elfutils is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by the
+# Free Software Foundation; version 2 of the License.
+#
+# Red Hat elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with Red Hat elfutils; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301 USA.
+#
+# Red Hat elfutils is an included package of the Open Invention Network.
+# An included package of the Open Invention Network is a package for which
+# Open Invention Network licensees cross-license their patents.  No patent
+# license is granted, either expressly or impliedly, by designation as an
+# included package.  Should you wish to participate in the Open Invention
+# Network licensing program, please visit www.openinventionnetwork.com
+# <http://www.openinventionnetwork.com>.
+
+. $srcdir/../tests/test-subr.sh
+
+srcdir=$srcdir/tests
+
+# Following program compiled with "default" gcc settings,
+# which is dwarf-2 + gnu extensions. Which will result in:
+#
+#  [    a2]      subrange_type
+#                type                 (ref4) [    ac]
+#                upper_bound          (block1)
+#                [   0] fbreg -24
+#                [   2] deref
+#
+# According to dwarf-2 DW_AT_upperbound cannot be encoded with block form.
+# It can however with later versions of dwarf, which gcc will output as
+# gnu extension (unless -gstrict-dwarf is given).
+#
+# int max_range = 42;
+#
+# int main (int argc, char **argv)
+# {
+#   char chars[max_range];
+#   chars[max_range -1] = 7;
+#   return 0;
+# }
+#
+# This would crash the low-level check_debug_info in the past.
+testfiles upper
+
+testrun_compare ./dwarflint --quiet --check=@low upper <<EOF
+EOF
diff --git a/dwarflint/tests/upper.bz2 b/dwarflint/tests/upper.bz2
new file mode 100755
index 0000000..8844886
Binary files /dev/null and b/dwarflint/tests/upper.bz2 differ