Re: [PATCH] Add C++ wrappers for iterating through various libdw and libdwfl objects

[Mark Wielaard <mjw at redhat dot com>]

On Tue, 2015-03-31 at 15:05 +0200, Petr Machata wrote:
> Mark Wielaard <mjw@redhat.com> writes:
> > On Fri, 2015-03-27 at 21:32 +0100, Petr Machata wrote:
> >> Mark Wielaard <mjw@redhat.com> writes:
> >> > On Wed, 2015-03-18 at 00:55 +0100, Petr Machata wrote:
> >> The iterators are essentially glorified structs.  The usual C++ ABI
> >> concerns don't apply (reordering, adding virtuals).  They only use
> >> single inheritance off system classes, and those only serve for tagging
> >> (so that algorithms can dispatch based on iterator type).
> >
> > But won't you have issues when some program uses one version of the
> > elfutils iterators and it also uses a library which was compiled against
> > an elfutils version that uses a different one? Or can/should/will they
> > never mix anyway (some of your answers below hint at that).
> 
> Depends on how they change, but in general yes, that would be a problem.
> E.g. moving methods around won't be a problem, adding methods won't be a
> problem.  Changing fields however would be a problem.
> 
> Unlike C, this can't be mitigated by adding a padding field, because the
> user inlines both the layout information, and the behavior (all the code
> is in headers as well).
> 
> The way it's written now, care must be taken that any DSO that uses
> these iterators on the API boundary is built against version of the
> iterator that's compatible with its clients.
> 
> If these types should be safely usable on API boundary of another
> library, we would need to move most of them out of the header, and
> expose only pure pimpl'd interface.  That would make the interface
> somewhat more heavy-weight.

Lets not do that then. But it would be nice to figure out a way to use
libabigail to flag any abi-incompatibilities we might accidentally
introduce.

> > Great. I wouldn't mind if you prototyped how that might look in the
> > future. But again, see below. I think my idea of exceptions is slightly
> > skewed.
> >
> 
> Something like:
> 
> namespace elfutils
> {
>   class exception
>     : public std::exception
>   {};
> 
>   class libdw_exception
>     : public elfutils::exception
>   {
>     int m_error;
>   public:
>     explicit libdw_exception (int error) : m_error (error) {}
>     virtual const char *what() const
>     { return dwarf_errmsg (m_error); }
>   };
> }
> 
> Then constructing libdw_exception itself is super cheap (though actually
> throwing and unwinding is of course still expensive), and what() returns
> directly the static buffer from inside libdw.
> 
> You would similarly have libelf_exception and libdwfl_exception.

Does that need some kind of hierarchy?
Should a libdwfl_exception extend libdw_exception for example?
Or is there some kind of nested exception concept, so you could get the
original libelf_exception from a libdw_exception? Or am I thinking too
"java-like" again?

> > It might not be an issue. Maybe we can just assume that a whole system
> > is completely using the gcc c++89 abi or the gcc c++11 abi but never two
> > at the same time.
> 
> I'm eyeing the list of ABI changes between C++98 and C++11.  I don't see
> any that could affect the iterators.  There is "`vector::data()`'s
> return type changes from `pointer` to `_Tp*`" but ::data itself is only
> available in C++11, so I think this doesn't apply for cross-C++-version
> compilation.  (It might maybe apply for C++11 as compiled by pre-4.6 GCC
> vs. post-4.6 GCC, but _that_ I'm pretty sure we can safely disregard.)

Maybe libabigail can just tell use when we comparing the object files
compiled with the old and new C++ abi?

> >> It is a convention in these header files.  It could be named just
> >> "impl", or maybe "aux", or something else still, I don't think there is
> >> convention to speak of.  Point is somehow to signal to the client that
> >> there be lions.
> >
> > It might be my own confusion. I am still thinking this is exposing an
> > abi.
> 
> Yeah, in a way.  But if a DSO exposes these auxiliary symbols as public
> symbols, that's that DSO's problem, not ours.
> 
> But as written above.  Since both behavior and layout are inlined in
> clients, clients that end up binary-interacting with each other have to
> make sure their understanding of these artifacts is compatible.

Isn't that also our problem then? I assume we like people to write
libraries based on these classes. But that means we must maintain
compatibility even for such "artifacts" because the user might want to
upgrade their elfutils version independent from other libraries that
also might be based on elfutils.

> > It feels like the iterator is slightly too low-level and/or doesn't give
> > enough ways to ask how/what you really want to iterate over. I think a
> > unit_iterator where you can select which unit kinds you would like to
> > iterate over would be very useful.
> 
> OK, I'll work on making this into a unit_iterator.  It will go through
> both non-type units and type units.

Nice.

> >> Well, you are not supposed to compare iterators coming from different
> >> sources, so this doesn't need to work.
> >
> > Is that common C++ knowledge? How do you define "source"? I assume
> > people will expect that if you compare two "wrong/incompatible"
> > iterators anything can happen (including crashes)? I guess this just
> > shows my inexperience with C++ iterators. Once you hit an "end" iterator
> > you always stop processing and won't try comparing against anything.
> 
> E.g. libstdc++ has this mode where when you build with -D_GLIBCXX_DEBUG
> the following would be diagnosed:
> 
> #include <vector>
> int main () {
>   std::vector <int> a;
>   std::vector <int> b;
>   a.begin () == b.begin ();
> }
> 
> $ ./a.out 
> /usr/include/c++/4.9.2/debug/safe_iterator.h:510:error: attempt to compare 
>     iterators from different sequences.
> 
> I think C++11 has a definition of "same source" that goes something
> like, given two iterators, they are from the same source if you can take
> one of them, ++ it sufficient number of times, and get to the point
> where both reference the same object.

That makes sense.

BTW. Feel free to not take some of my comments too seriously. I am just
abusing you to get a crash course in idiomatic C++. I am sure the
correct response to some of my "questions" is just "Go away! You clearly
have never hacked in C++ in anger. This is how it is done. Deal with
it!"

Cheers,

Mark