pushed: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.

[Joel Brobecker <brobecker at adacore dot com>]

> 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.

Retested on x86_64-linux and pushed.

-- 
Joel

>From 55426c9d52fdba13df81fcce1b18469cc0362e50 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 22 Jan 2014 18:40:20 +0400
Subject: [PATCH] DWARF: Set enum type "flag_enum" and "unsigned" flags at type
 creation.

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.
---
 gdb/ChangeLog                             |  8 +++
 gdb/dwarf2read.c                          | 82 ++++++++++++++++++++-----
 gdb/testsuite/ChangeLog                   |  4 ++
 gdb/testsuite/gdb.dwarf2/arr-subrange.c   | 20 +++++++
 gdb/testsuite/gdb.dwarf2/arr-subrange.exp | 99 +++++++++++++++++++++++++++++++
 5 files changed, 197 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/ChangeLog b/gdb/ChangeLog
index 53666b3..8f2f0dc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2014-02-27  Joel Brobecker  <brobecker@adacore.com>
+
+	* 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.
+
 2014-02-26  Pedro Alves  <palves@redhat.com>
 
 	* bsd-uthread.c (bsd_uthread_xfer_partial): Delete function.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index d7dc38c..00bba47 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -13107,6 +13107,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 = "<anonymous enumerator>";
+
+      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.  */
 
@@ -13156,6 +13219,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);
 }
 
@@ -13180,10 +13246,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)
@@ -13198,15 +13261,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)
 		    {
@@ -13237,10 +13291,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/ChangeLog b/gdb/testsuite/ChangeLog
index 476e1a3..b14257f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2014-02-26  Joel Brobecker  <brobecker@adacore.com>
 
+	* gdb.dwarf2/arr-subrange.c, gdb.dwarf2/arr-subrange.exp: New files.
+
+2014-02-26  Joel Brobecker  <brobecker@adacore.com>
+
 	* gdb.dwarf2/arr-stride.c: New file.
 	* gdb.dwarf2/arr-stride.exp: New file.
 
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 <http://www.gnu.org/licenses/>.  */
+
+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 <http://www.gnu.org/licenses/>.
+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