On Tue, 2014-04-15 at 08:21 +0200, Jakub Jelinek wrote:
> On Mon, Apr 14, 2014 at 03:48:06PM -0700, Cary Coutant wrote:
> > >> Also note that size_of_die and value_format will still choose
> > >> DW_FORM_data[1248] for dw_val_class_unsigned_const in most cases.
> > >> Don't you really want to use DW_FORM_udata?
> > >
> > > DW_FORM_data[1248] is in many cases smaller than DW_FORM_udata (though,
> > > one
> > > has to take into account possibly larger .debug_abbrev size).
> >
> > Yes, but it's up to the consumer to deduce from context whether the
> > value is signed or unsigned. If it's still true that GDB will
> > interpret DW_FORM_data[1248] as signed (as the deleted comment said),
> > and you output a value between 128 and 255 using DW_FORM_data1, this
> > isn't going to work. Maybe that comment only applies to
> > DW_FORM_data[48] (whichever matches HOST_WIDE_INT)?
>
> If there is no agreement between producer and consumer what is signed
> and what is unsigned for DW_FORM_data[1248], then of course that is a
> problem, I wasn't aware of such disagreements.
Cary is right, I should have clarified/fixed the comment instead of just
removing it completely. There used to be a very brief period where GDB
(5.x timeframe) treated DW_FORM_data[1248] as signed. This hasn't been
true for a very long time anymore. There must indeed be agreement
between the producers and consumers how to interpret these forms. GCC
always outputs DW_FORM_data[1248] as unsigned values and consumers (at
least GDB and elfutils explicitly agreed on this) explicitly always
zero-extend these forms. This is documented in other places in
dwarf2out.c, in the GDB sources and elfutils comments, but it would not
be a bad idea to have a comment here too to make sure this is kept
consistent.
The other issue is when HOST_WIDE_INT is smaller than 64 bits. I didn't
want to fix that issue in this patch because I don't have any such
setups. And as Cary also pointed out in the previous thread that does
require some changes to how "doubles" are treated. It would need a new
add_AT_unsigned_double function. And I think it would mean fixing the
case were add_AT_double generates either a constant class or a block
class form (add_AT_double is used for both those cases, but not in all
places where it is used is a block class form allowed - here it is for a
DW_AT_const, but it isn't in all cases were it is used in dwarf2out.c).
But those issues/TODOs are out of scope for this patch.
Added a clarifying comment to the code and reinstated the TODO for the
double case. OK to push?
Thanks,
Mark
commit f7c10a0ae5e99b680335b1a13e082fcad4ad0236
Author: Mark Wielaard <m...@redhat.com>
Date: Fri Mar 7 22:27:15 2014 +0100
Add DW_AT_const_value as unsigned or int depending on type and value used.
As the comment in the code already indicated DWARF2 does provide
DW_FORM_sdata/DW_FORM_udata to represent signed/unsigned data.
Enumeration constants wider than HOST_WIDE_INT are already handled
separately. Those constant values that do fit a HOST_WIDE_INT can
be encoded as signed or unsigned depending on type and value for
more efficient encoding.
* dwarf2out.c (gen_enumeration_type_die): Add DW_AT_const_value
as unsigned or int depending on type and value used.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 802b587..e4d6669 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,8 @@
+2014-03-21 Mark Wielaard <m...@redhat.com>
+
+ * dwarf2out.c (gen_enumeration_type_die): Add DW_AT_const_value
+ as unsigned or int depending on type and value used.
+
2014-03-20 Mark Wielaard <m...@redhat.com>
* dwarf2out.c (add_bound_info): If HOST_WIDE_INT is big enough,
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 7eef56c..70b0716 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -17369,22 +17369,23 @@ gen_enumeration_type_die (tree type, dw_die_ref
context_die)
if (simple_type_size_in_bits (TREE_TYPE (value))
<= HOST_BITS_PER_WIDE_INT || tree_fits_shwi_p (value))
- /* DWARF2 does not provide a way of indicating whether or
- not enumeration constants are signed or unsigned. GDB
- always assumes the values are signed, so we output all
- values as if they were signed. That means that
- enumeration constants with very large unsigned values
- will appear to have negative values in the debugger.
-
- TODO: the above comment is wrong, DWARF2 does provide
- DW_FORM_sdata/DW_FORM_udata to represent signed/unsigned data.
- This should be re-worked to use correct signed/unsigned
- int/double tags for all cases, instead of always treating as
- signed. */
- add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value));
+ {
+ /* For constant forms created by add_AT_unsigned DWARF
+ consumers (GDB, elfutils, etc.) always zero extend
+ the value. Only when the actual value is negative
+ do we need to use add_AT_int to generate a constant
+ form that can represent negative values. */
+ HOST_WIDE_INT val = TREE_INT_CST_LOW (value);
+ if (TYPE_UNSIGNED (TREE_TYPE (value)) || val >= 0)
+ add_AT_unsigned (enum_die, DW_AT_const_value,
+ (unsigned HOST_WIDE_INT) val);
+ else
+ add_AT_int (enum_die, DW_AT_const_value, val);
+ }
else
/* Enumeration constants may be wider than HOST_WIDE_INT. Handle
- that here. */
+ that here. TODO: This should be re-worked to use correct
+ signed/unsigned double tags for all cases. */
add_AT_double (enum_die, DW_AT_const_value,
TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value));
}
