This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/5] Altera Nios II: dwarf generation fix


> A : host_integerp (value, TYPE_UNSIGNED (TREE_TYPE (value)))
> B : host_integerp (value, 0)
>
>                 AB      AB      AB      AB
> type_size,hwi | 00      01      10      11
> --------------+-------------------------------
> 32,32         | X       X       int     int
> 64,32         | X       X       int     int
> 32,64         | X       X       -       int
> 64,64         | X       X       int     int

In the third column (AB == 10), we're emitting a single int today even
though we know it's not technically correct: GDB will display the
unsigned value as a negative number. That's marginally better than
emitting nothing at all when the value is larger than an hwi, but I
was arguing that as long as we're adding the ability to emit the
constant as a double, why not also use a double for an unsigned that
doesn't fit in a signed hwi? Yes, it'll waste some space, but the
value will be correctly displayed as a result.

Upon further reflection, however...

This comment is wrong:

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

DWARF does in fact provide a way of indicating whether a constant is
signed or unsigned: DW_FORM_sdata and DW_FORM_udata. These forms were
in DWARF-2, and the following comment was added to the DWARF-3 spec:

"If one of the DW_FORM_data<n> forms is used to represent a signed or
unsigned integer, it can be hard for a consumer to discover the
context necessary to determine which interpretation is intended.
Producers are therefore strongly encouraged to use DW_FORM_sdata or
DW_FORM_udata for signed and unsigned integers respectively, rather
than DW_FORM_data<n>."

We should really be emitting unsigned constants using add_AT_unsigned:

  if (TYPE_UNSIGNED (TREE_TYPE (value)))
    {
      if (host_integerp (value, 1))
        add_AT_unsigned (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value));
      else
        add_AT_unsigned_double (enum_die, DW_AT_const_value,
                                TREE_INT_CST_HIGH (value),
                                TREE_INT_CST_LOW (value));
    }
  else
    {
      if (host_integerp (value, 0))
        add_AT_int (enum_die, DW_AT_const_value, TREE_INT_CST_LOW (value));
      else
        add_AT_double (enum_die, DW_AT_const_value,
                       TREE_INT_CST_HIGH (value), TREE_INT_CST_LOW (value));
    }

add_AT_unsigned_double would be new, and would need a new
dw_val_class_unsigned_const_double enum.

size_of_die() and value_format() will need to be changed to force the
use of DW_FORM_udata for dw_val_class_unsigned_const and
dw_val_class_unsigned_const_double. Given that GDB always treats
DW_FORM_data{1,2,4,8} as signed, we can leave signed values as they
are, but DW_FORM_sdata could also be used in cases where it would save
space.

> (The second row, AB=01 entry I'm not sure about -- that might be
> another '-' case in practice...)

I think that whole column is a '-' case: if an unsigned value fits in
a signed hwi, then it will also fit in an unsigned hwi (i.e., B => A).

-cary


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]