This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils 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] libdw: Add new functions dwarf_getlocation_attr and dwarf_getlocation_die.


On Tue, 2013-09-03 at 10:40 +0200, Jan Kratochvil wrote:
> On Fri, 30 Aug 2013 23:55:12 +0200, Mark Wielaard wrote:
> > DW_OP_implicit_value and DW_OP_GNU_implicit_pointer already had
> > their own special accessors (dwarf_getlocation_implicit_value
> > and dwarf_getlocation_implicit_pointer), but it seemed consistent
> > to include them in the new more generic accessors too.
> 
> So dwarf_getlocation_implicit_value and dwarf_getlocation_implicit_pointer can
> be deprecated - that is removed from the .h interfaces?

Yes, I think we should encourage people to use the more generic
functions from now on. But no, I don't think actually removing them from
the libdw.h header is a good idea. IMHO we shouldn't force people to
rewrite their code if they were using the old functions already (even if
we can do it in a binary compatible way). It is just that in new code
one can use the new functions generically.

> > +	  newloc->number2 = (Dwarf_Word) data; /* start of block inc. len.  */
> >  	  /* XXX Check size.  */
> >  	  get_uleb128 (newloc->number, data); /* Block length.  */
> >  	  if (unlikely ((Dwarf_Word) (end_data - data) < newloc->number))
> >  	    goto invalid;
> > -	  newloc->number2 = data - block->data; /* Relative block offset.  */
> 
> I think it breaks ABI.  Dwarf_Op is a public structure.  Layout of Dwarf_Op
> for each of the operations is not explicitly documented but there are various
> elfutils API details which are not completely documented.
> 
> I do not think it is such a concern but I do not want to be a joker.

Yes, you are right, we did expose the number2. And I hadn't considered
whether that would be an ABI change. We are indeed changing the value as
it is used internally. But is that really breaking ABI? Before the value
was useless to the user without using the dwarf_getlocation_* helper
function. And it still is useless without using the function. I don't
think it is reasonable for a user to depend on the value of number2 in
these cases.

We could explicitly document somewhere that the Dwarf_Op number/number2
values only have meaning if they have non-reference values according to
the DWARF standard and don't have to be looked up/used with the
dwarf_getlocation_* functions.

The dwarf_getlocation_* accessor functions all do already say: "The OP
pointer must point into an expression that dwarf_getlocation or
dwarf_getlocation_addr has returned given the same ATTR." I think that
is a big hint to the user not to rely on the "raw value" directly
already.

Cheers,

Mark


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