From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22837 invoked by alias); 10 Feb 2014 14:29:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 22828 invoked by uid 89); 10 Feb 2014 14:29:12 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.7 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN autolearn=no version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 10 Feb 2014 14:29:10 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CF0A6116203 for ; Mon, 10 Feb 2014 09:29:08 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Oud029o6zG34 for ; Mon, 10 Feb 2014 09:29:08 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 217451161AF for ; Mon, 10 Feb 2014 09:29:08 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 65243E075B; Mon, 10 Feb 2014 18:29:08 +0400 (RET) Date: Mon, 10 Feb 2014 14:29:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation. Message-ID: <20140210142908.GZ5485@adacore.com> References: <1390796357-3739-1-git-send-email-brobecker@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1390796357-3739-1-git-send-email-brobecker@adacore.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-02/txt/msg00304.txt.bz2 PING? Thank you! On Mon, Jan 27, 2014 at 08:19:17AM +0400, Joel Brobecker wrote: > Hello, > > Consider the following Ada code: > > -- An array whose index is an enumeration type with 128 enumerators. > type Enum_T is (Enum_000, Enum_001, [...], Enum_128); > type Table is array (Enum_T) of Boolean; > > When the compiler is configured to generate pure DWARF debugging info, > trying to print type Table's description yields: > > ptype pck.table > type = array (enum_000 .. -128) of boolean > > The expected output was: > > ptype pck.table > type = array (enum_000 .. enum_128) of boolean > > The DWARF debugging info for our array looks like this: > > <1><44>: Abbrev Number: 5 (DW_TAG_array_type) > <45> DW_AT_name : pck__table > <50> DW_AT_type : <0x28> > <2><54>: Abbrev Number: 6 (DW_TAG_subrange_type) > <55> DW_AT_type : <0x5c> > <59> DW_AT_lower_bound : 0 > <5a> DW_AT_upper_bound : 128 > > The array index type is, by construction with the DWARF standard, > a subrange of our enumeration type, defined as follow: > > <2><5b>: Abbrev Number: 0 > <1><5c>: Abbrev Number: 7 (DW_TAG_enumeration_type) > <5d> DW_AT_name : pck__enum_t > <69> DW_AT_byte_size : 1 > <2><6b>: Abbrev Number: 8 (DW_TAG_enumerator) > <6c> DW_AT_name : pck__enum_000 > <7a> DW_AT_const_value : 0 > [etc] > > Therefore, while processing these DIEs, the array index type ends > up being a TYPE_CODE_RANGE whose target type is our enumeration type. > But the problem is that we read the upper bound as a negative value > (-128), which is then used as is by the type printer to print the > array upper bound. This negative value explains the "-128" in the > output. > > To understand why the range type's upper bound is read as a negative > value, one needs to look at how it is determined, in read_subrange_type: > > orig_base_type = die_type (die, cu); > base_type = check_typedef (orig_base_type); > [... high is first correctly read as 128, but then ...] > if (!TYPE_UNSIGNED (base_type) && (high & negative_mask)) > high |= negative_mask; > > The negative_mask is applied, here, because BASE_TYPE->FLAG_UNSIGNED > is not set. And the reason for that is because the base_type was only > partially constructed during the call to die_type. While the enum > is constructed on the fly by read_enumeration_type, its flag_unsigned > flag is only set later on, while creating the symbols corresponding to > the enum type's enumerators (see process_enumeration_scope), after > we've already finished creating our range type - and therefore too > late. > > My first naive attempt at fixing this problem consisted in extracting > the part in process_enumeration_scope which processes all enumerators, > to generate the associated symbols, but more importantly set the type's > various flags when necessary. However, this does not always work well, > because we're still in the subrange_type's scope, and it might be > different from the scope where the enumeration type is defined. > > So, instead, what this patch does to fix the issue is to extract > from process_enumeration_scope the part that determines whether > the enumeration type should have the flag_unsigned and/or the > flag_flag_enum flags set. It turns out that, aside from the code > implementing the loop, this part is fairly independent of the symbol > creation. With that part extracted, we can then use it at the end > of our enumeration type creation, to produce a type which should now > no longer need any adjustment. > > Once the enumeration type produced is correctly marked as unsigned, > the subrange type's upper bound is then correctly read as an unsigned > value, therefore giving us an upper bound of 128 instead of -128. > > gdb/ChangeLog: > > * dwarf2read.c (update_enumeration_type_from_children): New > function, mostly extracted from process_structure_scope. > (read_enumeration_type): Call update_enumeration_type_from_children. > (process_enumeration_scope): Do not set THIS_TYPE's flag_unsigned > and flag_flag_enum fields. > > gdb/testsuite/ChangeLog: > > * gdb.dwarf2/arr-subrange.c, gdb.dwarf2/arr-subrange.exp: New files. > > Tested on x86_64-linux. Ok to commit? > > Thanks, > -- > Joel > > --- > gdb/dwarf2read.c | 82 ++++++++++++++++++++----- > gdb/testsuite/gdb.dwarf2/arr-subrange.c | 20 +++++++ > gdb/testsuite/gdb.dwarf2/arr-subrange.exp | 99 +++++++++++++++++++++++++++++++ > 3 files changed, 185 insertions(+), 16 deletions(-) > create mode 100644 gdb/testsuite/gdb.dwarf2/arr-subrange.c > create mode 100644 gdb/testsuite/gdb.dwarf2/arr-subrange.exp > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 54c538a..ba7623f 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -13080,6 +13080,69 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu) > new_symbol (die, type, cu); > } > > +/* Assuming DIE is an enumeration type, and TYPE is its associated type, > + update TYPE using some information only available in DIE's children. */ > + > +static void > +update_enumeration_type_from_children (struct die_info *die, > + struct type *type, > + struct dwarf2_cu *cu) > +{ > + struct obstack obstack; > + struct die_info *child_die = die->child; > + int unsigned_enum = 1; > + int flag_enum = 1; > + ULONGEST mask = 0; > + struct cleanup *old_chain; > + > + obstack_init (&obstack); > + old_chain = make_cleanup_obstack_free (&obstack); > + > + while (child_die != NULL && child_die->tag) > + { > + struct attribute *attr; > + LONGEST value; > + const gdb_byte *bytes; > + struct dwarf2_locexpr_baton *baton; > + const char *name; > + if (child_die->tag != DW_TAG_enumerator) > + continue; > + > + attr = dwarf2_attr (child_die, DW_AT_const_value, cu); > + if (attr == NULL) > + continue; > + > + name = dwarf2_name (child_die, cu); > + if (name == NULL) > + name = ""; > + > + dwarf2_const_value_attr (attr, type, name, &obstack, cu, > + &value, &bytes, &baton); > + if (value < 0) > + { > + unsigned_enum = 0; > + flag_enum = 0; > + } > + else if ((mask & value) != 0) > + flag_enum = 0; > + else > + mask |= value; > + > + /* If we already know that the enum type is neither unsigned, nor > + a flag type, no need to look at the rest of the enumerates. */ > + if (!unsigned_enum && !flag_enum) > + break; > + child_die = sibling_die (child_die); > + } > + > + if (unsigned_enum) > + TYPE_UNSIGNED (type) = 1; > + if (flag_enum) > + TYPE_FLAG_ENUM (type) = 1; > + > + do_cleanups (old_chain); > +} > + > /* Given a DW_AT_enumeration_type die, set its type. We do not > complete the type's fields yet, or create any symbols. */ > > @@ -13129,6 +13192,9 @@ read_enumeration_type (struct die_info *die, struct dwarf2_cu *cu) > if (die_is_declaration (die, cu)) > TYPE_STUB (type) = 1; > > + /* Finish the creation of this type by using the enum's children. */ > + update_enumeration_type_from_children (die, type, cu); > + > return set_die_type (die, type, cu); > } > > @@ -13153,10 +13219,7 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu) > struct symbol *sym; > struct field *fields = NULL; > int num_fields = 0; > - int unsigned_enum = 1; > const char *name; > - int flag_enum = 1; > - ULONGEST mask = 0; > > child_die = die->child; > while (child_die && child_die->tag) > @@ -13171,15 +13234,6 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu) > if (name) > { > sym = new_symbol (child_die, this_type, cu); > - if (SYMBOL_VALUE (sym) < 0) > - { > - unsigned_enum = 0; > - flag_enum = 0; > - } > - else if ((mask & SYMBOL_VALUE (sym)) != 0) > - flag_enum = 0; > - else > - mask |= SYMBOL_VALUE (sym); > > if ((num_fields % DW_FIELD_ALLOC_CHUNK) == 0) > { > @@ -13210,10 +13264,6 @@ process_enumeration_scope (struct die_info *die, struct dwarf2_cu *cu) > sizeof (struct field) * num_fields); > xfree (fields); > } > - if (unsigned_enum) > - TYPE_UNSIGNED (this_type) = 1; > - if (flag_enum) > - TYPE_FLAG_ENUM (this_type) = 1; > } > > /* If we are reading an enum from a .debug_types unit, and the enum > diff --git a/gdb/testsuite/gdb.dwarf2/arr-subrange.c b/gdb/testsuite/gdb.dwarf2/arr-subrange.c > new file mode 100644 > index 0000000..978ef8d > --- /dev/null > +++ b/gdb/testsuite/gdb.dwarf2/arr-subrange.c > @@ -0,0 +1,20 @@ > +/* Copyright 2014 Free Software Foundation, Inc. > + > + This program 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; either version 3 of the License, or > + (at your option) any later version. > + > + This program 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 this program. If not, see . */ > + > +int > +main (void) > +{ > + return 0; > +} > diff --git a/gdb/testsuite/gdb.dwarf2/arr-subrange.exp b/gdb/testsuite/gdb.dwarf2/arr-subrange.exp > new file mode 100644 > index 0000000..9847c13 > --- /dev/null > +++ b/gdb/testsuite/gdb.dwarf2/arr-subrange.exp > @@ -0,0 +1,99 @@ > +# Copyright 2014 Free Software Foundation, Inc. > + > +# This program 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; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program 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 this program. If not, see . > +load_lib dwarf.exp > + > +# This test can only be run on targets which support DWARF-2 and use gas. > +if {![dwarf2_support]} { > + return 0 > +} > + > +standard_testfile arr-subrange.c arr-subrange-dw.S > + > +# Make some DWARF for the test. > +set asm_file [standard_output_file $srcfile2] > +Dwarf::assemble $asm_file { > + cu {} { > + DW_TAG_compile_unit { > + {DW_AT_language @DW_LANG_Ada95} > + {DW_AT_name foo.adb} > + {DW_AT_comp_dir /tmp} > + {DW_AT_low_pc 0x1000} > + {DW_AT_high_pc 0x2000} > + } { > + declare_labels boolean_label typedef_label array_label enum_label > + > + boolean_label: DW_TAG_base_type { > + {DW_AT_byte_size 1 DW_FORM_sdata} > + {DW_AT_encoding @DW_ATE_boolean} > + {DW_AT_name boolean} > + } > + > + typedef_label: DW_TAG_typedef { > + {DW_AT_name pck__table} > + {DW_AT_type :$array_label} > + } > + > + array_label: DW_TAG_array_type { > + {DW_AT_name pck__table} > + {DW_AT_type :$boolean_label} > + } { > + DW_TAG_subrange_type { > + {DW_AT_type :$enum_label} > + {DW_AT_lower_bound 0 DW_FORM_data1} > + {DW_AT_upper_bound 128 DW_FORM_data1} > + } > + } > + > + enum_label: DW_TAG_enumeration_type { > + {DW_AT_name pck__enum_t} > + {DW_AT_byte_size 1 DW_FORM_sdata} > + } { > + DW_TAG_enumerator { > + {DW_AT_name pck__enum_000} > + {DW_AT_const_value 0 DW_FORM_sdata} > + } > + DW_TAG_enumerator { > + {DW_AT_name pck__enum_001} > + {DW_AT_const_value 1 DW_FORM_sdata} > + } > + DW_TAG_enumerator { > + {DW_AT_name pck__enum_128} > + {DW_AT_const_value 128 DW_FORM_sdata} > + } > + } > + } > + } > +} > + > +if {[gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \ > + object {nodebug}] != ""} { > + return -1 > +} > + > +if {[gdb_compile $asm_file ${binfile}2.o object {nodebug}] != ""} { > + return -1 > +} > + > +if {[gdb_compile [list ${binfile}1.o ${binfile}2.o] \ > + "${binfile}" executable {}] != ""} { > + return -1 > +} > + > +clean_restart ${testfile} > + > +gdb_test_no_output "set language ada" > + > +gdb_test "ptype pck.table" \ > + "type = array \\(enum_000 \\.\\. enum_128\\) of boolean" > -- > 1.8.3.2 -- Joel