* [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
@ 2014-01-27 8:09 Joel Brobecker
2014-02-10 14:29 ` Joel Brobecker
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-01-27 8:09 UTC (permalink / raw)
To: gdb-patches
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 = "<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. */
@@ -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 <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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-01-27 8:09 [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation Joel Brobecker
@ 2014-02-10 14:29 ` Joel Brobecker
2014-02-17 20:20 ` Joel Brobecker
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-02-10 14:29 UTC (permalink / raw)
To: gdb-patches
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 = "<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. */
>
> @@ -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 <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
--
Joel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-01-27 8:09 [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation Joel Brobecker
2014-02-10 14:29 ` Joel Brobecker
@ 2014-02-17 20:20 ` Joel Brobecker
2014-02-19 14:34 ` Mark Wielaard
2014-02-26 18:42 ` pushed: " Joel Brobecker
3 siblings, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-02-17 20:20 UTC (permalink / raw)
To: gdb-patches
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 = "<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. */
>
> @@ -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 <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
--
Joel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-01-27 8:09 [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation Joel Brobecker
2014-02-10 14:29 ` Joel Brobecker
2014-02-17 20:20 ` Joel Brobecker
@ 2014-02-19 14:34 ` Mark Wielaard
2014-02-19 15:18 ` Mark Wielaard
2014-02-26 18:42 ` pushed: " Joel Brobecker
3 siblings, 1 reply; 13+ messages in thread
From: Mark Wielaard @ 2014-02-19 14:34 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Mon, 2014-01-27 at 08:19 +0400, Joel Brobecker wrote:
> 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.
This looks suspicious. I think the explicit sign-extension is in general
wrong. It seems to assume that the DWARF producer encoded the
DW_AT_upper_bound wrongly (not as a signed value, but as an unsigned
value that needs to be sign-extended). Something like that might happen
in theory if the producer used DW_FORM_data[1248] because DWARF doesn't
define how to encode signed values in that case. But in practice this
seems to have been settled by interpreting these values as zero-extended
values (not sign-extended) and by the producer using either
DW_FORM_sdata or DW_FORM_udata to remove any ambiguity (like in your
testcase).
Does anything break if you just remove the sign-extension part?
If not, then you don't have to go through the whole
update_enumeration_type_from_children. Or do you need that for anything
else?
Cheers,
Mark
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-19 14:34 ` Mark Wielaard
@ 2014-02-19 15:18 ` Mark Wielaard
2014-02-21 9:21 ` Joel Brobecker
2014-02-26 18:32 ` Joel Brobecker
0 siblings, 2 replies; 13+ messages in thread
From: Mark Wielaard @ 2014-02-19 15:18 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Wed, 2014-02-19 at 15:34 +0100, Mark Wielaard wrote:
> Does anything break if you just remove the sign-extension part?
> If not, then you don't have to go through the whole
> update_enumeration_type_from_children. Or do you need that for anything
> else?
So, this patch doesn't show any regressions in the testsuite:
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 54c538a..0b5de99 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14303,7 +14303,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
LONGEST low, high;
int low_default_is_valid;
const char *name;
- LONGEST negative_mask;
orig_base_type = die_type (die, cu);
/* If ORIG_BASE_TYPE is a typedef, it will not be TYPE_UNSIGNED,
@@ -14433,13 +14432,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
}
}
- negative_mask =
- (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);
- if (!TYPE_UNSIGNED (base_type) && (low & negative_mask))
- low |= negative_mask;
- if (!TYPE_UNSIGNED (base_type) && (high & negative_mask))
- high |= negative_mask;
-
range_type = create_range_type (NULL, orig_base_type, low, high);
/* Mark arrays with dynamic length at least as an array of unspecified
So, my hope is that sign extension hack really isn't needed.
Of course it could be that there is some case where it was really needed
and there just isn't a test case for it. Does anybody know/remember?
Thanks,
Mark
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-19 15:18 ` Mark Wielaard
@ 2014-02-21 9:21 ` Joel Brobecker
2014-02-25 14:32 ` Mark Wielaard
2014-02-26 18:32 ` Joel Brobecker
1 sibling, 1 reply; 13+ messages in thread
From: Joel Brobecker @ 2014-02-21 9:21 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gdb-patches
Hi Mark,
> > This looks suspicious. I think the explicit sign-extension is in general
> > wrong. It seems to assume that the DWARF producer encoded the
> > DW_AT_upper_bound wrongly (not as a signed value, but as an unsigned
> > value that needs to be sign-extended). Something like that might happen
> > in theory if the producer used DW_FORM_data[1248] because DWARF doesn't
> > define how to encode signed values in that case. But in practice this
> > seems to have been settled by interpreting these values as zero-extended
> > values (not sign-extended) and by the producer using either
> > DW_FORM_sdata or DW_FORM_udata to remove any ambiguity (like in your
> > testcase).
Thanks again for looking at the patch, and for your comments.
Interestingly, I had thought at the time that the only way to express
signedness of the underlying value was by using a base type as a subtype
(IIRC) which, of course, if a lot of data for one small attribute.
The use of the form seems like a much more efficient way to achieve
that goal. But the above explains why I interpreted the current code
as a way to work around the fact that compilers might not be emitting
the required subtype for enums with negative values. Not sure how
correct or not that interpretation was...
> > Does anything break if you just remove the sign-extension part?
> > If not, then you don't have to go through the whole
> > update_enumeration_type_from_children. Or do you need that for anything
> > else?
>
> So, this patch doesn't show any regressions in the testsuite:
I will verify your fix against my testcase as well, but I like the idea
of removing code that should normally not be there.
But to answer your question above (do I also need my patch?), I think
the need might become less obvious once your patch is in. But I also
think it would probably be cleaner to have a complete type right from
the get-go, especially since I don't think the patch actually
complexifies the code (maybe even the opposite). That being said,
I'm not strongly attached to it, as long as GDB does TRT :-).
Thanks again for your feedback!
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 54c538a..0b5de99 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -14303,7 +14303,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
> LONGEST low, high;
> int low_default_is_valid;
> const char *name;
> - LONGEST negative_mask;
>
> orig_base_type = die_type (die, cu);
> /* If ORIG_BASE_TYPE is a typedef, it will not be TYPE_UNSIGNED,
> @@ -14433,13 +14432,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
> }
> }
>
> - negative_mask =
> - (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);
> - if (!TYPE_UNSIGNED (base_type) && (low & negative_mask))
> - low |= negative_mask;
> - if (!TYPE_UNSIGNED (base_type) && (high & negative_mask))
> - high |= negative_mask;
> -
> range_type = create_range_type (NULL, orig_base_type, low, high);
>
> /* Mark arrays with dynamic length at least as an array of unspecified
>
>
> So, my hope is that sign extension hack really isn't needed.
> Of course it could be that there is some case where it was really needed
> and there just isn't a test case for it. Does anybody know/remember?
>
> Thanks,
>
> Mark
--
Joel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-21 9:21 ` Joel Brobecker
@ 2014-02-25 14:32 ` Mark Wielaard
0 siblings, 0 replies; 13+ messages in thread
From: Mark Wielaard @ 2014-02-25 14:32 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Fri, 2014-02-21 at 10:21 +0100, Joel Brobecker wrote:
> Interestingly, I had thought at the time that the only way to express
> signedness of the underlying value was by using a base type as a subtype
> (IIRC) which, of course, if a lot of data for one small attribute.
> The use of the form seems like a much more efficient way to achieve
> that goal. But the above explains why I interpreted the current code
> as a way to work around the fact that compilers might not be emitting
> the required subtype for enums with negative values. Not sure how
> correct or not that interpretation was...
I think DWARF producers will normally use DW_FORM_sdata to indicate a
signed (negative) value. In theory there can be confusion if the
producer uses DW_FORM_data[1248] because you then don't know the
"encoding" of the value. That is also why the DWARF spec discourages the
use of those forms. And it explicitly doesn't define how to interpret
the value of DW_FORM_data[1248]. Second guessing the producer if it did
explicitly use DW_FORM_sdata or DW_FORM_udata seems wrong to me. If you
do want to do that, then only do it for DW_FORM_data[1248]. But I think
you will get it wrong because in practice when a producer uses those
forms it expects you to zero-extend the value if it is encoded in less
bits than the type size [*]. This is something that could use
clarification or a hint from the DWARF spec, because I think the current
wording isn't very helpful in practice.
> I will verify your fix against my testcase as well, but I like the idea
> of removing code that should normally not be there.
I hope it works, because the current code looks a bit fragile to me.
> But to answer your question above (do I also need my patch?), I think
> the need might become less obvious once your patch is in. But I also
> think it would probably be cleaner to have a complete type right from
> the get-go, especially since I don't think the patch actually
> complexifies the code (maybe even the opposite). That being said,
> I'm not strongly attached to it, as long as GDB does TRT :-).
I haven't looked too deeply into how GDB represents types. But the
problem you are trying to solve here is trying to determine whether the
type represented by a DW_TAG_enumeration_type is signed or unsigned. You
try to determine that by iterating through all DW_TAG_enumerator
children values and if you see one encoded as a negative value, you then
assume the enumeration_type as a whole is signed, otherwise you assume
unsigned. I am not really convinced that is the proper way to determine
whether the enumeration_type is signed or not (the language might not
care about that and the result might be random depending on DW_FORM used
for the enumerator constants).
I am intrigued by the idea of setting TYPE_FLAG_ENUM. The heuristic to
determine that is very cute. It looks like it is only used for printing
a value of an enum type. Does it work reliably? Are there languages that
let you declare enumeration values that way? (If so, would it make sense
to propose a DWARF extension attribute to mark them?) Or is this mainly
for detecting people defining enums by hand as flag values?
Cheers,
Mark
[*] This is something fixed in gdb some years ago:
commit 053315c2134b7832b351c599fa3fa11abf6ca4e7
Author: Tom Tromey <tromey@redhat.com>
Date: Wed Jul 28 20:05:03 2010 +0000
* dwarf2read.c (dwarf2_const_value_data): Never sign extend.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-19 15:18 ` Mark Wielaard
2014-02-21 9:21 ` Joel Brobecker
@ 2014-02-26 18:32 ` Joel Brobecker
2014-02-26 18:58 ` Joel Brobecker
2014-02-27 10:30 ` Mark Wielaard
1 sibling, 2 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-02-26 18:32 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gdb-patches
> So, this patch doesn't show any regressions in the testsuite:
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 54c538a..0b5de99 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -14303,7 +14303,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
> LONGEST low, high;
> int low_default_is_valid;
> const char *name;
> - LONGEST negative_mask;
>
> orig_base_type = die_type (die, cu);
> /* If ORIG_BASE_TYPE is a typedef, it will not be TYPE_UNSIGNED,
> @@ -14433,13 +14432,6 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
> }
> }
>
> - negative_mask =
> - (LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);
> - if (!TYPE_UNSIGNED (base_type) && (low & negative_mask))
> - low |= negative_mask;
> - if (!TYPE_UNSIGNED (base_type) && (high & negative_mask))
> - high |= negative_mask;
> -
I finally had some time to test this patch, and unfortunately,
it does introduce some regression (in Ada). For instance in
homonym.exp:
type Integer_Range is new Integer range -100 .. 100;
subtype Local_Type is Integer_Range;
This is what GDB would print afterwards:
(gdb) ptype local_type
type = range 4294967196 .. 100
The lower bound should be -100. The debugging info is generated
as follow:
<2><80>: Abbrev Number: 2 (DW_TAG_subrange_type)
<81> DW_AT_lower_bound : 0xffffff9c
<85> DW_AT_upper_bound : 100
<86> DW_AT_name : (indirect string, offset: 0x76): homonym__get_value__local_type___XDLU_100m__100
<8a> DW_AT_type : <0x37>
And the corresponding abbrev gives the form:
2 DW_TAG_subrange_type [no children]
DW_AT_lower_bound DW_FORM_data4
DW_AT_upper_bound DW_FORM_data1
DW_AT_name DW_FORM_strp
DW_AT_type DW_FORM_ref4
DW_AT value: 0 DW_FORM value: 0
It's the dreaded DW_FORM_dataN form... And unfortunately, I get the same
representation with pre-versions of GCC 4.9, so it looks like we're not
going to be able to remove that bit anytime soon :-(.
I'll add a comment in the code...
--
Joel
^ permalink raw reply [flat|nested] 13+ messages in thread
* pushed: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-01-27 8:09 [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation Joel Brobecker
` (2 preceding siblings ...)
2014-02-19 14:34 ` Mark Wielaard
@ 2014-02-26 18:42 ` Joel Brobecker
3 siblings, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-02-26 18:42 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 509 bytes --]
> 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
[-- Attachment #2: 0001-DWARF-Set-enum-type-flag_enum-and-unsigned-flags-at-.patch --]
[-- Type: text/x-diff, Size: 13713 bytes --]
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-26 18:32 ` Joel Brobecker
@ 2014-02-26 18:58 ` Joel Brobecker
2014-02-27 10:30 ` Mark Wielaard
1 sibling, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-02-26 18:58 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 174 bytes --]
> I'll add a comment in the code...
Attached is the patch adding the comment...
gdb/ChangeLog:
* dwarf2read.c (read_subrange_type): Add comment.
Thanks,
--
Joel
[-- Attachment #2: 0001-Add-comment-in-dwarf2read.c-read_subrange_type.patch --]
[-- Type: text/x-diff, Size: 1810 bytes --]
From dbb9c2b1f231262ece36790241fe1fc3902cf03d Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 26 Feb 2014 10:53:05 -0800
Subject: [PATCH] Add comment in dwarf2read.c::read_subrange_type
This comment explains why we sometimes sign-extend the range type
bounds when we normally shouldn't have to.
gdb/ChangeLog:
* dwarf2read.c (read_subrange_type): Add comment.
---
gdb/ChangeLog | 4 ++++
gdb/dwarf2read.c | 7 +++++++
2 files changed, 11 insertions(+)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8f2f0dc..49d8113 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2014-02-27 Joel Brobecker <brobecker@adacore.com>
+ * dwarf2read.c (read_subrange_type): Add comment.
+
+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.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 00bba47..52208d6 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14522,6 +14522,13 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
}
}
+ /* Normally, the DWARF producers are expected to use a signed
+ constant form (Eg. DW_FORM_sdata) to express negative bounds.
+ But this is unfortunately not always the case, as witnessed
+ with GCC, for instance, where the ambiguous DW_FORM_dataN form
+ is used instead. To work around that ambiguity, we treat
+ the bounds as signed, and thus sign-extend their values, when
+ the base type is signed. */
negative_mask =
(LONGEST) -1 << (TYPE_LENGTH (base_type) * TARGET_CHAR_BIT - 1);
if (!TYPE_UNSIGNED (base_type) && (low & negative_mask))
--
1.8.3.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-26 18:32 ` Joel Brobecker
2014-02-26 18:58 ` Joel Brobecker
@ 2014-02-27 10:30 ` Mark Wielaard
2014-02-27 11:15 ` Mark Wielaard
2014-02-27 14:29 ` Joel Brobecker
1 sibling, 2 replies; 13+ messages in thread
From: Mark Wielaard @ 2014-02-27 10:30 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Wed, 2014-02-26 at 10:31 -0800, Joel Brobecker wrote:
> I finally had some time to test this patch, and unfortunately,
> it does introduce some regression (in Ada). For instance in
> homonym.exp:
Sorry, I didn't have gcc-gnat installed and so missed this. I have it
installed now. BTW. Are there any overviews of what are expected
results? For make check RUNTESTFLAGS='--directory gdb.ada' I get:
=== gdb Summary ===
# of expected passes 490
# of unexpected failures 29
# of unexpected successes 8
# of expected failures 2
# of known failures 1
# of unsupported tests 3
Is that reasonable? The amount of failures seems a bit high. The
testsuite is not supposed to be (near) zero-fail?
> type Integer_Range is new Integer range -100 .. 100;
> subtype Local_Type is Integer_Range;
>
> This is what GDB would print afterwards:
>
> (gdb) ptype local_type
> type = range 4294967196 .. 100
>
> The lower bound should be -100. The debugging info is generated
> as follow:
>
> <2><80>: Abbrev Number: 2 (DW_TAG_subrange_type)
> <81> DW_AT_lower_bound : 0xffffff9c
> <85> DW_AT_upper_bound : 100
> <86> DW_AT_name : (indirect string, offset: 0x76): homonym__get_value__local_type___XDLU_100m__100
> <8a> DW_AT_type : <0x37>
>
> And the corresponding abbrev gives the form:
>
> 2 DW_TAG_subrange_type [no children]
> DW_AT_lower_bound DW_FORM_data4
> DW_AT_upper_bound DW_FORM_data1
> DW_AT_name DW_FORM_strp
> DW_AT_type DW_FORM_ref4
> DW_AT value: 0 DW_FORM value: 0
>
> It's the dreaded DW_FORM_dataN form... And unfortunately, I get the same
> representation with pre-versions of GCC 4.9, so it looks like we're not
> going to be able to remove that bit anytime soon :-(.
Grrr and sigh. Why does GCC do that? Encoding the negative lower bound
like that actually takes up more space than simply using DW_FORM_sdata.
The comment in the GCC sources even says this is suboptimal:
/* Otherwise represent the bound as an unsigned value with the
precision of its type. The precision and signedness of the
type will be necessary to re-interpret it unambiguously. */
Luckily it seems GCC only does this when adding bounds info. But it does
seem we are stuck with it for now :{
I'll see if I can fix GCC so one day day in the far, far, future this
hack won't be necessary.
Cheers,
Mark
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-27 10:30 ` Mark Wielaard
@ 2014-02-27 11:15 ` Mark Wielaard
2014-02-27 14:29 ` Joel Brobecker
1 sibling, 0 replies; 13+ messages in thread
From: Mark Wielaard @ 2014-02-27 11:15 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Thu, 2014-02-27 at 11:30 +0100, Mark Wielaard wrote:
> Sorry, I didn't have gcc-gnat installed and so missed this. I have it
> installed now. BTW. Are there any overviews of what are expected
> results? For make check RUNTESTFLAGS='--directory gdb.ada' I get:
>
> === gdb Summary ===
>
> # of expected passes 490
> # of unexpected failures 29
> # of unexpected successes 8
> # of expected failures 2
> # of known failures 1
> # of unsupported tests 3
>
> Is that reasonable? The amount of failures seems a bit high. The
> testsuite is not supposed to be (near) zero-fail?
It was pointed out on irc that it would be helpful to mention the
gcc-gnat version used and the actual failures from gdb.sum.
GNAT 4.4.7 20120313 (Red Hat 4.4.7-4)
FAIL: gdb.ada/aliased_array.exp: print bt
FAIL: gdb.ada/array_bounds.exp: print table'first
FAIL: gdb.ada/array_bounds.exp: print table'last
FAIL: gdb.ada/arrayidx.exp: print e_one_two_three, indexes off
FAIL: gdb.ada/arrayidx.exp: print r_two_three, indexes off
FAIL: gdb.ada/arrayidx.exp: print p_one_two_three, indexes off
FAIL: gdb.ada/arrayidx.exp: print e_one_two_three
FAIL: gdb.ada/arrayidx.exp: print r_two_three
FAIL: gdb.ada/arrayidx.exp: print p_one_two_three
FAIL: gdb.ada/dyn_loc.exp: info locals
FAIL: gdb.ada/enum_idx_packed.exp: print full
FAIL: gdb.ada/enum_idx_packed.exp: print full'first
FAIL: gdb.ada/iwide.exp: print sp_access.all
FAIL: gdb.ada/iwide.exp: print dp_access.all
FAIL: gdb.ada/mi_interface.exp: compilation foo.adb
FAIL: gdb.ada/null_array.exp: ptype my_table
FAIL: gdb.ada/optim_drec.exp: print z
FAIL: gdb.ada/packed_tagged.exp: print x
FAIL: gdb.ada/taft_type.exp: print w.e.all
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print My_Object with null Ptr
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print My_Object.Ptr when null
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print My_P_Object with null Ptr
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print My_P_Object.Ptr when null
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print my_object after setting Ptr
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print My_P_Object.Ptr when no longer null
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print my_p_object after setting Ptr
FAIL: gdb.ada/unc_arr_ptr_in_var_rec.exp: print My_P_Object.Ptr when no longer null
FAIL: gdb.ada/variant_record_packed_array.exp: print adress content
FAIL: gdb.ada/whatis_array_val.exp: print full
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation.
2014-02-27 10:30 ` Mark Wielaard
2014-02-27 11:15 ` Mark Wielaard
@ 2014-02-27 14:29 ` Joel Brobecker
1 sibling, 0 replies; 13+ messages in thread
From: Joel Brobecker @ 2014-02-27 14:29 UTC (permalink / raw)
To: Mark Wielaard; +Cc: gdb-patches
> Sorry, I didn't have gcc-gnat installed and so missed this. I have it
> installed now. BTW. Are there any overviews of what are expected
> results? For make check RUNTESTFLAGS='--directory gdb.ada' I get:
>
> === gdb Summary ===
>
> # of expected passes 490
> # of unexpected failures 29
> # of unexpected successes 8
> # of expected failures 2
> # of known failures 1
> # of unsupported tests 3
>
> Is that reasonable? The amount of failures seems a bit high. The
> testsuite is not supposed to be (near) zero-fail?
IMO, it depends too much on the compiler used. Although we try to
actively contribute all our GCC patches the same way we contribute
our GDB patches, there are always a few that keep missing in the FSF
tree. To have near-clean results, I think you would need to use our
latest GPL'ed binary (we publish one each year). That's why you'll
see me generate C testcases as much as possible, even if the problem
is only showing up in Ada code.
Regardless of that, I don't expect anyone but me to really set their
environment up towards clean results. As long as it's not regressing
in your environment, I'm happy taking take care of accidental
regressions.
--
Joel
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-02-27 14:29 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-27 8:09 [RFA/DWARF] Set enum type "flag_enum" and "unsigned" flags at type creation Joel Brobecker
2014-02-10 14:29 ` Joel Brobecker
2014-02-17 20:20 ` Joel Brobecker
2014-02-19 14:34 ` Mark Wielaard
2014-02-19 15:18 ` Mark Wielaard
2014-02-21 9:21 ` Joel Brobecker
2014-02-25 14:32 ` Mark Wielaard
2014-02-26 18:32 ` Joel Brobecker
2014-02-26 18:58 ` Joel Brobecker
2014-02-27 10:30 ` Mark Wielaard
2014-02-27 11:15 ` Mark Wielaard
2014-02-27 14:29 ` Joel Brobecker
2014-02-26 18:42 ` pushed: " Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).