Re: [RFA/DWARF] constant class of DW_AT_high_pc is offset for version >=4 only.

[Joel Brobecker <brobecker@adacore.com>]

Hi Mark,

First of all, thanks for the comments!

On Tue, Feb 18, 2014 at 05:02:49PM +0100, Mark Wielaard wrote:
> On Tue, 2014-02-18 at 14:30 +0100, Joel Brobecker wrote:
> >   1. The introduction of attr_value_as_address, to be used
> >      in place of DW_ADDR when dealing with address attributes.
> >      I left a few uses of this macro in the situations where
> >      we actually know that the form is an address form.
> 
> This accepts any form as address/unsigned. I would at least check that
> it is either DW_FORM_data4 or DW_FORM_data8 (even better would be to
> check the CU address width too, although that would require to pass
> around cu too, which might not be practical).

I don't mind adding that, but the real question is what would we do
if we found an unexpected size? We could emit a complaint, but
I wouldn't necessarily stop processing the unit's debugging info.
I think the guideline here is that we should try to do our best
within reasonable constraints. I think adding a complaint, here,
is of marginal value as presumably the address read in the odd
format might be correct. On the other hand, any dump of the debugging
info should quickly reveal the format used for those addresses.

> Also I would add a comment
> that this is really to work around buggy producers.

I will definitely add a comment saying something about some compiler
producing odd debugging info. It seems logical to me that address
attributes would naturally be encoded using an address form, but is
a constant form clearly forbidden by the (older) standard(s)?
I would therefore just label the format chosen as unusual. :-).

> > @@ -4201,7 +4217,7 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
> >    attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
> >    if (attr)
> >      {
> > -      cu->base_address = DW_ADDR (attr);
> > +      cu->base_address = attr_value_as_address (attr);
> >        cu->base_known = 1;
> >      }
> 
> Note that this might break for DWARF5. See http://dwarfstd.org/ShowIssue.php?issue=120719.1

Interesting. I am curious why you would handle this attribute as
an offset even when the value is encoded in address form? Would
that not help improve backward compatibility with older versions
of DWARF?

But regardless, I think my change doesn't break the current behavior;
and to support the DWARF5 standard, the behavior implemented by
the function is still correct (reading the data from the correct
union field). You will need to add some code right after the call,
regardless, which adds the base address if version >= 5.

> In general I would only use attr_value_as_address for attributes (low_pc
> and high_pc) which you know a buggy producer might encode with
> DW_FORM_data[48].

I think this suggestion goes against the spirit of trying to do our
best. Not using the function means reading the attribute value from
the wrong field of the attribute union, which means increasing our
chances of getting it wrong. On the other hand, using the function
might allow us to read the correct address and get things to actually
work.

Also, I don't have acces to said compiler, so I can't try to study
its output and list the attributes using this constant form. But
if the low/hi pc attributes use a constant form, why not the
entry_pc?

-- 
Joel