| Message ID | 1397510723.5633.49.camel@bordewijk.wildebeest.org |
|---|---|
| State | New |
| Headers | show |
>> * dwarf2out.c (gen_enumeration_type_die): Add DW_AT_const_value >> as unsigned or int depending on type and value used. > > Since stage 1 opened up I would like to request approval again to push > this. Patch rebased to current master attached. The discussion that led to that TODO is here: http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01086.html In that thread, I suggested a reorganization of this piece of code that would also take care of the wider-than-unsigned-hwi case. 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? -cary
On Mon, Apr 14, 2014 at 02:55:52PM -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). Jakub
>> 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)? At the very least, you should replace the comment block you removed with one that explains why GDB will interpret the values correctly. Keep in mind that the DWARF standard "strongly" encourages producers to use DW_FORM_udata and DW_FORM_sdata. -cary
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. Anyway, at least DW_FORM_data[1248] values which don't have topmost bit set should be always fine, because they are not ambiguous even if there is disagreement between producer and consumer on if it is signed or unsigned. Jakub
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b1706ce..355ba29 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2014-03-21 Mark Wielaard <mjw@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 <mjw@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..d4e4b17 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -17369,19 +17369,14 @@ 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)); + { + 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. */