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